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

Integrated terminal mouse reporting not working #96058

Closed
probablykasper opened this issue Apr 24, 2020 · 20 comments · Fixed by #120145
Closed

Integrated terminal mouse reporting not working #96058

probablykasper opened this issue Apr 24, 2020 · 20 comments · Fixed by #120145
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders terminal Integrated terminal issues verified Verification succeeded
Milestone

Comments

@probablykasper
Copy link
Contributor

probablykasper commented Apr 24, 2020

  • VSCode 1.44.2
  • macOS 10.14.6

Steps to Reproduce:

  1. Open a file in vim by running vim <file> in the integrated terminal
  2. Type :set mouse=a and press enter, to enable mouse reporting
  3. Now, selecting text in the file you opened does not work

This apparently does not affect tmux.

Does this issue occur when all extensions are disabled?: Yes

@Tyriar
Copy link
Member

Tyriar commented Apr 25, 2020

tmux mouse events are working fine for me:

Kapture 2020-04-25 at 5 56 33

Version info:

Version: 1.45.0-insider
Commit: a250df7
Date: 2020-04-24T20:46:59.782Z
Electron: 7.2.2
Chrome: 78.0.3904.130
Node.js: 12.8.1
V8: 7.8.279.23-electron.0
OS: Darwin x64 19.0.0

@Tyriar Tyriar added the info-needed Issue requires more information from poster label Apr 25, 2020
@probablykasper
Copy link
Contributor Author

@Tyriar My bad, I haven't tested this on tmux or emacs. I thought mouse reporting just wasn't supported at all, but that's clearly not the case. Updated the issue description.

Were you able to reproduce the issue in vim?

@Tyriar
Copy link
Member

Tyriar commented Apr 27, 2020

You're right it doesn't seem to be working for vim only. @jerch maybe a bug with the mouse protocol changes? 🤔

When entering vim:

xterm.js: Binding to mouse events: {down: true, up: true, drag: true, move: false, wheel: true}

Some clicks that don't seem to do anything:

xterm.js: sending binary "�[M #"" (6) [27, 91, 77, 32, 35, 34]
xterm.js: sending binary "�[M##"" (6) [27, 91, 77, 35, 35, 34]
xterm.js: sending binary "�[M */" (6) [27, 91, 77, 32, 42, 47]
xterm.js: sending binary "�[M#*/" (6) [27, 91, 77, 35, 42, 47]

tmux which works fine:

