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

enhanced mouse tracking mode (1006) is set improperly when exiting tcell #512

Merged
merged 2 commits into from Mar 13, 2022

Conversation

tmatz
Copy link
Contributor

@tmatz tmatz commented Feb 9, 2022

Fixed to properly reset enhanced mouse tracking mode (1006) when calling enableMouse(0).

related #477

Behavior before modification:

If I quit the tcell application with the mouse enabled and then start vim, the cursor doesn't move when I click the mouse.

The symptom was confirmed by _demos/mouse.go in the linux environment on Chromebook and Windows.

@gdamore
Copy link
Owner

gdamore commented Feb 9, 2022

So the problem is that VIM and tcell are fighting over mouse control...

I don't think it's good not to disable mouse tracking by default, because vim aside, most people will not like the result -- you'll move the mouse and it will inject a bunch of garbage into your shell window.

What need to do is figure out if we can query the mouse state when starting up, and restore it to what it was.

Unconditionally not clearing the mouse flags is unacceptable.

(To be honest, I think the problem here is VIM's fault -- when returning from a subshell, it should reenable the mouse behavior it wants, rather than making assumptions that the child process didn't mess with it. This is done for other settings like terminfo flags, and the mouse handling needs to be treated in the precise same way.)

@gdamore gdamore self-requested a review February 9, 2022 19:17
tscreen.go Outdated Show resolved Hide resolved
@tmatz tmatz changed the title enhanced mouse tracking is not restored enhanced mouse tracking mode (1006) is set improperly when exiting tcell Feb 10, 2022
@tmatz
Copy link
Contributor Author

tmatz commented Feb 14, 2022

I found the specification of control sequnces of terminal app in chromebook.

https://chromium.googlesource.com/apps/libapps/+/master/hterm/doc/ControlSequences.md

@tmatz
Copy link
Contributor Author

tmatz commented Feb 14, 2022

@gdamore
It seems that the chromebook uses a terminal made with javascript called hterm.

I checked the specifications and implementation of hterm's control sequence. As a result, I found two things.

First, it turns out that hterm completely ignores her 1003 settings. Therefore, not only motion events but also button and drag events are no longer generated. I suspect that the fix of #477 is a hack to deal with this behavior.

Second, SGR (1006) must be reset at the finalization of tcell. If the SGR setting remains, older applications that do not use SGR (such as vim) will be notified of unexpectedly formatted mouse events and will malfunction. In other words, the current tcell breaks the backward compatibility of the terminal.

I also found that the current implementation of tcell is incomplete. You can give MouseFlags as an argument to EnableMouse(), but if you give only MouseMotionEvents, the # 477 Hack won't work. It will work if you modify it as follows.

func (t *tScreen) enableMouse(f MouseFlags) {
        // Rather than using terminfo to find mouse escape sequences, we rely on the fact that
        // pretty much *every* terminal that supports mouse tracking follows the
        // XTerm standards (the modern ones).
        if len(t.mouse) != 0 {
                // start by disabling all tracking.
                t.TPuts("\x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1006l")
                if f&MouseMotionEvents != 0 {
                        t.TPuts("\x1b[?1000h\x1b[?1002h\x1b[?1003h\x1b[?1006h")
                } else if f&MouseDragEvents != 0 {
                        t.TPuts("\x1b[?1000h\x1b[?1002h\x1b[?1006h")
                } else if f&MouseButtonEvents != 0 {
                        t.TPuts("\x1b[?1000h\x1b[?1006h")
                }
        }
}

@tmatz
Copy link
Contributor Author

tmatz commented Feb 15, 2022

@gdamore
I also checked the source of putty (https://git.tartarus.org/simon/putty.git) and found that putty also completely ignores 1003. Also, I didn't see any particular reason why 1006 had to be enabled when mouse events were disabled.

@gdamore
Copy link
Owner

gdamore commented Feb 16, 2022

I'm going to take a close look at this. My fear is that there is some impedance mismatch here in the intended behavior and the actual... and what happens in the terminals.

It is not surprising to me that some of these flags are ignored by terminals... the design actually depends upon it -- they should enable what they can, and ignore the rest.

The other thing is that my commentary earlier may also have been misunderstanding the context where this occurs.

tscreen.go Outdated Show resolved Hide resolved
@tmatz tmatz force-pushed the restore_mouse_event_setting branch from e47b2bd to 39161d6 Compare February 17, 2022 04:00
@gdamore gdamore merged commit 2a1a1b5 into gdamore:master Mar 13, 2022
@tmatz tmatz deleted the restore_mouse_event_setting branch March 14, 2022 02:15
@tmatz
Copy link
Contributor Author

tmatz commented Mar 14, 2022

@gdamore thank you very much!

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