-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Conversation
Comment from @zadjii-msft in base PR:
Correct. I feel like the default should be 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. |
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 😕 |
There was a problem hiding this 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
@@ -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. | |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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'.
There was a problem hiding this comment.
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
9a4aad7
to
0ff7682
Compare
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 |
There was a problem hiding this 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
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. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
Urgh. Yeah, so there's 2 parts here: anchorMode: ToggleI think where this spec stands, this is fine? You can just use your anchorMode: HoldThe main idea behind ConHost CMD Mark Mode is basically that as long as shift is held, the 'start' endpoint is static/frozen/held.
So, with the logic above, the scenario above might still be acceptable?
Alternative PlanAlternatively, 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 PlanWe 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 |
Could support be added for the following scenario:
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 @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. |
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:
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. |
Yea I'm totally cool with ditching the 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: DetailsWhat if we had a Theoretically, the user could enter mark mode, mode the selection start anchor to (1,1), then toggle the endpoint (with 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" }, |
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. |
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... |
Actually, this would be an extension/keybinding on its own. It's pretty independent. Created issue #6232 for tracking. |
|
be4b35b
to
eac00b2
Compare
The arrows for moveSelectionPoint in the middle are odd... are some missing? are the lines running oddly? |
#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
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? |
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. |
This comment was marked as resolved.
This comment was marked as resolved.
## 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)
## 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
## 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.
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment has been minimized.
Re: #5804 (comment) Apparently "modern" vim also can do way more than that: |
@DHowett cmon man you know you wanna close this out 😉 |
I sure did. |
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