feat(completion): use cobras default completion command#204
Merged
Conversation
Member
Author
|
I know this is quite hacky, but so is our current bash completion override which prevents us from supporting zsh (#134) and other shell completions. |
peknur
approved these changes
Feb 2, 2023
Contributor
peknur
left a comment
There was a problem hiding this comment.
Looks good. Left non-blocking comment about RE.
Comment on lines
+11
to
+25
| func RemoveWordBreaks(input string) string { | ||
| re := regexp.MustCompile(`\s+`) | ||
| return re.ReplaceAllString(input, "\u00A0") | ||
| } | ||
|
|
||
| // MatchStringPrefix returns a list of string in vals which have a prefix as specified in key. Quotes are removed from key and output strings are escaped according to completion rules | ||
| func MatchStringPrefix(vals []string, key string, caseSensitive bool) []string { | ||
| var r []string | ||
| key = strings.Trim(key, "'\"") | ||
|
|
||
| for _, v := range vals { | ||
| if (caseSensitive && strings.HasPrefix(v, key)) || | ||
| (!caseSensitive && strings.HasPrefix(strings.ToLower(v), strings.ToLower(key))) || | ||
| key == "" { | ||
| r = append(r, Escape(v)) | ||
| r = append(r, RemoveWordBreaks(v)) |
Contributor
There was a problem hiding this comment.
Maybe compile that regular expression before loop and just do re.ReplaceAllString in loop.
Member
Author
There was a problem hiding this comment.
Good idea! Moved the re definition to package-level var.
Member
Author
|
The linting error seems to come from generics usage in node group parameters parsing, I'll try to see if there a route around that |
…cobra This removes custom bash completion logic and custom completion escape logic as that is not supported by completion scripts cobra generates.
41a51e6 to
7da143e
Compare
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This requires removing whitespace from completions as cobras bash completions do not support values with whitespace (See spf13/cobra#1740). This is done by replacing whitespace with non-breaking spaces during completion generation. The argument resolvers with values that can include whitespace are also updated to handle both modified and original arguments, i.e. user can still input whitespaces normally when argument is quoted or escaped.