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

Emit focus events #689

Merged
merged 1 commit into from Jul 28, 2022
Merged

Conversation

groves
Copy link
Contributor

@groves groves commented Jul 22, 2022

I'd like to receive focus change events from crossterm to let me save on loss of focus in Helix.

This adds focus emitting on Unix, though I still need to add documentation and tests.

I wanted to open a PR now as I'm not clear on the best way to support this on Windows.

Option 1: Windows Terminal added these Unix-style focus events in 1.14.186. That came out a couple weeks ago. I think we'd be able to get those codes since we're enabling virtual terminal processing in Windows, but I don't know enough about the Windows terminal to be sure. I don't see us parsing ansi codes anywhere else in the Windows code, so that makes me worry it's not the case. It would also restrict this to really recent Windows terminal versions.

Option 2: Listen for the FOCUS_EVENT_RECORD in crossterm-winapi. That would be hacky as that focus record is supposed to be internal. It looks like that's what they're using in Windows Terminal to emit the Unix-style codes, so I'm guessing we could use it.

I'm open to any other options, of course. Thoughts?

P.S. This is the first Rust code I've written, and it was super easy to drop it in and modify the example to try it. Thanks for putting together such a well structured project!

@groves groves requested a review from TimonPost as a code owner July 22, 2022 21:14
@TimonPost
Copy link
Member

This document specifies what is supported by windows: https://docs.microsoft.com/en-us/windows/console/console-virtual-terminal-sequences

@TimonPost
Copy link
Member

TimonPost commented Jul 24, 2022

It doesn't seem from the specs they support this yet. So then option two seems a good way to do this. Its quite simple to add custom logic for windows only systems by implementing this function.

Copy link
Member

@TimonPost TimonPost left a comment

Choose a reason for hiding this comment

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

Nice!

src/event.rs Outdated Show resolved Hide resolved
@groves
Copy link
Contributor Author

groves commented Jul 25, 2022

OK, option 2 implemented! Seems to work on Windows and I believe this is ready to merge. I couldn't think of a straightforward way to test an actual terminal gaining and losing focus. I only wrote a unit test as a result.

@TimonPost I've rebased it on #690 to get it to compile on Windows. If this PR looks good, you can merge this one and close 690. If this still needs work, you may want to merge 690 to fix Windows compilation in the meantime.

@groves groves changed the title [Draft] Emit focus events Emit focus events Jul 25, 2022
src/style.rs Outdated
@@ -366,6 +366,16 @@ impl Command for SetStyle {

Ok(())
}

#[cfg(windows)]
Copy link
Member

@TimonPost TimonPost Jul 27, 2022

Choose a reason for hiding this comment

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

If you rebase on main, i'll approve and merge. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@groves groves force-pushed the emit_focus_events branch 2 times, most recently from 4f232ec to 0287275 Compare July 27, 2022 20:39
@TimonPost TimonPost merged commit 069497b into crossterm-rs:master Jul 28, 2022
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

2 participants