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

macOS: Fix double-click the title bar to maximize the window does not occupy the full screen and others #6055

Merged
merged 2 commits into from
Feb 24, 2023

Conversation

page-down
Copy link
Contributor

  • Fix resize_in_steps being applied when double-clicking on the title bar to maximize the window.
  • Add comments to explain how to get the services related methods to be called and restore IME pre-edit text after inserting text from the service.

I've found one more annoying issue.

Using launch --dont-take-focus (--keep-focus) to create a new window, the on_focus callback will still be called. And there are four of these, the current window's as well as the new window, in and out.

And the side effect is not only this, it also causes IME composition to be cancelled.

I think any operation that creates, removes, changes the active window of an inactive tab, etc. should not trigger on_focus_changed as long as it does not actually change the current focus.

If you don't have time to debug this, where do you think is the best place to fix it?

I found that the KITTY_VCS_REV exists and is empty in the recent debug_config repots, which leads to kitty 0.27.1 () ... a parenthesis with no content.
The related code has not been changed recently and the issue does not exist in the official release 0.26.5, is there a change in the build environment?

@kovidgoyal kovidgoyal merged commit 7ab0c30 into kovidgoyal:master Feb 24, 2023
@page-down page-down deleted the fix-macos branch February 24, 2023 12:03
@kovidgoyal
Copy link
Owner

kovidgoyal commented Feb 24, 2023 via email

@kovidgoyal
Copy link
Owner

the VCS rev is showing up in the debug window for me.

@page-down
Copy link
Contributor Author

the VCS rev is showing up ... for me.

I downloaded the nightly release built from c5149de, ran debug_config and saw kitty 0.27.0 () created by ... .
The version number should be 0.27.1 (better yet, the unreleased 0.28.0, but I don't care) and the VCS REV should not be empty (or without empty parentheses).
And some of the recent bug reports have had this problem.

This can be fixed by a simple check, hasattr(fast_data_types, 'KITTY_VCS_REV') is not enough, but I wanted to let you know first if some inspection is needed in the build environment.

Oh, I saw that macOS nightly is already 3 weeks old, and did not upload the latest nightly version (or build failed), no wonder it is 0.27.0.
I tried the latest linux nightly again, still no VCS Rev.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Feb 24, 2023 via email

@page-down
Copy link
Contributor Author

I tried the latest linux nightly ...

It turns out that the latest linux amd64 nightly is a week old, which is inconsistent with the others.

... which is rather a lot of effort ...

Yes, while nightly with VCS rev would be useful for identifying problems, I have seen very few reports on nightly versions so far.

... need to add another parameter to the window creation functions ...

I think I found the root cause.
In the previous discussion of kitty @ ls, we already discussed the definition of active and focus.
There is only one kitty window with keyboard focus, i.e. the tab it belongs to is the active tab and the OS window it is in is the currently focused window.
The on_focus event should only be fired on the correct window if this condition changes.

However, currently focus_changed is called in notify_on_active_window_change. This causes an active window change in an inactive tab or OS window to also raise the on_focus event.
I don't think we should mix terms here. If the window active changes it should be the on_active event.

It's not a serious problem, it's just that this big state machine is hard to manage, I just want kitty to not go into the wrong state and trigger unwanted events.
Have revised more than 20 places still not satisfied, please let me know if there is anything else to consider.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Feb 24, 2023 via email

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