r32 prints `^[[I` or `I` in libvte-based terminals

[fguada]

Hello, and thank you very much for this gem.

I noticed yesterday a quirk that didn't happen in versions preceding r32. Only on libvte-based terminals, and only in subshells, lf prints either ^[[I or just I.

Tested positively on several Debian 12 machines with Xfce4-terminal, lxterminal, Gnome Terminal and Terminator. Doesn't happen in kitty, Konsole, Xterm, st.

Reproduced with this simple lfrc:

cmd one $lf -remote "send $id two"
cmd two $fzf

Executing :one in lf cli results in fzf's query to be I. If you replace fzf with for instance micro (text editor), [I] will be inserted in the opened buffer. In either case, and also if you replace fzf with any gui app, lf will leave ^[[I printed in terminal after you quit it. That doesn't happen if you execute :two directly.

(I use a heavily customized lfrc, where a cmd opens fzf with a kind of "command palette" from which I run some scripts that also use fzf. It's in these secondary instances of fzf that I noticed this quirk.)

[joelim-work]

Thanks for reporting.

I did a bisect, this is caused by #965, where terminal focus event reporting is enabled. This means that terminals will send ^[[I when gaining focus. The problem is that the terminals mentioned above also send this sequence when resuming the UI following a $ shell command (subsequently appears in the two command), while other terminals don't.

I don't know if this is something that can be solved in the tcell library. If not then we may have to consider adding an option to control whether focus event reporting is enabled or not (possibly not all users are interested in this): https://github.com/gokcehan/lf/blob/d26eb0108a63ec2e23b85e2060203e5abc27311d/client.go#L39

Otherwise for now you can try avoiding the use of $ commands if you don't need to run synchronously:

cmd one &lf -remote "send $id two"
cmd two $fzf

cc @gokcehan

[joelim-work]

Maybe a quick fix is to just enable focus reporting only when the hidecursorinactive option is set:

diff --git a/client.go b/client.go
index db60df4..559102d 100644
--- a/client.go
+++ b/client.go
@@ -36,7 +36,6 @@ func run() {
    if gOpts.mouse {
        screen.EnableMouse()
    }
-   screen.EnableFocus()

    if gLogPath != "" {
        f, err := os.OpenFile(gLogPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600)
diff --git a/eval.go b/eval.go
index bdb7ff8..be3b1c2 100644
--- a/eval.go
+++ b/eval.go
@@ -114,6 +114,13 @@ func (e *setExpr) eval(app *app, args []string) {
        }
    case "hidecursorinactive", "nohidecursorinactive", "hidecursorinactive!":
        err = applyBoolOpt(&gOpts.hidecursorinactive, e)
+       if err == nil {
+           if gOpts.hidecursorinactive {
+               app.ui.screen.EnableFocus()
+           } else {
+               app.ui.screen.DisableFocus()
+           }
+       }
    case "history", "nohistory", "history!":
        err = applyBoolOpt(&gOpts.history, e)
    case "icons", "noicons", "icons!":

[gokcehan]

Thank you very much @fguada for reporting and @joelim-work for the investigation and solutions. I played around with it for a while and it seems like an annoying issue as it happens even though the user does not use the focus event feature themselves and also there is no way around it.

@joelim-work Your second approach to enable focus only when using hidecursorinactive sounds reasonable to me as we can simply get rid of it later if this is fixed in tcell somehow at some point.

I'm still trying to evaluate whether this is an issue that requires a follow-up bug fix release before we start merging new features. #965 is relatively old and we had no complaints up until now, so this does not seem a common use case among our users, but it effects all vte terminals and makes it unusable for users who have such custom commands. In any case, I think it is better to wait until the weekend in case other issues might be reported even if we decide to make a bug fix release. Let me know what you think.

[fguada]

Thank you for the quick inquiry and answer.

I also favor the second solution. If focus event reporting is only used by, and mandatory to, hidecursorinactive, having a separate option for it seems useless and misleading.

A bug fix commit is fine for me, as I can run master instead of a release. I've no idea though how many of your users do use a libvte-based terminal. Such terminals are the default choice on most popular Linux distros.

[joelim-work]

I have submitted this patch as a PR #1676, hopefully this works for you. It is not a 100% complete fix, since there's still an issue for users who wish to use hidecursorinactive, but I think it is quite niche.

Regarding the release schedule, I don't think this needs to be released urgently and can wait one or two weeks.