Skip to content

Bugfix: don't round to nearest cell for mouse movements and VT mouse mode#18602

Merged
DHowett merged 1 commit intomainfrom
dev/cazamor/selection/bugfix-round
Feb 26, 2025
Merged

Bugfix: don't round to nearest cell for mouse movements and VT mouse mode#18602
DHowett merged 1 commit intomainfrom
dev/cazamor/selection/bugfix-round

Conversation

@carlos-zamora
Copy link
Member

Summary of the Pull Request

Missed a few _getTerminalPosition() on the first run. Disabled rounding for pointer movements and mouse wheel events (which are used for hyperlink hover detection and vt mouse mode). The only time we round now is...

  • SetEndSelectionPoint() --> because we're updating a selection
  • ControlCore->LeftClickOnTerminal() --> where all paths are used for selection*

*the only path that doesn't is RepositionCursorWithMouse being enabled, which also makes sense based on clicking around Notepad with a large font size.

References and Relevant Issues

Follow-up for #18486
Closes #18595

Validation Steps Performed

In large font size, play around with midnight commander and hover over hyperlink edges.

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-Interaction Interacting with the vintage console window (as opposed to driving via API or hooks) Product-Terminal The new Windows Terminal. Impact-Correctness It be wrong. Impact-Visual It look bad. labels Feb 20, 2025
const bool pointerPressedInBounds)
{
const auto terminalPosition = _getTerminalPosition(til::point{ pixelPosition }, true);
const auto terminalPosition = _getTerminalPosition(til::point{ pixelPosition }, false);
Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see why this is confusing. I was wondering how this change didn't impact selection. This variable is defined early and used late.... and selection ignores it. Wow.

@carlos-zamora carlos-zamora added the Needs-Second It's a PR that needs another sign-off label Feb 21, 2025
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Second It's a PR that needs another sign-off label Feb 26, 2025
@DHowett DHowett merged commit e5b972a into main Feb 26, 2025
19 checks passed
@DHowett DHowett deleted the dev/cazamor/selection/bugfix-round branch February 26, 2025 18:51
DHowett pushed a commit that referenced this pull request Feb 26, 2025
…mode (#18602)

Missed a few `_getTerminalPosition()` on the first run. Disabled
rounding for pointer movements and mouse wheel events (which are used
for hyperlink hover detection and vt mouse mode). The only time we round
now is...
- `SetEndSelectionPoint()` --> because we're updating a selection
- `ControlCore->LeftClickOnTerminal()` --> where all paths are used for
selection*

*the only path that doesn't is `RepositionCursorWithMouse` being
enabled, which also makes sense based on clicking around Notepad with a
large font size.

## References and Relevant Issues
Follow-up for #18486
Closes #18595

## Validation Steps Performed
In large font size, play around with midnight commander and hover over
hyperlink edges.

(cherry picked from commit e5b972a)
Service-Card-Id: PVTI_lADOAF3p4s4AxadtzgXgv1c
Service-Version: 1.23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Interaction Interacting with the vintage console window (as opposed to driving via API or hooks) Impact-Correctness It be wrong. Impact-Visual It look bad. Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All URL hovers (OSC 8 and autodetected) are off by 0.5 cells

3 participants