Improve consistency of repeat_last_motion #14478
Conversation
Depending on the motion save to Editor.last_motion, the current Mode of the editor may or may not be observed. Updated all saved last_motions to obey to current state of the editor where appropriate. Fixes helix-editor#14476
|
|
||
| fn find_next_char(cx: &mut Context) { | ||
| find_char(cx, Direction::Forward, true, false) | ||
| find_char(cx, Direction::Forward, true) |
There was a problem hiding this comment.
Hmm this sort of breaks the methods: find and extend now are identical and behave implicitly according to the mode. The whole point was to have two distinct commands so the behavior is more predictable when writing macros
There was a problem hiding this comment.
Yeah, that does stick out like sore thumb. I flip flopped a bit on what I thought made sense here. Options that occurred to me are:
- the extend flavors first explicitly enter select mode. If you are already in select mode, then this is a no-op.
- the extend flavors get dropped, and macros are expected to explicitly enter a state, or operate within the existing state.
- The extend flavors stick around a human readable name, but with no real effect.
IMO the most consistent thing to do is option 2 above, but it will also break custom key maps which is why I left option 3 in place.
|
Also see #12884 - we can add additional parameters to the motion closure like whether to move or extend |
My first cut of this change worked that way, but without significant extra work it wouldn't make motion consistent with select mode. I'm new here so take this with a grain of salt. It seems to me like motions most naturally shouldn't consider their mode at all. Rather, they should move or create cursors and it should be up to the mode if that extends or not. As implemented most of the motions follow this pattern, eg IMO either all motions are changed so that they explicitly extend, or all motions extend based on the mode. If all motions were explicit, that takes away the point of a modality. The default key bindings agree here. I could see value in a follow up change where you could make key binds that could be bookended with a mode change. Eg a binding where for the duration of that binding you were in normal mode. |
|
Btw, I recognize this change needs a doc update. If some form of this PR is accepted I will update the docs too in the same PR. |
Depending on the motion save to Editor.last_motion, the current Mode of the editor may or may not be observed. Updated all saved last_motions to obey to current state of the editor where appropriate.
Fixes #14476