deb0ch / emacs-winum

Window numbers for Emacs - Navigate windows and frames using numbers
141 stars 13 forks source link

Rerun initialization when scope changes at runtime. #7

Closed Alexander-Miller closed 7 years ago

Alexander-Miller commented 7 years ago

That's the best simple solution I could come up with. Imo it's either something like this or some major rewrites wrt accessing all those internal tables and vectors.

deb0ch commented 7 years ago

That's a very good one, thank you !

I think this is the way to go, I can't think of a better solution.

I just wonder if these calls are made in the best possible places. I'm sure the one in winum-select-window-by-number isn't needed, and I wonder if the ones in winum-get-number and winum-get-window-by-number wouldn't be better in the getters and setters or if where they are now is already the best (this is a real question, I actually don't know where they would be best).

Well, waiting for your feedback about the call in winum-update and wether we should put the other calls in the getters/setters or not.

PS: there are two FIXMEs just under the file header, I think that you can take the first one down in your commit :smile_cat:

Alexander-Miller commented 7 years ago

New version. Check is now called in the accessor methods. That 1) saves us a few calls, especially if more functions are added, and 2) removes the confusion about not knowing if we really need this here, because this also calls that and so on.

Alexander-Miller commented 7 years ago

Removed the fixme, too. How do I tell github that I did the requested changes? Or is it something you need to ok on your end?

Alexander-Miller commented 7 years ago

At any rate the change is quite simple now and everything's going well in my testing so far. I think this is the way to go.

deb0ch commented 7 years ago

All good to me ! Thank you very much, this is was a very nice PR :thumbsup::sparkles:

deb0ch commented 7 years ago

And yes, on Github interface it is me who is supposed to click "approve changes" for the changes to be approved