Complete: Allow passing multiple conditions#8967
Conversation
|
I have converted the git completions now (locally). This only helps with the condition, so in case where the argument completion actually needs to do work (by calling git) it won't show up a lot. So e.g. A quick glimpse at a profile shows that it about halves the Note: Rewriting this will, of course, be an issue for the people who try to backport completion scripts to older fishes. I don't believe that's a big problem. We can accomodate them to a degree by not doing backwards-incompatible cleanup, but we shouldn't hold back improvements in correctness or performance for that. |
|
Fixes #8536. Huh, hadn't seen that one |
|
@kidonng Headsup, you'll have to deal with this. |
|
looks great, nice improvement. I wonder if we should add a "please restart fish" message when and old fish loads new completions that are known to be incompatible. Or perhaps we can embed them into the binary.. |
This makes it so `complete -c foo -n test1 -n test2` registers *both* conditions, and when it comes time to check the candidate, tries both, in that order. If any fails it stops, if all succeed the completion is offered. The reason for this is that it helps with caching - we have a condition cache, but conditions like ```fish test (count (commandline -opc)) -ge 2; and contains -- (commandline -opc)[2] length test (count (commandline -opc)) -ge 2; and contains -- (commandline -opc)[2] sub ``` defeats it pretty easily, because the cache only looks at the entire script as a string - it can't tell that the first `test` is the same in both. So this means we separate it into ```fish complete -f -c string -n "test (count (commandline -opc)) -ge 2; and contains -- (commandline -opc)[2] length" -s V -l visible -d "Use the visible width, excluding escape sequences" +complete -f -c string -n "test (count (commandline -opc)) -ge 2" -n "contains -- (commandline -opc)[2] length" -s V -l visible -d "Use the visible width, excluding escape sequences" ``` which allows the `test` to be cached. In tests, this improves performance for the string completions by 30% by reducing all the redundant `test` calls. The `git` completions can also greatly benefit from this.
Tbh you should always restart fish after updating(*). We reload changed function files! (which I still believe we should stop doing, but even if we did autoloading means you get the new files if you do something new anyway) (*) "updating" here refers to actual version upgrades from 3.3 to 3.4, not a few commits at a time like we do |
Description
This makes it so
complete -c foo -n test1 -n test2registers bothconditions (instead of using only test2, as is the case at the moment).
When it comes time to check the candidate, both conditions are attempted,
in that order. If any fails it stops, if all succeed the completion is offered.
The reason for this is that it helps with caching - we have a
condition cache, but conditions like
fish-shell/share/completions/string.fish
Lines 51 to 58 in 86ab81d
defeat it pretty easily, because the cache only looks at the entire
script as a string - it can't tell that the first
testis the same.So this will run
test (count (commandline -opc)) -lt 2once, because it's cached, and runonce each - so the
testis run at least twice.So this means we separate it into two conditions, like
which allows the
testto be cached.The entire string completions (which are quite simple) have a total of 3 distinct tests - less than two (and offer a subcommand), greater or equal to two (and offer options) and less or equal to two (and offer --help). So if, say there already are three arguments the le and lt tests only need to be done once each, instead of 17 times. If there are less than two, the gt test only needs to run once instead of 31 times.
In tests, this improves performance for the string completions by 60%
by reducing all the redundant
testcalls.The
gitcompletions can also greatly benefit from this.A slight downside is, of course, that the completions need to be altered to take advantage of this.
TODOs: