apple / swift-argument-parser

Straightforward, type-safe argument parsing for Swift
Apache License 2.0
3.3k stars 311 forks source link

Fix zsh/bash completions for arguments in option groups with a custom completion #648

Closed CraigSiemens closed 1 month ago

CraigSiemens commented 2 months ago

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.

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 ArgumentSet even 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 InputKey that will exactly match one in the ArgumentSet.

To avoid having the logic spread out, an extension was added to InputKey to get the fullPathString from a key and an init to create an InputKey from a fullPathString.

I also updated the bash completion script to use ArgumentDefinition.customCompletionCall(_:) that the zsh script was already using to deduplicate the logic.

Checklist

CraigSiemens commented 2 months ago

Let's use the stdlib method here instead of Foundation's:

👍 That's been pushed to the branch.

Have you had a chance to see if this behavior occurs with fish completions as well?

From what I can tell, it appears that completions for arguments aren't being generated currently for fish.

This is returning nil if the ArgumentDefinition.names is empty. https://github.com/apple/swift-argument-parser/blob/5be327c/Sources/ArgumentParser/Completions/FishCompletionsGenerator.swift#L62-L64

names will always be empty for positional arguments. https://github.com/apple/swift-argument-parser/blob/5be327c/Sources/ArgumentParser/Parsing/ArgumentDefinition.swift#L109-L114

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.

natecook1000 commented 1 month ago

@swift-ci Please test