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.

Sign up for GitHub

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

Jump to bottom

Implement more modern word-wise cursor navigations #15787

Open
lhecker opened this issue Aug 1, 2023 · 3 comments
Open

Implement more modern word-wise cursor navigations #15787

lhecker opened this issue Aug 1, 2023 · 3 comments
Labels
Area-CookedRead The cmd.exe COOKED_READ handling Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal.
Milestone
Terminal v1.23

Comments

@lhecker
Copy link
Member

lhecker commented Aug 1, 2023 •
edited
Loading

Description of the new feature/enhancement

Conhost and Windows Terminal currently use the classic Windows algorithm, which is also used by Notepad and Visual Studio. Given 3 classes of characters - whitespace, delimiters and regular text - it works something like this:

  • Forward navigation:
    • Skip text until (after) you encounter a different character class
    • Skip whitespace
  • Backward navigation:
    • Skip whitespace
    • Skip text until (before) you encounter a different character class

In comparison Visual Studio Code's navigation works like this:

  • Forward navigation:
    • Skip whitespace
    • Skip 1 glyph
    • Skip text until (before) you encounter a different character class
  • Backward navigation:
    • Skip whitespace
    • Skip 1 glyph
    • Skip text until (before) you encounter a different character class

Due to this symmetry navigating forward/backward will produce consistent and identical results. It'll feel more "intuitive" to new users.

Risk

People can be very picky about changes to our word selection algorithm. Double-clicking words needs to behave similar/identical to before. Cursor navigation needs to be at least so close that no one will feel bothered by it. There should be an opt-out if not at least via a registry key, just in case things go bad.
@lhecker lhecker added Product-Conhost For issues in the Console codebase Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. Area-CookedRead The cmd.exe COOKED_READ handling labels Aug 1, 2023
@carlos-zamora carlos-zamora removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 2, 2023
@carlos-zamora carlos-zamora added this to the Backlog milestone Aug 2, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Aug 2, 2023
@chrisant996
Copy link

@lhecker This is referring only to processed input mode in ReadConsole(), right?

@DHowett
Copy link
Member

DHowett commented Aug 3, 2023

Yup!

@zadjii-msft zadjii-msft mentioned this issue Aug 17, 2023
Merged
@carlos-zamora
Copy link
Member

Screen readers also use the other kind of word navigation. So, when a user is navigating the buffer, it also feels really different and confusing.

@carlos-zamora carlos-zamora modified the milestones: Backlog, Terminal v1.23 Sep 10, 2024
@carlos-zamora carlos-zamora mentioned this issue Sep 11, 2024
Closed
@carlos-zamora carlos-zamora mentioned this issue Oct 23, 2024
Merged
1 task
DHowett pushed a commit that referenced this issue Jan 28, 2025
@carlos-zamora
Make selection an exclusive range (#18106)
64d4fba 
Selection is generally stored as an inclusive start and end. This PR
makes the end exclusive which now allows degenerate selections, namely
in mark mode. This also modifies mouse selection to round to the nearest
cell boundary (see #5099) and improves word boundaries to be a bit more
modern and make sense for degenerate selections (similar to #15787).

Closes #5099
Closes #13447
Closes #17892

## Detailed Description of the Pull Request / Additional comments
- Buffer, Viewport, and Point
- Introduced a few new functions here to find word boundaries, delimiter
class runs, and glyph boundaries.
- 📝These new functions should be able to replace a few other functions
(i.e. `GetWordStart` --> `GetWordStart2`). That migration is going to be
a part of #4423 to reduce the risk of breaking UIA.
- Viewport: added a few functions to handle navigating the _exclusive_
bounds (namely allowing RightExclusive as a position for buffer
coordinates). This is important for selection to be able to highlight
the entire line.
- 📝`BottomInclusiveRightExclusive()` will replace `EndExclusive` in the
UIA code
- Point: `iterate_rows_exclusive` is similar to `iterate_rows`, except
it has handling for RightExclusive
- Renderer
- Use `iterate_rows_exclusive` for proper handling (this actually fixed
a lot of our issues)
- Remove some workarounds in `_drawHighlighted` (this is a boundary
where we got inclusive coords and made them exclusive, but now we don't
need that!)
- Terminal
   - fix selection marker rendering
- `_ConvertToBufferCell()`: add a param to allow for RightExclusive or
clamp it to RightInclusive (original behavior). Both are useful!
- Use new `GetWordStart2` and `GetWordEnd2` to improve word boundaries
and make them feel right now that the selection an exclusive range.
- Convert a few `IsInBounds` --> `IsInExclusiveBounds` for safety and
correctness
   - Add `TriggerSelection` to `SelectNewRegion`
- 📝 We normally called `TriggerSelection` in a different layer, but it
turns out, UIA's `Select` function wouldn't actually update the
renderer. Whoops! This fixes that.
- TermControl
- `_getTerminalPosition` now has a new param to round to the nearest
cell (see #5099)
- UIA
- `TermControlUIAProvider::GetSelectionRange` no need to convert from
inclusive range to exclusive range anymore!
- `TextBuffer::GetPlainText` now works on an exclusive range, so no need
to convert the range anymore!

## Validation Steps Performed
This fundamental change impacts a lot of scenarios:
- ✅Rendering selections
- ✅Selection markers
- ✅Copy text
- ✅Session restore
- ✅Mark mode navigation (i.e. character, word, line, buffer)
- ✅Mouse selection (i.e. click+drag, shift+click, multi-click,
alt+click)
- ✅Hyperlinks (interaction and rendering)
- ✅Accessibility (i.e. get selection, movement, text extraction,
selecting text)
- [ ] Prev/Next Command/Output (untested)
- ✅Unit tests

## Follow-ups
- Refs #4423
- Now that selection and UIA are both exclusive ranges, it should be a
lot easier to deduplicate code between selection and UIA. We should be
able to remove `EndExclusive` as well when we do that. This'll also be
an opportunity to modernize that code and use more `til` classes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-CookedRead The cmd.exe COOKED_READ handling Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
Terminal v1.23
Development

No branches or pull requests
4 participants
@DHowett @lhecker @carlos-zamora @chrisant996