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

Column selection isn't working correctly #3855

Closed
Tyriar opened this issue Jun 11, 2022 · 4 comments · Fixed by #3860
Closed

Column selection isn't working correctly #3855

Tyriar opened this issue Jun 11, 2022 · 4 comments · Fixed by #3860
Assignees
Labels
area/selection type/bug Something is misbehaving
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Jun 11, 2022

Repro: Alt+click and drag in the terminal

  • Moving right on the same line does not make a selection
  • Moving to top-right or bottom-left doesn't make a selection (this works in canvas renderer only?)
  • Cursor isn't correct

Recording 2022-06-11 at 06 04 10

Correct behavior in VS Code 1.67.0:

Recording 2022-06-11 at 06 07 07

@Tyriar Tyriar added type/bug Something is misbehaving area/selection labels Jun 11, 2022
@Tyriar Tyriar added this to the 4.19.0 milestone Jun 11, 2022
@Tyriar
Copy link
Member Author

Tyriar commented Jun 11, 2022

Should be able to write an api test for this

@silamon
Copy link
Contributor

silamon commented Jun 11, 2022

The "moving to top-right or bottom-left doesn't make a selection" isn't a regression but something that's been in like forever.
For DOM: Seems like the width of the selection element gets negative. Swapping colStart with colEnd in those cases will fix it.

private _createSelectionElement(row: number, colStart: number, colEnd: number, rowCount: number = 1): HTMLElement {
const element = document.createElement('div');
element.style.height = `${rowCount * this.dimensions.actualCellHeight}px`;
element.style.top = `${row * this.dimensions.actualCellHeight}px`;
element.style.left = `${colStart * this.dimensions.actualCellWidth}px`;
element.style.width = `${this.dimensions.actualCellWidth * (colEnd - colStart)}px`;
return element;
}

I couldn't reproduce the other mentioned things in xterm demo, but it definitely exists in vscode. (Notice how alt activates the menu shortcuts in the window bar.)

@Tyriar
Copy link
Member Author

Tyriar commented Jun 11, 2022

The "moving to top-right or bottom-left doesn't make a selection" isn't a regression but something that's been in like forever.

@silamon the gif shows it was working before?

I couldn't reproduce the other mentioned things in xterm demo, but it definitely exists in vscode

Not sure about the corner thing but if it's only in VS Code then chances are it's related to padding around the terminal elements.

@silamon
Copy link
Contributor

silamon commented Jun 12, 2022

I see that the code has changed for WebGL renderer, so you could be correct that it was working for WebGL before.

return x >= this._model.selection.startCol && y >= this._model.selection.viewportCappedStartRow &&
x < this._model.selection.endCol && y < this._model.selection.viewportCappedEndRow;

It could be the same thing that I mentioned for DOM renderer now. startCol is higher than endCol for top-right and bottom-left and this only happens in columnSelectMode.

Tyriar added a commit to Tyriar/xterm.js that referenced this issue Jun 14, 2022
The following parts were not respecting the column selection edge case where the
start column was greater than the end column while the start row was less than the
end row:

- Webgl renderer
- DOM renderer
- Selection service (ie. getSelection(), I don't think this was a
  regression)

Fixes xtermjs#3855
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/selection type/bug Something is misbehaving
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants