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.

Sign up for GitHub

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

Jump to bottom

Terminal + Neovim: FocusLost/Gained events don't fire. #11682

Closed
mikatpt opened this issue Nov 4, 2021 · 4 comments · Fixed by #12900
Closed

Terminal + Neovim: FocusLost/Gained events don't fire. #11682

mikatpt opened this issue Nov 4, 2021 · 4 comments · Fixed by #12900
Assignees
@zadjii-msft
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Area-VT Virtual Terminal sequence support Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase Product-Conpty For console issues specifically related to conpty Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v1.14

Comments

@mikatpt
Copy link

mikatpt commented Nov 4, 2021 •
edited
Loading

Windows Terminal version (or Windows build number)

1.12.2931.0

Other Software

Neovim 0.5+

Steps to reproduce

Steps to reproduce:

  1. Run Neovim 0.5 or higher using the following min.vim file, with the command nvim -u min.vim --clean
" min.vim
autocmd FocusLost * echo "Focus Lost"
autocmd FocusGained * echo "Focus Gained"
  1. Click away from terminal. Click to.
  2. Vim command line output at the bottom of the screen should update with the echoed text.

Expected Behavior

See title and steps to reproduce. This works in all Linux/MacOS terminals I've used, including Alacritty, Kitty, and iTerm2, as well as in tmux sessions if focus-events are enabled.

The Vim/Neovim release is not particularly important unless you would also like to test in tmux; in that case, choose a release any time after April 2021 and put set -g focus-events on in your .tmux.conf

Actual Behavior

Nothing happens.
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Nov 4, 2021
@j4james
Copy link
Collaborator

j4james commented Nov 4, 2021 •
edited
Loading

I suspect this is because we don't support XTerm's focus event mode (private mode 1004). I thought there might have been an issue for it already, but I haven't been able find one.

Edit: Not a duplicate as such, but I see now it has been discussed before here: #10531 (comment)

@zadjii-msft
Copy link
Member

Well cool, I would have suspected there was an issue already for this. Plumbing through conpty shouldn't be too hard, if a bit annoying.

@zadjii-msft zadjii-msft added Area-Input Related to input processing (key presses, mouse, etc.) Area-VT Virtual Terminal sequence support Product-Conhost For issues in the Console codebase Product-Conpty For console issues specifically related to conpty Product-Terminal The new Windows Terminal. labels Nov 4, 2021
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Nov 4, 2021
@zadjii-msft zadjii-msft added the Help Wanted We encourage anyone to jump in on these. label Nov 4, 2021
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Nov 17, 2021
@zadjii-msft zadjii-msft modified the milestones: Terminal Backlog, Backlog Jan 4, 2022
@zadjii-msft zadjii-msft mentioned this issue Mar 31, 2022
Merged
3 tasks
@zadjii-msft
Copy link
Member

zadjii-msft commented Apr 7, 2022 •
edited
Loading

Oh boy I sure did go overboard on my branch off #12799. We may just have to do this whole thing, so, here it is:

gh-11682-works

(left is before, right is after)

note to self: ~/nvim.appimage -u ~/gh-11682.vim ~/gh-11682.vim to do the thing

dev/migrie/b/2988-focus-fg-with-owner...dev/migrie/f/11682-vt-focus-events

@zadjii-msft zadjii-msft modified the milestones: Backlog, Terminal v1.14 Apr 7, 2022
@zadjii-msft zadjii-msft added the Issue-Task It's a feature request, but it doesn't really need a major design. label Apr 7, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Apr 7, 2022
@zadjii-msft zadjii-msft self-assigned this Apr 7, 2022
@zadjii-msft zadjii-msft removed the Help Wanted We encourage anyone to jump in on these. label Apr 7, 2022
@zadjii-msft zadjii-msft mentioned this issue Apr 13, 2022
Merged
3 tasks
@ghost ghost added the In-PR This issue has a related PR label Apr 13, 2022
zadjii-msft added a commit that referenced this issue Apr 19, 2022
@zadjii-msft
Allow windows created by console apps to appear above the Terminal (#…
a496af3 
…12799)

## Window shenanigans, part the third:

Hooks the Terminal's focus state up to the underlying ConPTY. This is LOAD BEARING for allowing windows created by console applications to bring themselves to the foreground.

We're using the [FocusIn/FocusOut](https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h3-FocusIn_FocusOut) sequences to communicate to ConPTY when a control gains/loses focus. Theoretically, other terminals could do this as well.

## References

#11682 tracks _real_ support for this sequence in Console & conpty. When we do that, we should consider even if a client application disables this mode, the Terminal & conpty should always request this from the hosting terminal (and just ignore internally to ConPTY).

See also #12515, #12526, which are the other two parts of this effort. This was tested with all three merged together, and they worked beautifully for all our scenarios. They are kept separate for ease of review.

## PR Checklist
* [x] This is prototype 3 for #2988
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated

## Detailed Description of the Pull Request / Additional comments

This allows windows spawned by console processes to bring themselves to the foreground _when the console is focused_. (Historically, this is also called in the WndProc, when focus changes).

Notably, before this, ConPTY was _never_ focused, so windows could never bring themselves to the foreground when run from a ConPTY console. We're not blanket granting the SetForeground right to all console apps when run in ConPTY. It's the responsibility of the hosting terminal emulator to always tell ConPTY when a particular instance is focused.

## Validation Steps Performed

(gif below)
@ghost ghost closed this as completed in #12900 Apr 20, 2022
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Apr 20, 2022
ghost pushed a commit that referenced this issue Apr 20, 2022
@zadjii-msft
Plumb Focus events through VT Input (#12900)
87f5034 
Further builds on #12799. #12799 assumes that the connection is prepared to receive FocusIn/FocusOut events as input. For ConPTY we can be relatively sure of that, but that's not _technically_ correct. In the hypothetical world where the connection is not a ConPTY connection, then the other side might not be expecting those sequences. 

This remedies the issue by
* ConPTY will always request focus event mode (from the terminal) when it starts up
* when a client tries to disable focus events in conpty, conpty is gonna note that internally, but never transmit that to the hosting terminal, to leave the terminal in focus event mode.
* `TerminalDispatch` and `ControlCore` are hooked up now to only send focus events when the Terminal is in focus event mode (which will be always for conpty)
* At this point, it was like, 4LOC in `terminalInput.cpp` to add support for focus events to conhost as well.

## checklist
* [x] closes #11682
  * This combined with #12515 will finally close out #2988 as well, but we can do that manually.
* [x] I work here
* [ ] There aren't tests for this. There probably should be.
@ghost
Copy link

ghost commented May 24, 2022

🎉This issue was addressed in #12900, which has now been successfully released as Windows Terminal Preview v1.14.143.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zadjii-msft zadjii-msft

Labels
Area-Input Related to input processing (key presses, mouse, etc.) Area-VT Virtual Terminal sequence support Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase Product-Conpty For console issues specifically related to conpty Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v1.14
Development

Successfully merging a pull request may close this issue.

Plumb Focus events through VT Input microsoft/terminal
4 participants
@DHowett @j4james @zadjii-msft @mikatpt