Actually split mi and ma into several subcommands
#1723
Conversation
|
Note: I have some simplifications of the |
|
Ended up rewriting the changes. The general design is pretty much the same, except some of the command palette |
| "a" => { "Select around" | ||
| "w" => select_around_word, | ||
| "W" => select_around_long_word, | ||
| "c" => select_around_class, | ||
| "f" => select_around_function, | ||
| "a" => select_around_parameter, | ||
| "m" => select_around_cursor_pair, | ||
| _ => select_around_pair, | ||
| }, | ||
| "i" => { "Select inside" | ||
| "w" => select_inside_word, | ||
| "W" => select_inside_long_word, | ||
| "c" => select_inside_class, | ||
| "f" => select_inside_function, | ||
| "a" => select_inside_parameter, | ||
| "m" => select_inside_cursor_pair, | ||
| _ => select_inside_pair, | ||
| }, |
There was a problem hiding this comment.
Nice, I was looking into doing something like this.
| pub struct FallbackCommand { | ||
| pub name: &'static str, | ||
| pub fun: fn(cx: &mut Context, event: KeyEvent), | ||
| pub with_prompt: MappableCommand, |
There was a problem hiding this comment.
Why do we need with_prompt?
There was a problem hiding this comment.
For use in reverse_map, so that the binding for the fallback command shows up for its prompting version in the command palette. I suppose it could be left out, but I think the binding should really show up in the palette. It could also be just the name instead, since that's what ultimately ends up getting used.
At one point, I tried to come up with a way to have just a prompt message that'd allow for the "generation" of an appropriately structured function, but I couldn't figure that out. Doing that might require a third MappableCommand variant, and I'm not sure that's super worth looking into.
There was a problem hiding this comment.
Is it possible not have it behaves like a command without needing any prompt? I don't see why we need to prompt here given that it can be used like any other key already. At least in my own opinion, the prompt could be a separate thing.
| impl std::str::FromStr for FallbackCommand { | ||
| type Err = anyhow::Error; | ||
|
|
||
| fn from_str(s: &str) -> Result<Self, Self::Err> { | ||
| FallbackCommand::FALLBACK_COMMAND_LIST | ||
| .iter() | ||
| .find(|cmd| cmd.name() == s) | ||
| .cloned() | ||
| .ok_or_else(|| anyhow!("No command named '{}'", s)) | ||
| } | ||
| } | ||
|
|
||
| impl<'de> Deserialize<'de> for FallbackCommand { | ||
| fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> { | ||
| let s: &'de str = Deserialize::deserialize(deserializer)?; | ||
| s.parse().map_err(de::Error::custom) | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we can pass on deserializing fallback commands from the config for now since that's better supported via scripting in the future.
| enum TextObjectSelector { | ||
| Word(bool), | ||
| Treesitter(&'static str), | ||
| Matching(Option<char>), | ||
| } |
There was a problem hiding this comment.
A separate LongWord variant would be clearer instead of a bool.
| fn select_around_pair(cx: &mut Context, event: KeyEvent) { | ||
| select_pair(cx, textobject::TextObject::Around, event); | ||
| } |
There was a problem hiding this comment.
I'd rather this be called select_around_pair_fallback and prompt_and_select_around_pair be called select_around_pair.
| .find(|component| component.id() == Some(id)) | ||
| .and_then(|component| component.as_any_mut().downcast_mut()) | ||
| .filter(|component| component.id() == Some(id)) | ||
| .find_map(|component| component.as_any_mut().downcast_mut::<T>()) |
There was a problem hiding this comment.
This change seems unnecessary ?
| impl PartialEq for FallbackCommand { | ||
| fn eq(&self, other: &Self) -> bool { | ||
| self.name == other.name | ||
| } | ||
| } |
There was a problem hiding this comment.
Seems more accurate to compare both name and function pointers?
the-mikedavis
added
the
S-waiting-on-author
kirawi
added
A-helix-term
|
would like to still see this altough I would prefer a different approach (that will require much less invasive changes). That said this PR has gone stale with lots of conflicts and still has undressed comments so I am going to close this out. Thank you for contributing! |
Well, this was poorly timed.
Adds a
fallbackfield toKeyTrieNode, used for surround pairs as discussed. For that command to actually work, I added alast_keyfield toContext.Outstanding concerns:
last_keybe onEditorinstead?select_*_pairbe used from the command palette?config.toml?