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

Attempt to fix unfocus. #6

Closed
wants to merge 1 commit into from
Closed

Conversation

iago-lito
Copy link
Contributor

Here is an attempt to fix the focus problem introduced by #3. FWIU, calls to inhibit_unfocus() and uninhibit_unfocus() need to be exactly matched (?). This modification takes care of that. Maybe this'll solve the problem?

/!\ WIP I haven't actually tested it yet. I'll test it asap.

@iago-lito
Copy link
Contributor Author

Well, this does not seem to work @elcste. I'm not sure I understand why it's broken or what to do to fix it then :(

@elcste
Copy link
Owner

elcste commented Sep 20, 2024

I tried it too and it also didn't work for me :-/ My original extension was already at the edge of my js abilities, so I don't have anything to add.

I want to consider it more, but right now I am used to the original hiding behavior and much prefer with the focus preserved. (Not to mention the original code is much simpler, which means less likely to break with GNOME updates.) So I would like revert back to my original version and change the description not to claim the timeout is 5 seconds ;-) If you prefer your version with the fixed timeout, I'd suggest you submit your fork to EGO. I would add a link to your version for those who want that trade-off. I looked like incorporating PR #2 to add a timeout time preference wouldn't be too hard as well. Then we end up with two version people could choose from: my original, simpler version and a more complicated one with better timeout behavior but the caveat with focus.

@iago-lito
Copy link
Contributor Author

iago-lito commented Sep 23, 2024

Considering that I have been unable to preserve the original behaviour wrt focus, I'm very okay with the reversion.
I wish I could read more about this inhibit_unfocus logic, but damn, the dev-documentation for gnome really are cryptic :(

I don't think it is a good idea to submit my fork to EGO though, as it is an obvious way towards confusion for everyone. Instead, I can offer to maintain an 'official branch' timer here, and commit to keeping it in sync by rebasing atop your main whenever it updates. The README would explain the situation and offer to git clone --branch timer https://github.com/elcste/hide-cursor ~/.local/share/gnome-shell/extensions to get the alternative version.

This avoids duplicating the extension and having to invent/deal with variations around the name hide-cursor*. Also, it makes it very straightforward to re-unite both versions into one the day we eventually figure how to fix focus. What do you think? :)

@elcste
Copy link
Owner

elcste commented Sep 23, 2024

That sounds good.

I'm a fairly beginning git user so maybe you can heklp. Do you see any problem with me just creating a new commit to go back to my version? And then I can create the new branch and merge your PR in that.

@iago-lito
Copy link
Contributor Author

Uh, nope, this is not the usual way to proceed, and it'd duplicate the version reverted ^ ^" The best option here might be to rebase the main branch (note that this is rarely the best option), to get something like:

* xxxxxxx (timer) 🚧 Attempt to fix focus inhibition by pairing calls.
* xxxxxxx Add note about timer improvement to README.md
* xxxxxxx Correct timer behavior to start when the mouse is idle
* c15c743 (tag: v.1.3.0, main) Update for GNOME 47
* 39ccafb Add GNOME 46 compat note
...

(also I've just noticed the v2.0.0 tag. Is this really what you meant wrt semver?

I do know enough of git shenanigans to help you with this, but I'd need push access to this repo + permission to force-push on the main branch. Alternately, I can craft a mirror of all required commits/tags/branchs on my fork and then help you click the right buttons in order to import them here. You tell me :)

@elcste
Copy link
Owner

elcste commented Sep 25, 2024

I decided to do what I could personally easily understand, even if it was not optimal.

To do as you suggested above, you can create a PR and then I can create a new branch timer and merge the PR to that? Or am I missing something (likely ;-) )?

@iago-lito
Copy link
Contributor Author

iago-lito commented Sep 26, 2024

Sure, I understand that you prefer not to rebase. Do whatever you're confident with, then I'll rebase my timer fix on top of that and PR it against branch timer :)

@elcste
Copy link
Owner

elcste commented Sep 26, 2024

Voilà: https://github.com/elcste/hide-cursor/tree/timer

@iago-lito iago-lito closed this Oct 2, 2024
@iago-lito iago-lito deleted the unfocus branch October 2, 2024 18:56
@iago-lito iago-lito mentioned this pull request Oct 2, 2024
Merged
@iago-lito
Copy link
Contributor Author

There you go #7 ;)

@iago-lito
Copy link
Contributor Author

iago-lito commented Oct 2, 2024

(reopening after accidental deletion of unfocus branch) Arh, it seems I can't reopen, can you @elcste?

@elcste
Copy link
Owner

elcste commented Oct 2, 2024

@iago-lito The Reopen button has is disabled with the message The unfocus brand was force-pushed or recreated. :-(

elcste added a commit that referenced this pull request Oct 2, 2024
This fixes the timer as per #3, but in a way that breaks the unfocus inhibition as explained in #6. iago-lito will keep this branch up-to-date for people willing to enjoy correct timing and are okay with loosing focus... until someone finds a way to fix unfocus inhibition.
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