custom comp: do not complete flags after args when interspersed is false #1308
Conversation
There was a problem hiding this comment.
This is really nice.
I have mentioned in the comments two more problematic cases, but they are corner cases, especially the one about the args being incomplete.
I don't know if supporting these cases is worth the possible extra complexity to the code, but I haven't tried to fix it myself, so I'm not sure. I personally would not consider them blockers as this PR already improves this scenario for the vast majority of cases.
Thanks for the continuing great work @Luap99 !
Luap99
force-pushed
the
fix-1307
branch
2 times, most recently
from
ab9afbc
to
9805c41
Compare
custom_completions_test.go
Outdated
| childCmd.ValidArgsFunction = func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) { | ||
| return args, ShellCompDirectiveDefault | ||
| } | ||
| output, err = executeCommand(rootCmd, ShellCompRequestCmd, "child", "--", "--validarg", "") |
There was a problem hiding this comment.
We have to use a valid flag for this test, which will then trigger the problem:
output, err = executeCommand(rootCmd, ShellCompRequestCmd, "child", "--", "--string", "")
--validarg is not found as a valid flag by checkIfFlagCompletion() so your new logic handling errors is triggered. However, with --string, there will not be an error as it is a valid flag and the --string argument will be trimmed:
Line 466 in a4ab3fa
But this is such a rare (and possibly invalid) case that I don't think we should fix it (unless it is a very simple fix).
There was a problem hiding this comment.
You are right catching these cases is more difficult and would require a bigger refactor of the checkIfFlagCompletion function.
|
I just rebased to solve the merge conflict. |
|
This PR is being marked as stale due to a long period of inactivity |
|
@jpmcb If you still have energy and time, this is another fix for completion which I had forgotten to mention. This will make the last Fish shell test of the |
If the interspersed option is set false and one arg is already set all following arguments are counted as arg and not parsed as flags. Because of that we should not offer flag completion. The same applies to arguments followed after `--`. Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
|
@jpmcb done |
If the interspersed option is set false and one arg is already set all
following arguments are counted as arg and not parsed as flags. Because
of that we should not offer flag completion. The same applies to arguments
followed after
--.Fixes #1307