Add an experimental setting for moving the cursor with the mouse #15758

zadjii-msft · 2023-07-25T17:40:08Z

Summary of the Pull Request

This adds a new experimental per-setting to the terminal.

"experimental.moveCursorWithMouse": bool

When:

the setting is on
AND you turn on shell integration (at least 133;B)
AND you click is somewhere after the "active command" mark

we'll send a number of simulated keystrokes to the terminal based off the number of cells between the place clicked and where the current mouse cursor is.

PR Checklist

Related to Allow mouse clicks to reposition cursor by emitting cursor key sequences #8573. I'm not marking as closed, because we should probably polish this before we close that out. This is more a place to start.

Detailed Description of the Pull Request / Additional comments

There was a LOT of discussion in #8573. This is kinda a best effort feature - it won't always work, but it should improve the experience most of the time. We all kinda agreed that as much as the shell probably should be responsible for doing this, there's myriad reasons that won't work in practicality:

That would also disable selection made by the terminal. That's a hard sell.
We'd need to invent some new mouse mode to support click-to-reposition-but-drags-to-select-I-don't-want
We'd then need shells to adopt that functionality.

And eventually settled that this was the least horrifying comprimise.

This has e d g e c a s e s:

Does it work for wrapped lines? Well, kinda okay actually.
Does it work for vim/emacs? Nope.
Does it work for emoji/wide glyphs? I wouldn't expect it to! I mean, emoji input is messed up anyways, right?
Other characters like ESC (which are rendered by the shell as two cells "^[")? Nope.
Does it do selections? Nope.
Clicking across lines with continuation prompts? Nope.
Tabs? Nope.
Wraps within tmux/screen? Nope.

https://github.com/xtermjs/xterm.js/blob/master/src/browser/input/MoveToCell.ts has probably a more complete implementation of how we'd want to generate the keypresses and such.

src/cascadia/TerminalControl/ControlCore.cpp

lhecker · 2023-07-26T09:12:06Z

src/cascadia/TerminalControl/ControlCore.cpp

+                        _terminal->SendKeyEvent(key, 0, {}, true);
+                        _terminal->SendKeyEvent(key, 0, {}, false);


For purposes like this it would be nice if we could stop flushing our input pipe so that the keystrokes are sent as a unison. For both pwsh and cmd this would avoid re-printing the entire commandline on every cursor navigation. One option would be to do something like _terminal->CorkInputPipe() and UncorkInputPipe() (borrowing TCP_CORK terminology), but I also somewhat feel like it would be better if we instead made it so that SendKeyEvent will never flush the input pipe and instead we add a _terminal->FlushInput() function which you need to call when you're done generating input. Alternatively, we could also extract TerminalInput out of Terminals control and accumulate a std::string here before sending it off. Or we just generate cursor sequences ourselves. I don't think there's any fantastic solution, but there's a lot of alright ones. 🙂

(xterm.js does this as well btw.)

Could we create a follow-up issue for this? I think it would be worthwhile in general to keep track of features that would benefit of corking/uncorking the input pipe.

…eriment

src/buffer/out/textBuffer.cpp

src/cascadia/TerminalControl/ControlCore.cpp

…eriment

zadjii-msft · 2023-08-09T15:25:31Z

TODO!

Use CMD.
Submit a command.
Then click off to the right of the prompt.
We'll send → until that point, which CMD will use to fill in the rest of the previous commandline.

gotta find a way to clamp it to the rightmost character. Probably wont work right for folks with right-aligned prompts.

DHowett · 2023-08-10T21:09:41Z

nit: maybe repositionCursorOnClick is more in line with what other terminals call it?

DHowett

Sorry to block two in a row, but I've got Questions! And a suggestion for a name.

DHowett · 2023-08-10T21:42:27Z

src/cascadia/UnitTests_Control/ControlCoreTests.cpp

+                                  false,
+                                  needToCopy);
+
+        VERIFY_IS_TRUE(core->HasSelection());


wait, why now does a single left click result in a selection? I thought it for sure didn't do that...

that true means shift was pressed. A single click with a shift does select

DHowett · 2023-08-10T21:42:46Z

src/cascadia/UnitTests_Control/ControlCoreTests.cpp

+            VERIFY_ARE_EQUAL(expectedStart, start);
+            VERIFY_ARE_EQUAL(expectedEnd, end);
+        }
+        VERIFY_IS_TRUE(gotSelectionUpdate);


I don't actually understand how this selection test is related to click-to-reposition...

It's actually not! But I broke that selection rendering thing in that last bug bash, in an earlier commit in this PR. This test I wrote to make sure I un-broke it, and won't re-break it in the future.

Oh, I see. I thought it was a render lock bug, so i didn't put the two together

zadjii-msft · 2023-08-10T21:46:15Z

repositionCursorOnClick

Sure, I don't care

…eriment

zadjii-msft · 2023-08-14T12:37:02Z

(force merging since it seems like GH is confused. Probably since we changed the build rules during the course of this PR)

