-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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 for case-insensitive command names #1802
Conversation
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.
Thank you @YuviGold for stepping up and implementing this!
I have a couple of comments but this looks great.
cobra.go
Outdated
const ( | ||
DefaultPrefixMatching = false | ||
DefaultCommandSorting = true | ||
DefaultCaseInsensitive = false |
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.
Are you imagining a scenario where a program would like to access these?
I was thinking a program may want to use the same default as Cobra for case-sensitivity of other things; however, it wouldn't be the constant they should use but the value set in cobra. EnableCaseInsensitive
.
So, if these constants don't need to be exported, let's make them private.
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.
Very true Marc! +1 - I don't expect users to need these constants since they delineate internal default behavior
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.
Agree. Just wanted to make those constants so they can be used in tests where toggling back to default configuration . Casting to private :)
@jpmcb what do you think of aiming to have this in the 1.6 release? |
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.
Overall, I think this is something we can get in the next release - let's address any nits / comments and go for it!
@@ -1263,6 +1263,99 @@ func TestSuggestions(t *testing.T) { | |||
} | |||
} | |||
|
|||
func TestCaseInsensitive(t *testing.T) { |
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.
Hooray tests! 👏🏼
1a47cd5
to
0257d38
Compare
@marckhouzam @jpmcb |
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.
Could you add a test to make sure defaultCaseInsensitive
is false? That way if a future PR tries to change it, we'll noticed right away that it's not allowed.
Add a global `EnableCaseInsensitive` variable to allow case-insensitive command names. The variable supports commands names and aliases globally. Resolves spf13#1382
0257d38
to
8830648
Compare
@marckhouzam |
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 @YuviGold this looks great!
LGTM
* main: (39 commits) Add '--version' flag to Help output (spf13#1707) Expose ValidateRequiredFlags and ValidateFlagGroups (spf13#1760) Document option to hide the default completion cmd (spf13#1779) ci: add workflow_dispatch (spf13#1387) add missing license headers (spf13#1809) ci: use action/setup-go's cache (spf13#1783) Adjustments to documentation (spf13#1656) Rename Powershell completion tests (spf13#1803) Support for case-insensitive command names (spf13#1802) Deprecate ExactValidArgs() and test combinations of args validators (spf13#1643) Use correct stale action `exempt-` yaml keys (spf13#1800) With go 1.18, we must use go install for a binary (spf13#1726) Clarify SetContext documentation (spf13#1748) ci: test on Golang 1.19 (spf13#1782) fix: show flags that shadow parent persistent flag in child help (spf13#1776) Update gopkg.in/yaml.v2 to gopkg.in/yaml.v3 (spf13#1766) fix(bash-v2): activeHelp length check syntax (spf13#1762) fix: correct command path in see_also for YAML doc (spf13#1771) build(deps): bump github.com/inconshreveable/mousetrap (spf13#1774) docs: add zitadel to the list (spf13#1772) ...
Add a global
EnableCaseInsensitive
variable to allowcase-insensitive command names.
The variable supports commands names and aliases globally.
Resolves #1382