8000 Navigation decoupling using events by OceanWolf · Pull Request #2 · fariza/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

Navigation decoupling using events #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

OceanWolf
Copy link

Have only made this work on TkAgg, will convert for GTK3 later. In fixing ConfigureSubplotsTk, I have noticed a marked decrease in dialogue reopening time :), I guess the "figure" creation slowed it down.

I have noticed problems with the home/back/forward buttons on testing, though I guess this bug already existed, so I will look into it and fix it with a separate PR later.

@fariza
8000 Copy link
Owner
fariza commented Aug 1, 2014

I will comment on specific lines as I understand the changes but the one thing that is clear.

Please split this in two PRs.

  • One for the tool instances. This I'm convinced.
  • Other for the Google handling and moving the add tool to the toolbar. This I'm not convinced

@fariza
Copy link
Owner
fariza commented Aug 1, 2014

Don't worry for the GTK changes. When we finish all the changes I can take care of that

if name in self._instances:
del self._instances[name]
self._handle_toggle(name)
if name in self._tools:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what case, name is not present in self._tools?

@OceanWolf
Copy link
Author

Hmm, I thought I may have done too much for one pull request, but it seemed like one would go in as a very small request, the other much larger, but I can try and separate them out and see, in which case it will become a part 2 and a part 3.

Do you know of a way to do that on github? If not, would it make it easier for you to wait until I have split the PRs before you start commenting?

What do you mean by Google handling?

@fariza
Copy link
Owner
fariza commented Aug 1, 2014

Sorry, I don't know.
You can leave this PR open, and do the other in another branch, so it's easier for you to follow your changes.

@fariza
Copy link
Owner
fariza commented Aug 1, 2014

Please describe the problem with the home/back/forward buttons. I haven't noticed anything.

@OceanWolf
Copy link
Author

Problem with home/back/forward:
it seems fine when I switched back, so it comes from me. I shall investigate. (it seems to forget the history).

PR:
If I understand correctly, I should revert all changes to this branch, except for the minimum of code required to keep tool instances alive, i.e. _instances -> tools.
I do that by taking a branch of this as is. Then switching back to this branch and taking the stuff out.

I will probably do this tomorrow as it gets late here.

@fariza
Copy link
Owner
fariza commented Aug 1, 2014

What I would do (I'm far from expert) I would create a new branch based on the original code. And port only the instances relate code. This way you keep this branch as reference. But do it as you like, at the end is the same

@OceanWolf
Copy link
Author

Reading up on the subject, I think I will perform a rebase as it seems cleaner.

Thinking about the code, I can actually see three PRs here, dividing the tool instances into two parts. Separating out the code should help you to see the logic behind the various bits of code.

I will let you know when it has become "safe" the code again ;).

@fariza
Copy link
Owner
fariza commented Aug 6, 2014

@OceanWolf
I'm going to be out of reach for the next two weeks (11 to 22), just in case you wonder why I don't answer.

@OceanWolf
Copy link
Author

@fariza No problem, my laptop died on me on Monday (general system board failure :'(), so trying to sort something out.  But yes, the reason for my silence so far.

Enjoy your holiday!

On Wednesday, 6 August 2014, 15:02, Federico Ariza notifications@github.com wrote:

@OceanWolf
I'm going to be out of reach for the next two weeks (11 to 22), just in case you wonder why I don't answer.

Reply to this email directly or view it on GitHub.

@fariza
Copy link
Owner
fariza commented Aug 25, 2014

@OceanWolf I'm back, did you have time to advance on the split of the PR?

@OceanWolf
Copy link
Author

No, sorry, laptop still fried. Slowly upgrading this desktop to a point where I can work on it. Hopefully only a few days more.

As I remember, when I split it, the first patch contained around 20 lines of additions, and around 80 deletions, or something like that anyway.

@fariza
Copy link
Owner
fariza commented Sep 3, 2014

@OceanWolf
I just removed Persistent Tools before going any further you have to rebase your code

fariza pushed a commit that referenced this pull request Feb 4, 2015
BUG : missing manager assignment
@fariza fariza closed this Feb 27, 2015
@OceanWolf OceanWolf deleted the navigation-toolbar-coexistence-event-framework-part2 branch July 25, 2015 20:21
fariza pushed a commit that referenced this pull request Aug 8, 2015
FIX: respect alpha on alt face color
fariza added a commit that referenced this pull request Sep 22, 2016
fariza pushed a commit that referenced this pull request Oct 21, 2016
fariza pushed a commit that referenced this pull request Dec 5, 2017
This fixes some possible heap buffer overflows, such as the following
triggered by our cmmi10.ttf:

```
ERROR: AddressSanitizer: heap-buffer-overflow on address 0x617000235709 at pc 0x7f95efd3c48a bp 0x7ffe41b6ecc0 sp 0x7ffe41b6ecb0
READ of size 1 at 0x617000235709 thread T0
    #0 0x7f95efd3c489 in utf16be_to_ascii extern/ttconv/pprdrv_tt.cpp:178
    #1 0x7f95efd3c489 in Read_name(TTFONT*) extern/ttconv/pprdrv_tt.cpp:339
    #2 0x7f95efd499ef in read_font(...) extern/ttconv/pprdrv_tt.cpp:1325
    #3 0x7f95efd4c602 in get_pdf_charprocs(...) extern/ttconv/pprdrv_tt.cpp:1420
    #4 0x7f95efd35c22 in py_get_pdf_charprocs src/_ttconv.cpp:217

0x617000235709 is located 1 bytes to the right of 648-byte region [0x617000235480,0x617000235708)
allocated by thread T0 here:
    #0 0x7f9612262a38 in __interceptor_calloc (/usr/lib64/libasan.so.4+0xdea38)
    #1 0x7f95efd3b261 in GetTable(TTFONT*, char const*) extern/ttconv/pprdrv_tt.cpp:140
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0