New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support PathFlag.TakesFile in fish completion #1198
Conversation
Performing reflection on the given flag ensures that fishAddFileFlag will behave correctly if any flag types get a TakesFile field in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting this together!
Left a couple comments with some options for how to go forward. Feel free to head down either path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What type of PR is this?
What this PR does / why we need it:
Flags of type
PathFlag
weren't processed correctly when generating fish completions—the generatedcomplete
command told fish that the flag doesn't take a filename, even whenTakesFile
is true.fish.go
now respects theTakesFile
field, regardless of the flag's underlying type.PathFlag
to the list of explicit types in the switch statement.fish_test.go
has a test case for aPathFlag
withTakesFile = true
.expected-fish-full.fish
has a corresponding change in its fixture data.Which issue(s) this PR fixes:
Fixes #1156
Special notes for your reviewer:
@lynncyrin had suggested a lighter-weight form of reflection, but I wasn't able to make the type assertion work. AFAIK go's type system only deals in interfaces and specific, concrete types.
Release Notes