Improved update selections #306
Conversation
| getSelectionInfo( | ||
| editor.document, | ||
| originalSelection.selection.selection, | ||
| DecorationRangeBehavior.OpenOpen |
There was a problem hiding this comment.
Just specify here that the target range is open on both ends and it will then automatically expand to include inserted text
| editor.selections, | ||
| targets.map((target) => target.selection.selection), | ||
| ]); | ||
| const delimiterSelectionInfos: FullSelectionInfo[] = |
There was a problem hiding this comment.
I think this file turned out really nicely. Notice how we now just give a few ranges to the selection updater and it takes care of all the complex logic:
- For the delimiters that we'll be highlighting, we give it an empty range on the left, that is open at the left, so that it will expand to include the left delimiter, but not the right delimiter (in the case that we're wrapping an empty range). We give it an empty range on the right that is open on the right for the right delimiter highlight
|
|
||
| editor.selections = updatedOriginalSelections; | ||
| const cursorSelectionInfos = editor.selections.map((selection) => |
There was a problem hiding this comment.
- For the new cursor selection, we give it a closed range so it won't include delimiters
| end.translate({ characterDelta: right.length }) | ||
| ), | ||
| ] | ||
| const thatMarkSelectionInfos = targets.map( |
There was a problem hiding this comment.
For the that mark we give it same range as cursor, but make it open so that it will include both delimiters
| target.selection.selection.end | ||
| ), | ||
| range: end, | ||
| isReplace: true, |
There was a problem hiding this comment.
We use isReplace to signal that it shouldn't push empty selections to the right
| default.w: | ||
| start: {line: 0, character: 0} | ||
| end: {line: 0, character: 0} |
There was a problem hiding this comment.
Notice how clobbering the range just leaves the mark at the beginning of the deleted range
| default.w: | ||
| start: {line: 0, character: 2} | ||
| end: {line: 0, character: 5} |
There was a problem hiding this comment.
This behaviour is what we agreed upon in discussion: although technically the whale token has joined with the harp token, we preserve only the part of whale that remains; we don't join it with harp
| default.w: | ||
| start: {line: 0, character: 3} | ||
| end: {line: 0, character: 8} |
There was a problem hiding this comment.
This test is kinda bananas, but basically we've deleted first 2 characters of whale, then added stuff with a space in it to the end of whale. We preserve those last 3 chars of whale and pick up the test that was added until the space on the end! 😅
| @@ -0,0 +1,35 @@ | |||
| spokenForm: bring fine | |||
There was a problem hiding this comment.
The tests in this directory are all selection based, because we now control cursor in response to a bring. For example, in this one we check that the selection is moved to the right
| selections: | ||
| - anchor: {line: 0, character: 9} | ||
| active: {line: 0, character: 9} |
There was a problem hiding this comment.
I think this behaviour is kinda cool. If we say "bring air after this", then the cursor doesn't move the way it usually does when you say "bring air"
pokey
force-pushed
the
improved-update-selections
branch
from
1b4f769
to
2f591fc
Compare
This pull request is a complete rework of the way that we update ranges in response to document changes. There is now a
rangeUpdatercomponent on thegraphwith which you can register ranges that you'd like it to keep up-to-date.Closes #138
Closes #298