I could list the PRs but I aint got time for that closes #687 closes #668 Docs microsoft/terminal#15758

zadjii-msft added 2 commits July 24, 2023 14:47

I hate that this works

5348be2

cleanup for review

e28fc9b

lhecker reviewed Jul 26, 2023

View reviewed changes

zadjii-msft added 2 commits August 3, 2023 05:15

Merge remote-tracking branch 'origin/main' into dev/migrie/f/8573-exp…

9e32a51

…eriment

Count glyphs not wchars

e2457d7

lhecker reviewed Aug 3, 2023

View reviewed changes

src/buffer/out/textBuffer.cpp Show resolved Hide resolved

zadjii-msft added the zBugBash-Consider label Aug 8, 2023

zadjii-msft commented Aug 9, 2023

View reviewed changes

src/cascadia/TerminalControl/ControlCore.cpp Outdated Show resolved Hide resolved

zadjii-msft added 2 commits August 9, 2023 08:23

Merge remote-tracking branch 'origin/main' into dev/migrie/f/8573-exp…

797e2da

…eriment

fix this selection bug and add a test so we don't do it again

4603ffa

This comment has been minimized.

Sign in to view

kids don't play wit too many arrows

03ce341

This comment has been minimized.

Sign in to view

zadjii-msft added this to the Terminal v1.19 milestone Aug 10, 2023

zadjii-msft assigned DHowett Aug 10, 2023

DHowett requested changes Aug 10, 2023

View reviewed changes

microsoft-github-policy-service bot added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Aug 10, 2023

nits

4331c8b

This comment has been minimized.

Sign in to view

zadjii-msft requested a review from DHowett August 10, 2023 22:05

🤦

f54d4d4

DHowett approved these changes Aug 10, 2023

View reviewed changes

Merge remote-tracking branch 'origin/main' into dev/migrie/f/8573-exp…

8a22f9f

…eriment

zadjii-msft assigned lhecker and unassigned DHowett Aug 11, 2023

lhecker approved these changes Aug 11, 2023

View reviewed changes

zadjii-msft enabled auto-merge (squash) August 14, 2023 10:45

zadjii-msft disabled auto-merge August 14, 2023 12:36

zadjii-msft merged commit b556594 into main Aug 14, 2023

zadjii-msft deleted the dev/migrie/f/8573-experiment branch August 14, 2023 12:37

zadjii-msft mentioned this pull request Aug 23, 2023

Allow mouse clicks to reposition cursor by emitting cursor key sequences #8573

Open

zadjii-msft mentioned this pull request Sep 26, 2023

A BUNCH of docs for 1.19 MicrosoftDocs/terminal#712

Merged

zadjii-msft added a commit to MicrosoftDocs/terminal that referenced this pull request Sep 28, 2023

A BUNCH of docs for 1.19 (#712)

df0b756

I could list the PRs but I aint got time for that closes #687 closes #668 Docs microsoft/terminal#15758

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an experimental setting for moving the cursor with the mouse #15758

Add an experimental setting for moving the cursor with the mouse #15758

zadjii-msft commented Jul 25, 2023

lhecker Jul 26, 2023 •

edited

Loading

lhecker Aug 11, 2023

zadjii-msft commented Aug 9, 2023

This comment has been minimized.

This comment has been minimized.

DHowett commented Aug 10, 2023

DHowett left a comment

DHowett Aug 10, 2023

zadjii-msft Aug 10, 2023

DHowett Aug 10, 2023

zadjii-msft Aug 10, 2023

DHowett Aug 10, 2023

zadjii-msft commented Aug 10, 2023

This comment has been minimized.

zadjii-msft commented Aug 14, 2023

		_terminal->SendKeyEvent(key, 0, {}, true);
		_terminal->SendKeyEvent(key, 0, {}, false);

Add an experimental setting for moving the cursor with the mouse #15758

Add an experimental setting for moving the cursor with the mouse #15758

Conversation

zadjii-msft commented Jul 25, 2023

Summary of the Pull Request

PR Checklist

Detailed Description of the Pull Request / Additional comments

lhecker Jul 26, 2023 • edited Loading

Choose a reason for hiding this comment

lhecker Aug 11, 2023

Choose a reason for hiding this comment

zadjii-msft commented Aug 9, 2023

This comment has been minimized.

This comment has been minimized.

DHowett commented Aug 10, 2023

DHowett left a comment

Choose a reason for hiding this comment

DHowett Aug 10, 2023

Choose a reason for hiding this comment

zadjii-msft Aug 10, 2023

Choose a reason for hiding this comment

DHowett Aug 10, 2023

Choose a reason for hiding this comment

zadjii-msft Aug 10, 2023

Choose a reason for hiding this comment

DHowett Aug 10, 2023

Choose a reason for hiding this comment

zadjii-msft commented Aug 10, 2023

This comment has been minimized.

zadjii-msft commented Aug 14, 2023

lhecker Jul 26, 2023 •

edited

Loading