xterm.js: Binding to mouse events: {down: true, up: true, drag: true, move: false, wheel: true}
xterm.js: sending data "�[<0;31;8M" (10) [27, 91, 60, 48, 59, 51, 49, 59, 56, 77]
xterm.js: parsing data �[?25l�[32m�[1;45H│�[2;45H│�[3;45H│�[4;45H│�[5;45H│�[6;45H│�[7;45H│�[8;45H│�[9;45H│�[10;45H│�[11;45H│�[12;45H│�[13;45H│�[39m�[14;45H│�[15;45H│�[16;45H│�[17;45H│�[18;45H│�[19;45H│�[20;45H│�[21;45H│�[22;45H│�[23;45H│�[24;45H│�[25;45H│�(B�[m�[3;3H�[?12l�[?25h
xterm.js: sending data "�[<0;31;8m" (10) [27, 91, 60, 48, 59, 51, 49, 59, 56, 109]

@jerch
Copy link

jerch commented Apr 28, 2020

@Tyriar vim works fine for me in xterm.js demo. Does vscode forward binary event data from onBinary?

@Tyriar
Copy link
Member

Tyriar commented Apr 28, 2020

@jerch that must be it, onBinary was never adopted.

@Tyriar Tyriar added this to the April 2020 milestone Apr 28, 2020
@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug terminal Integrated terminal issues and removed info-needed Issue requires more information from poster labels Apr 28, 2020
@Tyriar Tyriar modified the milestones: April 2020, May 2020 Apr 29, 2020
@Tyriar Tyriar modified the milestones: May 2020, June 2020 May 11, 2020
@xgdgsc
Copy link
Contributor

xgdgsc commented May 13, 2020

htop mouse click also doesn' t work. Is it the same problem?

@jerch
Copy link

jerch commented May 13, 2020

@xgdgsc Well for me (Ubuntu 18 LTS) htop reports with the old mouse encoding, so yes. Imho it is built with ncurses, still its terminfo states SGR mouse for vscode terminal. Not sure when this was added, it might be an issue with an older ncurses version on your and my system.

@Tyriar Guess we should fix the DA responses soon, and maybe reach out to @ThomasDickey about a better fitting terminfo entry?

@Tyriar
Copy link
Member

Tyriar commented May 13, 2020

@jerch oh would removing the termName reports for xtermjs/xterm.js#2894 move it back to the older encoding?

@jerch
Copy link

jerch commented May 13, 2020

Well it seems apps and libs like ncurses have a hard time to figure out what terminal xterm.js is, it seems they go for "safe denominator" which switches off SGR mouse reports for us currently (thats what happens in vim above). This is also the reason why I wrote the DA issue after inspecting the vim source.

About a valid terminfo entry - that would help ncurses and similar libs alot to get along with xterm.js. The current entry looks abit like a stub: https://invisible-island.net/ncurses/terminfo.src.html#toc-_Visual__Studio. Also it points to some issues we had in the past, some of them are already fixed.

@ThomasDickey
Copy link

I recall that there was a Debian bug report where the Debian developer decided to disable SGR-mouse to simplify use on older systems. This report appears to be a result of that. If you'd like your program to be portable, it has to handle either mode.

@jerch
Copy link

jerch commented May 13, 2020

@ThomasDickey The terminal supports X11 reports up to the upper bound (beyond that will stop reporting anything) and SGR. We removed "UTF-8" and "rxvt" support after some discussions about app usage and decided to go with SGR only for higher coords. So yes, the terminal supports X11 and SGR mouse encoding.

Care to explain the downvote?

@ThomasDickey
Copy link

Your comments might have been constructive if they had been part of a regular bug report. In this context, they weren't constructive.

@ThomasDickey
Copy link

Actually, I took a quick look with the current Visual Studio Code, and the any-event report in normal (non-1006) did not appear to work. So I added a to-do item to review that version, and add notes as they apply to that terminal description.

@jerch
Copy link

jerch commented May 13, 2020

@ThomasDickey Ah sorry, my note about the terminfo wasnt meant to offend your hard work on it (I only can imagine how awful it is to test against all those lousy emulators popping up) - it was meant as an idea to help to keep the entry up to date. The terminal is still subject of many shifts, which makes it hard to pin it down. Same goes for my DA notion, the emulator does some weird things internally we had a discussion about before. All I am saying - we should fixate that to some reliable values. From my perspective this is well ontopic, because the initial report was about vim, which relies on DA reports (while htop relies on ncurses).

@ThomasDickey
Copy link

I see. Will update later, if I find something that helps with this report.

@ThomasDickey
Copy link

ThomasDickey commented May 16, 2020

The terminal in Visual Studio Code 1.45.0 does nothing when the xterm mouse-mode is set to the default (the original mode). I see this using vttest, which shows me that mouse-mode is only sending events when the "Mode" is set to "SGR coordinates".

While ncurses 6.1 changed the description for xterm-256color to use "SGR coordinates" (1006), older versions of the terminal database (such as that bundled with MacOS) do not have this change, because they have not been updated for a long time.

Debian (and Ubuntu) do not, for a different reason: the packagers modify the terminal database distributed with ncurses by pasting in other entries (including replacing the xterm entries with the source from xterm). You can see this in the package scripts for ncurses-base

None of those modifications improves the terminal database, but the replacement of xterm-entries further delays by a year or so the availability of the 1006 mouse mode. I modified xterm's source later, to incorporate changes I'd made for ncurses 6.1, which won't help with problems using bionic (Ubuntu 18).

@nojvek
Copy link
Contributor

nojvek commented Jan 24, 2021

@Tyriar is the onBinary hookup for mouse support something I can do without a ton of hand holding? Would really love to get mouse support in vscode terminal. It would open up gateway to build truly interactive terminal apps.

@Tyriar
Copy link
Member

Tyriar commented Jan 25, 2021

@nojvek that would be awesome! Here's roughly what's involved in that:

Create ITerminalInstance.onBinary, implement the event in TerminalInstance and hook up to xterm:

/**
* Attach a listener to the raw data stream coming from the pty, including ANSI escape
* sequences.
*/
onData: Event<string>;

Copy where onData (aka onProcessData) goes with onBinary (onProcessBinary), find references/text search should help here. It will need to cover:

  • TerminalInstance -> TerminalProcessManager -> TerminalProcess
  • TerminalInstance -> TerminalProcessManager -> TerminalProcessExtHostProxy -> MainThreadTerminalService -> ExtHostTerminalService -> TerminalProcess

There is also RemoteTerminalChannel, just fill in the interfaces and leave a TODO there and I can fill that part in.

@nojvek
Copy link
Contributor

nojvek commented Jan 26, 2021

@Tyriar For my own sanity, what is the difference between onData and onBinary? I'd assume the onData would be the raw binary data stream. For my own deeper understanding, is there an architecture page that describes how xterm <-> vscode architecture works? If not that is fine too. The topic interests me that's all.

@Tyriar
Copy link
Member

Tyriar commented Jan 27, 2021

@nojvek some data is not extra expressible via JS strings because of how they are encoded, some mouse events include that. I think the introduction of onBinary fixed some obscure bugs with certain mouse events not working, but now they use onBinary and actual bytes are passed around.

@Tyriar Tyriar modified the milestones: Backlog, March 2021 Mar 3, 2021
@Tyriar Tyriar assigned meganrogge and unassigned Tyriar Mar 3, 2021
meganrogge added a commit that referenced this issue Mar 5, 2021
@Tyriar Tyriar modified the milestones: March 2021, April 2021 Mar 18, 2021
@rzhao271 rzhao271 added the verified Verification succeeded label Apr 28, 2021
@github-actions github-actions bot locked and limited conversation to collaborators May 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders terminal Integrated terminal issues verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants