-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
keep (cursor) position when exactly replacing text #5930
base: master
Are you sure you want to change the base?
keep (cursor) position when exactly replacing text #5930
Conversation
I was not 100% sure if this is the right behavior for diagnostics. Initally I thaught this was the case but on second thaught I came to the conclusion that this behavior is only disable for selections. Exact replacements are pretty rare to begin with and basically always intentional by the user. |
I think this should be a different assoc type, I didn't port it all from CodeMirror: https://github.com/codemirror/state/blob/2117f5b58d040284f1e04ec054a1f2c79bf86a30/src/change.ts#L5-L16 |
Various usages here https://github.com/search?q=org%3Acodemirror+MapMode&type=code |
83ac2da
to
d76e361
Compare
I added the However none of the variants from the source you linked @archseer match the behavior I implemented here so I had to comeup with my own name. I think this usecase is probably pretty specific to a multi-cursor modal editor so other implementations don't need to handle it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the look of this 👍. I left a minor typo-fix comment and looks like this will need to be rebased on top of the recent optimizations
d76e361
to
c892d2a
Compare
Whenever a document is changed helix maps various positions like the cursor or diagnostics through the `ChangeSet` applied to the document. Currently, this mapping handles replacements as follows: * Move position to the left for `Assoc::Before` (start of selection) * Move position to the right for `Assoc::After` (end of selection) However, when text is exactly replaced this can produce weird results where the cursor is moved when it shouldn't. For example if `foo` is selected and a separate cursor is placed on each character (`s.<ret>`) and the text is replaced (for example `rx`) then the cursors are moved to the side instead of remaining in place. This change adds a special case to the mapping code of replacements: If the deleted and inserted text have the same (char) length then the position is returned as if the replacement doesn't exist. only keep selections invariant under replacement Keeping selections unchanged if they are inside an exact replacement is intuitive. However, for diagnostics this is not desirable as helix would otherwise fail to remove diagnostics if replacing parts of the document.
c892d2a
to
cea759f
Compare
Fixes #5910
Whenever a document is changed helix maps various positions like the
cursor or diagnostics through the
ChangeSet
applied to the document.Currently, this mapping handles replacements as follows:
Assoc::Before
(start of selection)Assoc::After
(end of selection)However, when text is exactly replaced this can produce weird results
where the cursor is moved when it shouldn't. For example if
foo
isselected and a separate cursor is placed on each character (
s.<ret>
)and the text is replaced (for example
rx
) then the cursors are movedto the side instead of remaining in place.
This change adds a special case to the mapping code of replacements:
If the deleted and inserted text have the same (char) length then
the position is returned as if the replacement doesn't exist.