-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is needed to keep track of which row is which outside of the class
This allows the application to take full control of the selection
This makes the context menu in macOS embed the right text
This was referenced Jun 9, 2017
Closed
Closed
This was referenced Jun 13, 2017
3 tasks
This was referenced Jul 5, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
IE v11, Windows(demo broken atm Demo doesn't work in IE11 #684)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)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
Follow ups
Fixes #207
Related microsoft/vscode#9958