-
Notifications
You must be signed in to change notification settings - Fork 4
feat: add completion install api #18
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
Conversation
} | ||
|
||
func DetectUserShell() (string, error) { | ||
func DetectUserShell(programName string) (Shell, error) { |
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.
This feels a bit awkward, but we need to provide it to use in the template itself, e.g. function _name
, and potentially in the install path, e.g. name.fish
, and it's much nicer for this function to return a Shell
, instead of a string that you just have to pass back to ShellByName
.
Alternatively, InstallPath
and WriteCompletion
could take it as an argument, but if you call one you're probably going to call the other, so I thought it'd be better to just store it.
30daf72
to
802129c
Compare
WriteCompletion(io.Writer) error | ||
ProgramName() string |
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.
Kind of annoying, but we need the program name during InstallCompletion
to figure out the exact header we're looking for - otherwise you obviously couldn't have multiple serpent
completion scripts installed.
@@ -352,7 +352,7 @@ func (optSet OptionSet) ByFlag(flag string) *Option { | |||
} | |||
for i := range optSet { | |||
opt := &optSet[i] | |||
if opt.Flag == flag || opt.FlagShorthand == flag { |
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.
These changes shouldn't really need reviewing but it's worth mentioning pflag
only lets you use the shorthand with a single hyphen, so I'm getting rid of the completion for now.
Closes #16.
Adds a way for API consumers to receive a recommended install path for the shell script, and to install or update the script at the path. This is done by exposing a
Shell
interface, which can be detected, or retrieved using the name of the shell.Adds a new
EnumArray
Flag value type, to enable use cases where multiple enum values can be supplied.In
coder/coder
this is the--output=table
--column
flags.This also makes the help formatting for these commands more consistent with
Enum
flags, where the possible values are stored on the type, instead of in the description. The type is represented as[Val1|Val2|...]
. For reference, regular enums areVal1|Val2|...
.Fixes shell scripts not handling completions with spaces. In
coder/coder
column names (the argument to--column
flags) often have spaces.Made Enum and EnumArray flag arguments now case-insensitive (using
strings.EqualFold
).