editor: Fix DeleteToPreviousWordStart and DeleteToNextWordEnd interaction with newlines#16848
Conversation
…previous line Added the action DeleteToPreviousWordStartIgnoringNewlines for situations in which the old behaviour might be desired.
|
Can we add another test into editor_tests.rs? |
…s_word_start_or_newline This reverts the modification I made to the function movement::previous_word_start. This way we avoid the side-effects this commit caused for the actions SelectToPreviousWordStart and MoveToPreviousWordStart. Also the function names better represent the outcome.
|
Feedback is welcome. If this commit gets merged I would be open to also tackle a similar issue with DeleteToNextWordEnd to match the behavior of VSCode/Sublime. This Issue was raised in #14389 which also contains a video showing the problem in that particular case. |
I don't really think it is personally, I'd prefer to just fix the bug and remove the old behavior entirely. cc: @SomeoneToIgnore, lmk if you disagree :) Otherwise, this PR looks great! |
|
This PR also wants to fix this issue: ref: #16963 |
There was a problem hiding this comment.
I would personally add this as a parameter into the DeleteToPreviousWordStart, there could be a
struct DeleteToPreviousWordStart {
include_newlines: bool,
}definition, and its default include_newlines = false will keep the current behavior for everyone.
But if it already looks nice to everybody, why not merge it.
Any follow-ups to make it more ergonomic are good too.
I'll leave the decision to Mikayla, as the reviewer with most context here.
|
(edited) You could merge both PR's as the behavior of the commands will differ slightly, but It might add confusion when there are so many variants of this command to choose from. I would rather merge my PR after I remove the extra command keeping the old behavior and instead add a flag which would activate the old behavior ignoring the newlines (opposed to activating the new behavior via the flag). If the old behaviour is indeed coming from vim, as the author of the other PR says, we should then update the default keymap for vim to use the flag. |
… on DeleteToPreviousWordStart
|
I believe this PR is now ready for merge with the implemented changes.
I would appreciate any feedback. |
SomeoneToIgnore
left a comment
There was a problem hiding this comment.
Sorry for the delay, I hoped for @mikayla-maki to step in, but let's merge this: for me, this looks fine and all my feedback is incorporated.
Thank you for working on this!
This comment was marked as outdated.
This comment was marked as outdated.
…nteraction with newlines (#46235) Closes #40110 Changes: - `DeleteToPreviousSubwordStart` now deletes `\n` separately from preceding subwords and whitespace. - `DeleteToNextSubwordEnd` now deletes `\n` and any following whitespace separately from subsequent subwords. - Added an `ignore_newlines` flag to both actions to optionally retain the old behavior. These modifications align the subword commands with their word counterparts and with other popular editors like VSCode and Sublime. Related to: #16848 Release Notes: - Improved `DeleteToPreviousSubwordStart` and `DeleteToNextSubwordEnd` interactions around newlines. You can opt-in into the previous behavior by adding `{"ignore_newlines": true}` to either action's binds in your keymap. --- This is my first contribution to Zed! If anything should be done differently, please let me know. Happy to learn :)
…nteraction with newlines (zed-industries#46235) Closes zed-industries#40110 Changes: - `DeleteToPreviousSubwordStart` now deletes `\n` separately from preceding subwords and whitespace. - `DeleteToNextSubwordEnd` now deletes `\n` and any following whitespace separately from subsequent subwords. - Added an `ignore_newlines` flag to both actions to optionally retain the old behavior. These modifications align the subword commands with their word counterparts and with other popular editors like VSCode and Sublime. Related to: zed-industries#16848 Release Notes: - Improved `DeleteToPreviousSubwordStart` and `DeleteToNextSubwordEnd` interactions around newlines. You can opt-in into the previous behavior by adding `{"ignore_newlines": true}` to either action's binds in your keymap. --- This is my first contribution to Zed! If anything should be done differently, please let me know. Happy to learn :)
Closes #5285, #14389
Changes:
DeleteToPreviousWordStartnow deletes '\n' separately from preceding words and whitespace.DeleteToNextWordEndnow deletes '\n' and any following whitespace separately from subsequent words.ignore_newlinesflag to both actions to optionally retain the old behavior.These modifications align the behavior more closely with other popular editors like VSCode and Sublime:
DeleteToPreviousWordStartnow matches the default <Ctrl+Backspace> action in those editors.DeleteToNextWordEndbecomes more intuitive and closely resembles the default <Ctrl+Delete> behavior in those editors.Release Notes:
DeleteToPreviousWordStartandDeleteToNextWordEndinteractions around newlines. You can opt-in into the previous behavior by adding {"ignore_newlines": true} to either action's binds in your keymap. (#5285, #14389)