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 foreground process shortcuts #680

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jeremypw
Copy link
Collaborator

Fixes #676

When a foreground process is current all key presses received by the MainWindow are passed straight to the terminal (MainWindow actions are therefore inhibited). As a consequence, the search entry and button are also made insensitive.

This has the drawback that MainWindow actions are inhibited whether or not the foreground uses any of the same shortcuts as MainWindow but there is no obvious way of determining whether or not a shortcut has been used by a foreground process. But as all actions can be initiated with the mouse this does not seem too much of an issue.

@jeremypw jeremypw marked this pull request as ready for review August 31, 2022 10:48
Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

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

Hm, I'm not sure about the changes to the searchbar here, especially since if you've opened the searchbar it's now stuck open until the foreground process ends. It also seems like we'll get issue reports about not being able to create or switch tabs while a command is running.

I wonder if we should expand the behavior of the current "Natural Copy/Paste" toggle to have more of a "CLI app mode" or have a per-tab option to disable all app shortcuts while the current tab is focused. Or just generally something that feels more deliberate.

@jeremypw
Copy link
Collaborator Author

OK, thanks for the review - I'll rethink it.

@jeremypw jeremypw marked this pull request as draft August 31, 2022 19:16
@jeremypw
Copy link
Collaborator Author

jeremypw commented Sep 2, 2022

It also seems like we'll get issue reports about not being able to create or switch tabs while a command is running.

You can always create a new tab or switch tabs with the mouse. If we allow the foreground process to receive all key presses first (which seems necessary to fix the issue) then there is likely to be this issue, especially if the process eats the key event.

@jeremypw
Copy link
Collaborator Author

jeremypw commented Sep 2, 2022

@danrabbit One solution may be to special case certain key combinations such as those that switch tabs or create a new tab? These would not be passed to the foreground process. Let me know which actions should be special cased and I'll try and implement it.

@jeremypw
Copy link
Collaborator Author

jeremypw commented Sep 2, 2022

It doesnt really make sense to perform a search on a tab that has a foreground process so it seems reasonable to make the searchbar insensitive when such a tab is focused?

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.

Keybinds for micro text editor do not work, making it less efficient to use the editor
2 participants