Closed cheywood closed 2 years ago
Hey Chris,
Thank you very much for this PR. Your changes look already very good and nicely polished.
gdk_event_triggers_context_menu
would be nice. Presumably, my GTK debugging skills are much worse than yours because I haven't used it for anything productive for a while._replace_word
Looks good!It would be great if you could address/respond to my comments. Afterwards, we should be good to go!
Thanks Maximilian, that all sounds good. I think I've made all the changes you raised. All your reasoning in the comment below makes sense to me too :+1: I think that's sensible for the major version bump with the considerable changes to supported Python and GTK versions, plus it will help with the minor API differences.
Perhaps there are remaining documentation changes to make? It would be good to note that importing the GTK version you want to use before importing the module will result in the GTK version selection. Perhaps you already have that noted somewhere for the old detection? I haven't checked.
Once we get this in I can work on the other two smaller PRs.
Perfect! 👍
Yes, indeed some minor changes to the documentation (doc/source/index.rst
) are necessary. However, this is also something I can do after merging this PR. If you want, you can also do the changes yourself. Just let me know, otherwise, I am ready to merge this PR.
I'm happy to leave that in your capable hands if you have the time :slightly_smiling_face:
I'll squash the commits pre-merge?
I think I am able to somehow squeeze this into my schedule.
You do not need to squash the commits—GitHub will take care of that when merging.
Thanks again for making this happen. 🙏
My pleasure!
You do not need to squash the commits—GitHub will take care of that when merging.
That it does. I hadn't used the GitHub mechanism for it before. FWIW there's a tiny downside in that the author name also gets squashed (to GitHub username).
Oh, sorry, I did not know that this would happen. I hope this is not too much of an issue.
I wonder whether this would also happen if you had your name associated with your GitHub account?
Anyway, we can use another procedure next time. :)
Oh, sorry, I did not know that this would happen. I hope this is not too much of an issue.
No, not a big issue
I wonder whether this would also happen if you had your name associated with your GitHub account?
Oh huh, good point, I didn't know this wasn't set. You're probably onto something there :slightly_smiling_face:
Hey Maximilian,
I've gotten around to massaging my GTK 4 changes in with some GTK 3 support for this first draft changeset.
As planned this drops Python 2 and GTK 2 support.
Mixed notes:
GestureClick
usage to repopulate suggestions in the menu (and update the current word mark)Ignore All
toIgnore
andAdd <word> to Dictionary
toAdd to Dictionary
. I still like this simpler wording (which follows the interaction set in one of the first GTK4 apps to have spell checking - Text Editor), but presumed you'd rather not change it."(no suggestions)"
menu item isn't added. The later is at least in part due to the fact that it's not possible to do italic formatting on in the menu with GTK 4, and again, copies what was in gnome-text-editor. All up for discussion.GObject
but I believe that's necessary for thePropertyAction
, which provides the language selection menu its state behaviourGestureClick
is occurring on a popup-generating button usinggdk_event_triggers_context_menu
but haven't yet been able to get a hold on the sequence or yet determine the root cause of why I can't (sorry, time lack). At the moment it's matching to button 3, which might be fine for now, but there's at least a TODO to remove. Perhaps you have some better GTK input handling debugging skills and might be able to solve this :slightly_smiling_face:_replace_word
has been changed to build the old word from the mark instead of passing it in (which works better with the GTK 4 action-based interaction). Do you see any issue with this?Hopefully a reasonable starting point 😀