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

Introduce Mark Mode spec (add-on) #5804

Merged
merged 31 commits into from
Jul 22, 2022
Merged

Introduce Mark Mode spec (add-on) #5804

merged 31 commits into from
Jul 22, 2022

Conversation

carlos-zamora
Copy link
Member

Summary of the Pull Request

This is a spec specifically dedicated to Mark Mode. It's an addition to the Keyboard Selection spec. I felt that it makes the most sense to make this a separate PR because there's a lot of ideas that are very specific to Mark Mode, and this gives us the space to modify some of that behavior and get a good look at how other terminal emulators designed this feature.

References

#2840 - Keyboard Selection Spec (base spec/branch/PR)

PR Checklist

@carlos-zamora carlos-zamora added Area-Interaction Interacting with the vintage console window (as opposed to driving via API or hooks) Product-Terminal The new Windows Terminal. labels May 8, 2020
@carlos-zamora
Copy link
Member Author

Comment from @zadjii-msft in base PR:

Okay lemme make sure I've got this right.

  • (by default) When the user presses the toggleMarkMode binding, and holds Shift, they'll enter mark mode, and Shift+arrow keys will move the endSelectionAnchor, until Shift is released, Esc is pressed, or the user copies.
  • If the user changes the anchorMode instead to toggle, their binding will enter mark mode. Shift+Arrows will move the cursor, as the startPosition. Shift will toggle the anchor we're moving (which would certainly act weirdly in combination with Shift+Arrows for navigation, but that's up to the user)

Correct. I feel like the default should be toggle though. I kind of like that more.

Also, I'm noticing a major annoyance with this. I don't like the second scenario because you should really just use the arrow keys (not shift+arrow keys). There should be no need for shift. Ideally, we could just "subtract" the modifier key from the keybinding. But would that be a difficult behavior to explain/understand?

Curious on your thoughts. I'm hoping this new PR can provide some good space for discussion on how to approach this in particular.

@zadjii-msft
Copy link
Member

There should be no need for shift. Ideally, we could just "subtract" the modifier key from the keybinding. But would that be a difficult behavior to explain/understand?

I'm worried that might not totally work. Think of an insane person, who definitely wants to be able to select text with WASD, and an interesting set of modifiers:

// Cell Selection
{ "keys": ["shift+w"], "command": { "action": "moveSelectionPoint", "direction": "up" } },
{ "keys": ["ctrl+a"],  "command": { "action": "moveSelectionPoint", "direction": "left" } },
{ "keys": ["alt+s"],   "command": { "action": "moveSelectionPoint", "direction": "down" } },
{ "keys": ["d"],       "command": { "action": "moveSelectionPoint", "direction": "right" } },

You can't really "subtract shift" from those modifiers 😕

Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay so I'm not sure where to really put these thoughts, so I'm putting them here for now.

Maybe where I'm confused is: right now in conhost, you can press ctrl+M to enter mark mode, then arrows to move the anchor. That's not really possible with these bindings currently, right? Unless you set the moveSelectionAnchor to arrows (no shift)

If a person wanted exactly conhost mark mode behavior (lets limit it to the arrow keys for brevity), what would that look like in their settings? And same with the iTerm2 Copy Mode equivalent bindings? I think if we had a breakdown of both of these cases and how they'd look&work, I think I'd understand this a bit more.

doc/specs/Keyboard-Selection.md Outdated Show resolved Hide resolved
@@ -74,6 +112,7 @@ Thanks to Keybinding Args, there will only be 4 new commands that need to be add
| | `Enum direction { up, down, left, right}` | The direction the selection will be moved in. |
| | `Enum expansionMode { cell, word, line, viewport, buffer }` | The context for which to move the selection anchor to. (defaults to `cell`)
| `selectEntireBuffer` | | Select the entire text buffer.
| `toggleMarkMode` | | Enter or exit mark mode. This allows you to create an entire selection using only the keyboard. |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait is the default toggle or hold?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'll update the spec after we've had some of these conversations)
Even though Mark Mode would normally be 'hold', I personally like the idea of 'toggle' more. I think we should make it 'toggle'.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. I defined the default as toggle below. So I guess now we can just discuss it, if you disagree haha

@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label May 8, 2020
@carlos-zamora carlos-zamora force-pushed the dev/cazamor/spec-ks/mark-mode branch from 9a4aad7 to 0ff7682 Compare May 8, 2020 17:54
@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label May 8, 2020
@jsoref
Copy link
Contributor

jsoref commented May 10, 2020

Fwiw, you could add a pattern ala https://github.com/microsoft/terminal/blob/master/.github/actions/spell-check/patterns/patterns.txt#L2 instead of whitelisting copymode.

Copy link
Contributor

@leonMSFT leonMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of clarifications:

