enable & handle terminal focus events #861

laktak · 2022-06-29T22:19:19Z

When using lf instances in multiple panes in the same window in tmux it is hard to tell which instance has the focus. The same is true when you have multiple terminal panes side by side.

This PR allows lf to receive focus events and turns the highlight for the current selection off when lf loses focus. This means only the active lf instance will show the highlight.

Tested with tmux and kitty on Linux/macos.

laktak · 2022-06-30T12:15:56Z

Also tested on Windows, of course it does not work there but just to make sure it has no side effects.

ilyagr · 2022-08-29T06:02:09Z

ui.go

+		if i == dir.pos && gOpts.hasfocus {
 			st = st.Reverse(true)
 		}


Thank you for making this! I'm using it, and it's nice.

However, I would personally prefer something like the following here:

if i == dir.pos { st = st.Reverse(true) if !gOpts.hasfocus { st = st.Foreground(tcell.ColorDarkGray) } }

I think the grey color looks nicer than a complete absence of a selection. Of course, in the ideal world the color would be configurable; I'm not sure if this can be done through the LS_COLORS variable or somehow else.

(To be clear, I have no official review powers here).

I switched to tcell.ColorGray. It should support more terminal and light themes better.

I implemented it this way to behave like a cursor in tmux - there is only one active cursor, no dimming.

So if you prefer dimming I'd like your PR to have options ;-)

I see. Here I was thinking dimming was strictly superior. Do you think dim cursors in unfocused windows are distracting? Do you not like them in general?

I was hoping to avoid an explosion of options, not sure what the best way is. One possibility is to have an on/off focusevents option and a dimcursor option with possible values gray, hide, and same that would govern what the "dimmed" cursor looks like both when focus is lost and for the directory preview (#924.).

Do you think dim cursors in unfocused windows are distracting?

Yes, too much noise IMO.

ilyagr · 2022-08-29T06:04:02Z

ui.go

+			if i == dir.pos && gOpts.hasfocus {
 				win.print(screen, lnwidth+1, i, st.Reverse(true), tag)
 			} else {


I think this can be simpler. st is already the style we want, it already has Revese(true) set.

if i == dir.pos { win.print(screen, lnwidth+1, i, st, tag) } else {

That makes it work with my suggestion above. (I may or may not get around to making these suggestions into a pull request on top of your pull request.

Here's a diff with these changes + a related feature: ilyagr/lf@13ff430...2673fc7

I now made the "related feature" into a proper pull request: #924. To make it work with focus events, the only thing that needs to change to merge with your PR is that dim_cursor: false in line 882 of ui.go (link below) has to change to dim_cursor: !gOpts.hasfocus.

https://github.com/gokcehan/lf/pull/924/files#diff-bcaef4efbe83b371a9c05e40ca6d2d0d27b63e0c9fc169993c9959397f9890ceR882

@laktak

When preview is on and consists of a directory listing, make the cursor in there dimmer to emphasize it's not active. This makes it visually obvious which cursor is active at all times, without having to read the path. The active cursor is now always the rightmost bright one, regardless of whether the `preview` option is on or off. See screenshots in the pull request. Before this, I noticed that I often open a file by accident because I got confused which of the three cursors on the screen is the active one. In the future, I also hope this dimming could be used to show when an lf window loses focus, based on @laktak's implementation of focus tracking in gokcehan#861.

@laktak

When preview is on and consists of a directory listing, make the cursor in there dimmer to emphasize it's not active. This makes it visually obvious which cursor is active at all times, without having to read the path. The active cursor is now always the rightmost bright one, regardless of whether the `preview` option is on or off. See screenshots in the pull request at gokcehan#924. Before this, I noticed that I often open a file by accident because I got confused which of the three cursors on the screen is the active one. In the future, I also hope this dimming could be used to show when an lf window loses focus, based on @laktak's implementation of focus tracking in gokcehan#861.

ilyagr · 2022-08-30T08:54:18Z

client.go

 	app.loop()

+	// disable focus reporting
+	fmt.Print("\x1b[?1004l")


This is actually a little buggy. Let's say you run lf inside another lf. After you exit the inner lf, the outer lf no longer receives focus events.

Also, if you use lf to launch a program that gets confused by focus events, that program will get focus events and be confused.

To solve this, every time lf launches a program that might have access to the terminal (for instance $ and ! but not %), it should first disable focus reporting. After that program finishes, lf should again enable focus reporting.

Thanks, that's a good input. focus events should be disabled in that case. I'll take a look when I find the time.

You should also consider using Go's defer feature to attempt to disable focus events in cases when lf is interrupted or something panics. I'm 80% sure it's the right thing to do, would need to do a bit more research to be sure.

@laktak

When preview is on and consists of a directory listing, make the cursor in there dimmer to emphasize it's not active. This makes it visually obvious which cursor is active at all times, without having to read the path. The active cursor is now always the rightmost bright one, regardless of whether the `preview` option is on or off. See screenshots in the pull request at gokcehan#924. Before this, I noticed that I often open a file by accident because I got confused which of the three cursors on the screen is the active one. In the future, I also hope this dimming could be used to show when an lf window loses focus, based on @laktak's implementation of focus tracking in gokcehan#861.

ilyagr · 2022-09-10T04:23:56Z

I discovered another minor bug with this functionality. It doesn't properly handle focus events when in prompt mode.

To reproduce, open lf, press :, and make lf lose focus and gain it back again. When lf loses focus, the letter O gets added to the command prompt. When lf gains focus, I gets added to the prompt.

UPDATE: This is actually more difficult to fix than it appears. It looks like cmap doesn't currently support multi-key bindings.

laktak · 2022-09-13T15:21:42Z

@ilyagr

I discovered another minor bug with this functionality. It doesn't properly handle focus events when in prompt mode.

I've fixed this. cmap now handles multi-key bindings but only for <ESC>[ (didn't have more time and I am also not sure if there is a use case).

Also the focus events are disabled when shelling out.

Thanks and let me know if you find anything else.

ilyagr · 2022-09-14T02:13:09Z

Thanks, this looks nice! When I get a moment, I'll switch my personal lf to use this version of this PR and perhaps look at your code more carefully. At a glance, it looks great.

cmap now handles multi-key bindings but only for [ (didn't have more time and I am also not sure if there is a use case).

I think that's fine.

BTW, I made a feature request for tcell in gdamore/tcell#558. It looks like they want to implement it. If that happens, we'll be able to make this functionality less hacky. Until then, I see no problem with using this version.

@laktak

This mainly merges gokcehan#924 with @laktak's gokcehan#861. This also merges in the `:keys` command (gokcehan#918) as well as and some unrelated minor corrections (currently dd5dc59).

@laktak

This mainly merges gokcehan#924 with @laktak's gokcehan#861. This also merges in the `:keys` command (gokcehan#918) as well as and some unrelated minor corrections (currently dd5dc59).

gokcehan · 2022-10-15T12:47:22Z

@laktak @ilyagr I don't think it is a good idea to hard code terminal escape codes on lf side. Feel free to submit another patch again if this is supported on tcell.

laktak · 2022-10-15T20:19:32Z

@gokcehan You are right. I'll take a look at a better solution.

laktak mentioned this pull request Jul 4, 2022

Add on-select event #864

Merged

ilyagr reviewed Aug 29, 2022

View reviewed changes

ilyagr mentioned this pull request Aug 30, 2022

For directories in preview column, dim cursor #924

Merged

ilyagr reviewed Aug 30, 2022

View reviewed changes

enable & handle terminal focus events

68370c4

laktak force-pushed the focus-event branch from ed06bab to 7cb2ebb Compare September 13, 2022 15:10

handle focus events for prompts and suspend for shell

abba0c2

laktak force-pushed the focus-event branch from 7cb2ebb to abba0c2 Compare September 13, 2022 15:19

ilyagr mentioned this pull request Sep 20, 2022

Dim unfocused or preview cursor + extras. #941

Closed

gokcehan closed this Oct 15, 2022

laktak mentioned this pull request Oct 15, 2022

Add hidecursorinactive option to indicate status when terminal loses focus #965

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enable & handle terminal focus events #861

enable & handle terminal focus events #861

laktak commented Jun 29, 2022

laktak commented Jun 30, 2022

ilyagr Aug 29, 2022 •

edited

Loading

ilyagr Aug 29, 2022

ilyagr Aug 30, 2022

laktak Aug 30, 2022

ilyagr Aug 31, 2022

laktak Sep 1, 2022 •

edited

Loading

ilyagr Aug 29, 2022

ilyagr Aug 29, 2022

ilyagr Aug 30, 2022 •

edited

Loading

ilyagr Aug 30, 2022 •

edited

Loading

laktak Aug 30, 2022

ilyagr Aug 31, 2022 •

edited

Loading

ilyagr commented Sep 10, 2022 •

edited

Loading

laktak commented Sep 13, 2022

ilyagr commented Sep 14, 2022 •

edited

Loading

gokcehan commented Oct 15, 2022

laktak commented Oct 15, 2022

enable & handle terminal focus events #861

enable & handle terminal focus events #861

Conversation

laktak commented Jun 29, 2022

laktak commented Jun 30, 2022

ilyagr Aug 29, 2022 • edited Loading

Choose a reason for hiding this comment

ilyagr Aug 29, 2022

Choose a reason for hiding this comment

ilyagr Aug 30, 2022

Choose a reason for hiding this comment

laktak Aug 30, 2022

Choose a reason for hiding this comment

ilyagr Aug 31, 2022

Choose a reason for hiding this comment

laktak Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

ilyagr Aug 29, 2022

Choose a reason for hiding this comment

ilyagr Aug 29, 2022

Choose a reason for hiding this comment

ilyagr Aug 30, 2022 • edited Loading

Choose a reason for hiding this comment

ilyagr Aug 30, 2022 • edited Loading

Choose a reason for hiding this comment

laktak Aug 30, 2022

Choose a reason for hiding this comment

ilyagr Aug 31, 2022 • edited Loading

Choose a reason for hiding this comment

ilyagr commented Sep 10, 2022 • edited Loading

laktak commented Sep 13, 2022

ilyagr commented Sep 14, 2022 • edited Loading

gokcehan commented Oct 15, 2022

laktak commented Oct 15, 2022

ilyagr Aug 29, 2022 •

edited

Loading

laktak Sep 1, 2022 •

edited

Loading

ilyagr Aug 30, 2022 •

edited

Loading

ilyagr Aug 30, 2022 •

edited

Loading

ilyagr Aug 31, 2022 •

edited

Loading

ilyagr commented Sep 10, 2022 •

edited

Loading

ilyagr commented Sep 14, 2022 •

edited

Loading