Conversation
|
Thanks for putting this up. I should've clarified on the issue, but I wanted to not only add a substring match algorithm but also remove |
ysthakur
left a comment
There was a problem hiding this comment.
I haven't tried running this yet or checked if we need more thorough tests, but this looks like pretty good so far.
Make sure to update doc_config.nu so that users can see what algorithms are available and how sorting works with substring.
Very small nitpick: you currently have <code for prefix stuff>, <code for fuzzy stuff>, <code for substring stuff> everywhere. I think it would look a little nicer if the order was instead prefix, substring, fuzzy, since prefix and substring matching are pretty similar.
Would also be nice to reduce the code duplication in completion_options a teeny bit, but it's not a huge deal, especially if removing duplication ends up complicating the code more.
ysthakur
left a comment
There was a problem hiding this comment.
I just tried this out, and it works great. Just need to change the logging a bit.
Also, this comment isn't necessary anymore:
| completion_options.positional = positional; | ||
| let positional = | ||
| options.get("positional").and_then(|val| val.as_bool().ok()); | ||
| if let Some(_positional) = positional { |
There was a problem hiding this comment.
Try this instead:
| if let Some(_positional) = positional { | |
| if positional.is_some() { |
|
Thanks, let's try this out |
Description
This PR should close #15474 .
User-Facing Changes
When users set the match algorithm to 'substring' by modifying
$env.configto$env.config.completions.algorithm = "substring"``), completions are done based on substring matches. This was previously possible by settingpositional` to be false in custom completers, but doing so now logs a warning as this feature is set to be deprecated and replaced by the new way of setting the matching algorithm to substring based.