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

Reimplement selection in the terminal #670

Merged
merged 64 commits into from
Jun 9, 2017

Conversation

Tyriar
Copy link
Member

@Tyriar Tyriar commented May 21, 2017

This change re-implements the selection model within the terminal, taking the responsibility away from the browser and into xterm.js. This may seem like a lot of work to replace something we already get for free, but selection in the browser was never meant to work with the unique rendering model of xterm.js and other quirks with terminals. As such, this implementation will fix a bunch of bugs, probably most notably is the ability to select and copy more than a single viewport worth of data.

Checklist

  • Selection
    • Drag selection works within viewport
      • Works with wide chars
    • Drag selection on bounds of viewport scrolls the buffer
    • Double click
      • Selects a word
      • Selects whitespace
      • Works with wide chars
    • Triple click selects a line
      • Works with wide chars
      • Dragging selects multiple full lines
    • Double and triple clicks allow the selection to be expanded
    • Selection is disabled when mouse events are enabled
    • Ensure the selection is being cleared when appropriate (eg. if a line is removed in the middle of the selection)
    • Select all
      • Basic implementation
      • cmd+a works
    • Zoom
    • Shift+click should expand selection when there is already a selection (send to terminal as normal if there is no selection)
  • Copy
    • Selection can be copied to clipboard via cmd/ctrl+c
    • Selection can be copied to clipboard via context menu
    • Works with wide chars
    • Alt buffer
      • Copying in the alt buffer will grab from the alt buffer, not the normal buffer
  • Paste via context menu
  • API
    • There is sufficient API to reimplement copy outside of xterm.js (eg. for custom hotkeys in Electron)
  • Testing
    • Ensure there is sufficient test coverage
    • Ensure selection works within tmux
    • Ensure copy works within tmux
    • Do some stress testing
    • Test several browsers
  • Polish/clean up
    • Resolve or accept all TODOs
    • Determine whether to keep or discard CircularList id (removed)
    • See if the cursor state can remain static when a selection is happening (as opposed to unfocused) (deferred to Improve focus/blur state #681)
    • Go through related issues and link them to this PR
    • Update PR write up (will defer write up of features to the release notes)

Linked issues

Below is a list of items that this change may or may not fix, originally composed on microsoft/vscode#9958. These items may or may not get resolved depending on how the implementation pans out, but checking them off here will make it easier to track what's changing:

To help reduce the diff I pulled out some of the changes into smaller PRs: #669, #668, #667

Known issues

  • Select all via context menu doesn't work in Edge/Chrome

Follow ups


Fixes #207
Related microsoft/vscode#9958

@Tyriar Tyriar added the work-in-progress Do not merge label May 21, 2017
@Tyriar Tyriar self-assigned this May 21, 2017
@Tyriar Tyriar removed the work-in-progress Do not merge label Jun 9, 2017
@Tyriar Tyriar added this to the 2.8.0 milestone Jun 9, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 69.497% when pulling 6075498 on Tyriar:207_selection_manager into 4b72791 on sourcelair:master.

@Tyriar Tyriar merged commit a889fef into xtermjs:master Jun 9, 2017
@Tyriar Tyriar deleted the 207_selection_manager branch June 9, 2017 20:54
@mofux mofux mentioned this pull request Jul 11, 2017
@weihanglo weihanglo mentioned this pull request Oct 10, 2017
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement virtual selection
4 participants