Skip to content
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

fix(gtk): select cloned item #604

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

nevarek
Copy link

@nevarek nevarek commented Aug 31, 2021

No description provided.

@@ -142,7 +143,7 @@ Bug fixes
- Both QT and GTK versions will reload hotkeys after a keymap change event.
- Fix locking issue
- Expose Alt_GR as a hotkey modifier on GTK.
- (GTK) Fixed GUI lock-up, if multiple script error notifications are posted in quick succession. The notifications are now rate-limited and won’t post more than one notification per second. Fixes issue #383
- (GTK) Fixed GUI lock-up, if multiple script error notifications are posted in quick succession. The notifications are now rate-limited and won’t post more than one notification per second. Fixes issue #383
Copy link
Collaborator

Choose a reason for hiding this comment

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

Whitespace change. Ideally we wouldn't have trailing whitespace in the first place, but it is usually considered annoying to open PRs that change whitespace in areas that aren't related to your change.

(Only a small PR so doesn't make a big difference in this case, but keep it in mind. Editor tools sometimes do this automatically, so review your diffs)

Copy link
Author

Choose a reason for hiding this comment

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

Ah dang it, this must have been introduced in the rebase. I can remove the change, so I will.

@BlueDrink9
Copy link
Collaborator

BlueDrink9 commented Sep 1, 2021

Thank you so much for the PR!
This looks fine to merge, but first could you please reword the changelog entry. It is unclear from looking at it what the old behaviour was that you are fixing, and what it has been changed to. It is useful to reference the issue number (#585) in the changelog.
Also, please use a (GTK) tag at the start to make it clear this only applies to one GUI.

Also, if you edit the PR description to add "fixes #585", when it gets merged it will automatically close the relevant issue on github!

@nevarek
Copy link
Author

nevarek commented Sep 2, 2021

Thanks for explaining the commit workflow, I'll be sure to make these updates!

@josephj11
Copy link
Contributor

While this stands on its own as a valid enhancement/fix, does the Qt front end also need attention?

@nevarek Welcome to our dev community! We really appreciate your contributions.

@BlueDrink9
Copy link
Collaborator

Looks like this isn't an issue in qt, just gtk

@josephj11
Copy link
Contributor

@nevarek Any reason this didn't get finished? It appears to be a valuable fix that just needs cosmetic changes.

@nevarek
Copy link
Author

nevarek commented Aug 12, 2023

@nevarek Any reason this didn't get finished? It appears to be a valuable fix that just needs cosmetic changes.

I don't recall, sorry.

@josephj11
Copy link
Contributor

@nevarek From your response, I assume you are no longer interested in completing this bug fix. I am going to ask another developer to take a look at this if you don't want to do it.

Thanks again for this PR. (Please don't delete it because we haven't copied it yet, etc.)

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.

None yet

3 participants