Fix zsh/bash completions for arguments in option groups with a custom completion#648
Conversation
This resolves an issue where custom completion for arguments in an OptionGroup would fail to match the argument. It was caused by: - the completion script only using the name of the argument (instead of the full path) - the CommandParser looking for the matching argument by comparing a name only IndexKey with the “full” IndexKeys
The zsh completions already uses this function. The function’s implementation is the same as what the BashCompletionsGenerator is doing. This removes the duplicated logic.
natecook1000
left a comment
There was a problem hiding this comment.
Looks good, thanks for working on this! One nit, then we should be good to merge.
Have you had a chance to see if this behavior occurs with fish completions as well?
Prevously was using .components(seperatedBy:) from Foundation.
👍 That's been pushed to the branch.
From what I can tell, it appears that completions for arguments aren't being generated currently for fish. This is returning
It'll take me some more time see if there's a reason for the condition in the guard and what the solution would be. |
CraigSiemens
left a comment
There was a problem hiding this comment.
I've added a commit that adds the arguments to the fish completions. I tried implementing the logic to follow the bash/zsh implementations to control which arguments are shown/hidden.
| end | ||
|
|
||
| complete -c math -n \'_swift_math_using_command \"math add\"\' -l hex-output -s x -d \'Use hexadecimal notation for the result.\' | ||
| complete -c math -n \'_swift_math_using_command \"math add\"\' -l version -d \'Show the version.\' |
There was a problem hiding this comment.
The completions now include the --version flag like the bash/zsh commands.
| complete -c math -n \'_swift_math_using_command \"math stats\" \"average stdev quantiles help\"\' -f -a \'average\' -d \'Print the average of the values.\' | ||
| complete -c math -n \'_swift_math_using_command \"math stats\" \"average stdev quantiles help\"\' -f -a \'stdev\' -d \'Print the standard deviation of the values.\' | ||
| complete -c math -n \'_swift_math_using_command \"math stats\" \"average stdev quantiles help\"\' -f -a \'quantiles\' -d \'Print the quantiles of the values (TBD).\' | ||
| complete -c math -n \'_swift_math_using_command \"math stats\" \"average stdev quantiles help\"\' -f -a \'help\' -d \'Show subcommand help information.\' |
There was a problem hiding this comment.
The changes seems to have also fixed another bug where it could suggest help following subcommands when it isn't a valid subcommand.
|
@swift-ci Please test |
Issue
Description
The issue
It was caused by a mismatch in the completion scripts and the custom completion parsing. This is following the example from the issue above.
The completion scripts contained the name of the argument
':arg-in-option-group:{_custom_completion $_completion_example_commandname ---completion -- argInOptionGroup $words}'The parsing of the completion command failed to find the argument using
ArgumentSet.firstPositional(named:).This was caused by the function creating an
InputKeywith just anameand comparing to keys in the set that have theirnameandpathset.In other words
There's also another issue that would occur when a command and an option group each contain an argument with the same name. In that case the completion would always match the first one in the
ArgumentSeteven if the user is trying to get completions for the second argument.The fix
The completions scripts were updated to contain the full path of the argument instead of just the name. Then the completions parser can create an
InputKeythat will exactly match one in theArgumentSet.To avoid having the logic spread out, an extension was added to
InputKeyto get thefullPathStringfrom a key and an init to create anInputKeyfrom afullPathString.I also updated the bash completion script to use
ArgumentDefinition.customCompletionCall(_:)that the zsh script was already using to deduplicate the logic.Checklist