doc/specs/Keyboard-Selection.md Outdated Show resolved Hide resolved
doc/specs/Keyboard-Selection.md Outdated Show resolved Hide resolved
In ConHost, output would be paused when a selection was present. This is a completely separate issue that is being tracked in (#2529)[https://github.com/microsoft/terminal/pull/2529].

#### Interaction with CopyOnSelect
If `copyOnSelect` is enabled, the selection is copied when the selection operation is "complete". If `anchorMode=Hold`, the user has to use the `copy` keybinding to signify that they have finished creating a selection. If `anchorMode=Toggle`, the selection is copied either when the `copy` keybinding is used, or when the user presses the `anchorModifier` key and the 'end' endpoint is set.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be misunderstanding the last part of this section, but it sounds like it'll only copy if the user hits the anchorModifier key and that one keypress anchors the end endpoint, but not if it anchors the start endpoint. It should probably copy no matter which endpoint is set right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, my concern here is that we shouldn't be copying too often. With mouse selection, it's easy because we know a selection is "complete" when the user releases the left button. Abstracting that logic to Mark Mode (in anchorMode=toggle),...

  • enter mark mode --> X (you just use the mouse)
  • switch selection anchor target --> press/hold left mouse button
  • move 'end' anchor --> drag mouse
  • switch selection anchor target --> release left mouse button

@carlos-zamora
Copy link
Member Author

carlos-zamora commented May 13, 2020

Urgh. Yeah, so there's 2 parts here:

anchorMode: Toggle

I think where this spec stands, this is fine? You can just use your moveSelectionAnchor keybindings normally.

anchorMode: Hold

The main idea behind ConHost CMD Mark Mode is basically that as long as shift is held, the 'start' endpoint is static/frozen/held.

I'm worried that might not totally work. Think of an insane person, who definitely wants to be able to select text with WASD, and an interesting set of modifiers:

// Cell Selection
{ "keys": ["shift+w"], "command": { "action": "moveSelectionPoint", "direction": "up" } },
{ "keys": ["ctrl+a"],  "command": { "action": "moveSelectionPoint", "direction": "left" } },
{ "keys": ["alt+s"],   "command": { "action": "moveSelectionPoint", "direction": "down" } },
{ "keys": ["d"],       "command": { "action": "moveSelectionPoint", "direction": "right" } },

You can't really "subtract shift" from those modifiers 😕

So, with the logic above, the scenario above might still be acceptable?

Direction Move 'start' Move 'end'
Up w shift+w
Left ctrl+a shift+ctrl+a
Down alt+s shift+alt+s
Right d shift+d

Alternative Plan

Alternatively, I feel like the safest option would be to have users specifically define controls in Mark Mode vs keyboard selection. I think that might be the clearest way to fix the problem above, but I feel like there's a lot of repeated keybindings set between mark mode and non-mark mode ones. :/

Alternative Alternative Plan

We could remove the "anchorMode: Hold". But then there's no way to replicate a similar behavior to that of ConHost CMD. :( I also think the concept of "hold" is still a good one, but the execution is the tough part ugh

@Neurrone
Copy link

Neurrone commented May 20, 2020

Could support be added for the following scenario:

  1. Toggle on mark mode.
  2. Move cursor to the start of selection point, and press a hotkey to mark the start of the selection anchor.
  3. Move the cursor around (without having to press shift with each keystroke) until you are at the end of the selection.
  4. Use a keystroke to mark the current position as the end of the selection point. The region enclosed will be selected. The end could also be text that appears before the start.

This is being implemented in Vs Code, see microsoft/vscode#95894, inspired by e.g, Notepad++ which has this. This is extremely useful when selecting large chunks of text.

@carlos-zamora
Copy link
Member Author

Could support be added for the following scenario:

  1. Toggle on mark mode.
  2. Move cursor to the start of selection point, and press a hotkey to mark the start of the selection anchor.
  3. Move the cursor around (without having to press shift with each keystroke) until you are at the end of the selection.
  4. Use a keystroke to mark the current position as the end of the selection point. The region enclosed will be selected. The end could also be text that appears before the start.

This is being implemented in Vs Code, see microsoft/vscode#95894, inspired by e.g, Notepad++ which has this. This is extremely useful when selecting large chunks of text.

Honestly, the best synergy with the moveSelectionAnchor keybindings is to do just that, then abandon the idea of anchorMode: hold. I had some offline discussions about that and I think the next revision will basically just move the concept of anchorMode: hold to a future considerations section. Maybe that could just be an extension.

@zadjii-msft unfortunately, that would mean that you would not be able to replicate ConHost's Mark Mode. But I think the positives outweigh the negatives.

@Neurrone
Copy link

Another suggestion: how hard would it be to account for navigation in the output buffer by command?

For example, lets say you have the following terminal output:

> pwd
/some/path

> dir
// lots of output

> whoami
some-user

I would like to be able to skip past the output of a command and move to the input prompt of the next one. For example, if my cursor were at the bottom, pressing some keystroke would move me to the second last line. Pressing it again moves me to the input prompt where I typed dir, allowing me to easily skip past the many lines of output. This could let me then go down to review the output of the dir command at my leisure, especially useful in the situation where it scrolls on for many lines.

@zadjii-msft
Copy link
Member

Yea I'm totally cool with ditching the hold mark mode. That seems to be the part that's creating the most friction. We've clearly established that pattern is hard to reconcile with the more widely adopted UX that's present in other applications.

I had a real showerthought of an idea, but I don't want to distract from the thread too much. Expand the below at your own risk:

Details

What if we had a swapSelectionEndpoint (name needs workshopping) property in the toggleMarkMode action. When we enter mark mode, if the user presses that key, we'll toggle them to the other selection anchor. When the release it, we'll toggle it back. So this key would act as a toggleEndpoint action on both keydown and keyup.

Theoretically, the user could enter mark mode, mode the selection start anchor to (1,1), then toggle the endpoint (with toggleEngpoint), move the selection end to (12, 34), then hold swapSelectionEndpoint to move the start to (12, 1), if they wanted. It's a weird workflow, sure, but then we might be able to convey "this key will move the other anchor".

Then, cmd mark mode would be

  { "action": "toggleMarkMode", "swapSelectionEndpoint": "shift", "keys":"ctrl+M" }

and iterm2 would be

  { "action": "toggleMarkMode", "keys":"ctrl+M" },
  { "action": "toggleEndpoint", "keys":"ctrl+space" },

@carlos-zamora
Copy link
Member Author

Another suggestion: how hard would it be to account for navigation in the output buffer by command?

For example, lets say you have the following terminal output:

> pwd
/some/path

> dir
// lots of output

> whoami
some-user

I would like to be able to skip past the output of a command and move to the input prompt of the next one. For example, if my cursor were at the bottom, pressing some keystroke would move me to the second last line. Pressing it again moves me to the input prompt where I typed dir, allowing me to easily skip past the many lines of output. This could let me then go down to review the output of the dir command at my leisure, especially useful in the situation where it scrolls on for many lines.

I think MacOS's terminal lets you do something like this (found it on accident the other day). For now, you'd probably have to do a search, then enter mark mode. Though this would be a neat extension or future feature to add in. I'll be sure to give it a shoutout.

doc/specs/Keyboard-Selection.md Outdated Show resolved Hide resolved
doc/specs/Keyboard-Selection.md Outdated Show resolved Hide resolved
@jsoref
Copy link
Contributor

jsoref commented May 28, 2020

Could you use an SVG instead of a JPG? JPGs are screen and reader hostile. Or at least a PNG so that the image won't suffer from artifacts...

@carlos-zamora
Copy link
Member Author

Another suggestion: how hard would it be to account for navigation in the output buffer by command?
For example, lets say you have the following terminal output:

> pwd
/some/path

> dir
// lots of output

> whoami
some-user

I would like to be able to skip past the output of a command and move to the input prompt of the next one. For example, if my cursor were at the bottom, pressing some keystroke would move me to the second last line. Pressing it again moves me to the input prompt where I typed dir, allowing me to easily skip past the many lines of output. This could let me then go down to review the output of the dir command at my leisure, especially useful in the situation where it scrolls on for many lines.

I think MacOS's terminal lets you do something like this (found it on accident the other day). For now, you'd probably have to do a search, then enter mark mode. Though this would be a neat extension or future feature to add in. I'll be sure to give it a shoutout.

Actually, this would be an extension/keybinding on its own. It's pretty independent. Created issue #6232 for tracking.

@jsoref
Copy link
Contributor

jsoref commented May 28, 2020

vi uses % to move between start/ends of things... not sure if one could get away w/ a modified version of that.

@carlos-zamora carlos-zamora force-pushed the dev/cazamor/spec-ks/mark-mode branch from be4b35b to eac00b2 Compare May 28, 2020 00:26
@jsoref
Copy link
Contributor

jsoref commented May 28, 2020

The arrows for moveSelectionPoint in the middle are odd... are some missing? are the lines running oddly?

@carlos-zamora
Copy link
Member Author

vi uses % to move between start/ends of things... not sure if one could get away w/ a modified version of that.

#5226 mentions something like this (albeit for double click selection), so I'm linking it here. Totally forgot to add it to the spec so that's my bad.

I don't follow what you're saying though. VIM uses % to move to the matching brace, right? The current form of the spec doesn't have that functionality. Instead, we're moving by cell/word/viewport. The idea behind #5226 is that a keybinding command gets added to specifically do that.

So, like, say we add the command "selectToMatchingBrace", then we could...

  • create a keybinding to do that. And that would work when you do a quick edit or are in Mark Mode.
  • link a pointer binding to this so that, for example, a "double-click" event results in invoking "selectToMatchingBrace"

I'll add this to the spec under future considerations, because I think this is a good example of extensibility for selection-oriented actions/commands?

@carlos-zamora
Copy link
Member Author

The arrows for moveSelectionPoint in the middle are odd... are some missing? are the lines running oddly?

You're talking about the giant code block under UI/UX Design right? They look fine to me. I excluded moving by word up/down because I feel like conceptually those don't really make sense. I also excluded moving by buffer left/right because that shouldn't differ from moving by viewport left/right.

@zadjii-msft zadjii-msft added the Issue-Docs It's a documentation issue that really should be on MicrosoftDocs/Console-Docs label Jun 16, 2020
@github-actions

This comment was marked as resolved.

ghost pushed a commit that referenced this pull request Jun 7, 2022
## Summary of the Pull Request
This introduced the `toggleBlockSelection` action to allow users to create a block selection using only the keyboard. This is not bound to any keys by default, however it is added to the command palette.

## References
#4993 - Epic
#5804 - Spec

## Validation Steps Performed
- [X] Mark mode always starts in line selection mode
- [X] Mouse selections are always in line selection mode by default
- [X] Can toggle block selection for an existing selection (regardless of how it was created)
- [X] The selection is copied properly (aka, no rendering issues)
DHowett pushed a commit that referenced this pull request Jun 20, 2022
## Summary of the Pull Request
This introduces a selection marker overlay that tells the user which endpoint is currently being moved by the keyboard. The selection markers are respect font size changes and `cursor` color.

## References
#715 - Keyboard Selection
#2840 - Keyboard Selection Spec
#5804 - Mark Mode Spec

## Detailed Description of the Pull Request / Additional comments
- `TermControl` layer:
   - Use a canvas (similar to the one used for hyperlinks) to be able to draw the selection markers.
   - If we are notified that the selection changed, update the selection markers appropriately.
   - `UpdateSelectionMarkersEventArgs` lets us distinguish between mouse and keyboard selections.  `ClearMarkers` is set to true in the following cases...
      1.  Mouse selection, via SetEndSelectionPoint
      2. `LeftClickOnTerminal`, pretty self-explanatory
      3. a selection created from searching for text
- `ControlCore` layer:
   - Responsible for notifying `TermControl` to update the selection markers when a selection has changed.
   - Transfers info (the selection endpoint positions and which endpoint we're moving) from the terminal core to the term control.
- `TerminalCore` layer:
   - Provides the viewport position of the selection endpoints.


## Validation Steps Performed
- mouse selection (w/ and w/out shift) --> no markers
- keyboard selection --> markers
- markers update appropriately when we pivot the selection
- markers scroll when you hit a boundary
- markers take the color of the cursor color setting
- markers are resized when the font size changes
ghost pushed a commit that referenced this pull request Jul 1, 2022
## Summary of the Pull Request
Introduces the `switchSelectionEndpoint` action which switches whichever selection endpoint is targeted when a selection is present. For example, if you are targeting "start", `switchSelectionEndpoint` makes it so that now you are targeting "end". This also updates the selection markers appropriately.

## References
Spec - #5804 
#13358
Closes #3663

## Detailed Description of the Pull Request / Additional comments
Most of the code is just standard code of adding a new action. Other than that, we have...
- if there is no selection, the action fails and the keybinding is passed through (similar to `copy()`)
- when we update the selection endpoint, we need to also update the "pivot". This ensures that future calls of `UpdateSelection()` respect this swap.
- [Corner Case] if the cursor is being moved, we make it so that you basically "anchored" an endpoint and you don't have to hold shift anymore.
@github-actions

This comment was marked as resolved.

@zadjii-msft zadjii-msft self-assigned this Jul 8, 2022
@zadjii-msft zadjii-msft removed their assignment Jul 14, 2022
@github-actions

This comment has been minimized.

@jsoref
Copy link
Contributor

jsoref commented Jul 18, 2022

Re: #5804 (comment)
My memory of % is that it handles at least [],()/{} jumps to the matching item.

Apparently "modern" vim also can do way more than that:
https://www.vim.org/scripts/script.php?script_id=39

@zadjii-msft
Copy link
Member

@DHowett cmon man you know you wanna close this out 😉

@DHowett DHowett merged commit f04ebbe into main Jul 22, 2022
@DHowett DHowett deleted the dev/cazamor/spec-ks/mark-mode branch July 22, 2022 23:08
@DHowett
Copy link
Member

DHowett commented Jul 22, 2022

@DHowett cmon man you know you wanna close this out 😉

I sure did.

@microsoft microsoft deleted a comment from github-actions bot Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Docs It's a documentation issue that really should be on MicrosoftDocs/Console-Docs Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants