added keep order to bash, zsh, pwsh & fish #1903

h4ck3rk3y · 2023-01-30T19:14:21Z

Closes #1900

CLAassistant · 2023-01-30T19:14:26Z

All committers have signed the CLA.

bash_completions.go

marckhouzam

This is really great!
I haven't gone through the code in detail yet, but the overall change looks great.

Two things I noticed while testing:

zsh seems to order all completions with descriptions first, then the ones without them, even with the -V flag. I don't think it is a big deal, but it does mean it does not totally respect the "keep order" directive. The other shells don't behave like this. Is this ok with your usecase?
When trying fish it looks like the completions are never sorted anymore, even if the new directive is not used. I haven't looked at the code yet to see where things broke.

h4ck3rk3y · 2023-02-07T09:35:24Z

@marckhouzam fish was indeed buggy, my bad. I have fixed it and tried it with the directive set & unset, have another run with it :-)

The zsh behavior works for my use case! We set the -V flag for non description dynamic completions.

marckhouzam

Almost ready. thanks for the efforts @h4ck3rk3y !
Now we get into some of the annoying "supporting older shell" details.

bash_completionsV2.go

powershell_completions.go

fish_completions.go

bash_completionsV2.go

h4ck3rk3y · 2023-02-23T11:16:20Z

@marckhouzam let me know if you get a chance to look at this

marckhouzam · 2023-02-23T11:23:14Z

Sorry for the delay @h4ck3rk3y, I'll do my best to find time this weekend.

marckhouzam

Looks great 🎉
Thanks @h4ck3rk3y

Closes #1900 Merge spf13/cobra#1903

* Create unit test illustrating unknown flag bug (spf13#1854) Created a unit test that tests the unknown flag error message when the unknown flag is located in different arg positions. * Update stale.yml (spf13#1863) * fix: force ForEach-Object to return array in pwsh completion (spf13#1850) Fixes spf13#1847 * Makefile: add target richtest (spf13#1865) Don't require contributors to install richgo but keep it as an option and for CI * build(deps): bump golangci/golangci-lint-action from 3.2.0 to 3.3.1 (spf13#1851) Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 3.2.0 to 3.3.1. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](golangci/golangci-lint-action@v3.2.0...v3.3.1) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Update kubescape org (spf13#1874) Signed-off-by: David Wertenteil <[email protected]> * ci: deprecate go 1.15 (spf13#1866) Remove testing for go 1.15 to allow CI to pass, but don't force projects to upgrade. * fix: conflict import name with variable (spf13#1879) `template` is an import in `cobra.go` file and also used as a variable name, which masks the library in the scope of that function. * Update badge route (spf13#1884) Based on badges/shields#8671 * fix: func name in doc strings (spf13#1885) Corrected the function name at the start of doc strings, as per the convention outlined in official go documentation: https://go.dev/blog/godoc * completions: do not detect arguments with dash as 2nd char as flag (spf13#1817) Fixes spf13#1816 Previously, arguments with a dash as the second character (e.g., 1-ff00:0:1) were detected as a flag by mistake. This resulted in auto completion misbehaving if such an argument was last in the argument list during invocation. * build(deps): bump github.com/inconshreveable/mousetrap (spf13#1872) * Add documentation about disabling completion descriptions (spf13#1901) * Improve MarkFlagsMutuallyExclusive example in User Guide (spf13#1904) * Update shell_completions.md (spf13#1907) align documentation with the code : completions.go:452 * build(deps): bump golangci/golangci-lint-action from 3.3.1 to 3.4.0 (spf13#1902) * Removes stale bot from GitHub action (spf13#1908) Signed-off-by: John McBride <[email protected]> * Add keeporder to shell completion (spf13#1903) This allows programs to request the shell to maintain the order of completions that was returned by the program * Add support for PowerShell 7.2+ (spf13#1916) PowerShell 7.2 has changed the way arguments are passed to executables. This was originally an experimental feature in 7.2, but as of 7.3 it is built-in. A simple "" is now sufficient for passing empty arguments, no back-tick escaping is required. Fixes spf13#1849 Signed-off-by: Oldřich Jedlička <[email protected]> Co-authored-by: Oldřich Jedlička <[email protected]> * ci: deprecate go 1.16 (spf13#1926) * ci: test Golang 1.20 (spf13#1925) * update copyright year (spf13#1927) * Update projects_using_cobra.md (spf13#1932) Signed-off-by: Florent Poinsard <[email protected]> * Document suggested layout for subcommands (spf13#1930) Signed-off-by: Luiz Carvalho <[email protected]> * Allow sourcing zsh completion script (spf13#1917) Although it is not the recommended approach, sourcing a completion script is the simplest way to get people to try using shell completion. Not allowing it for zsh has turned out to complicate shell completion adoption. Further, many tools modify the zsh script to allow sourcing. This commit allows sourcing of the zsh completion script. Signed-off-by: Marc Khouzam <[email protected]> * Update main image to better handle dark background (spf13#1883) Fixes spf13#1880 Signed-off-by: Marc Khouzam <[email protected]> Co-authored-by: Deleplace <[email protected]> * Fix typo in fish completions (spf13#1945) * build(deps): bump golangci/golangci-lint-action from 3.4.0 to 3.5.0 (spf13#1971) * Fix grammar: 'allows to' (spf13#1978) The use in generated bash completion files is getting flagged by Lintian (the Debian package linting tool). Signed-off-by: Taavi Väänänen <[email protected]> * test: make fish_completions_test more robust (spf13#1980) Use temporary files instead of assuming the current directory is writable. Also, if creating a temporary file still returns an error, prevent the test from failing silently by replacing `log.Fatal` with `t.Fatal`. * powershell: escape variable with curly brackets (spf13#1960) This fixes an issue with program names that include a dot, in our case `podman.exe`. This was caused by the change in commit 6ba7ebb. Fixes spf13#1853 Signed-off-by: Paul Holzinger <[email protected]> * build(deps): bump golangci/golangci-lint-action from 3.5.0 to 3.6.0 (spf13#1976) * Move documentation sources to site/content (spf13#1428) * Add 'one required flag' group (spf13#1952) * golangci: enable 'unused' and disable deprecated replaced by it (spf13#1983) * doc: fix typo, Deperecated -> Deprecated (spf13#2000) * minor corrections to unit tests (spf13#2003) * build(deps): bump golangci/golangci-lint-action from 3.6.0 to 3.7.0 (spf13#2021) * command: temporarily disable G602 due to securego/gosec#1005 (spf13#2022) * ci: test golang 1.21 (spf13#2024) * Customizable error message prefix (spf13#2023) * feat: add getters for flag completions (spf13#1943) * Add notes to doc on preRun and postRun condition (spf13#2041) * build(deps): bump actions/setup-go from 3 to 4 (spf13#1934) * build(deps): bump github.com/cpuguy83/go-md2man/v2 from 2.0.2 to 2.0.3 (spf13#2047) * Allow running persistent run hooks of all parents (spf13#2044) Currently, only one of the persistent pre-runs and post-runs is executed. It is always the first one found in the parents chain, starting at this command. Expected behavior is to execute all parents' persistent pre-runs and post-runs. Dependent projects implemented various workarounds for this: - manually building persistent hook chains (in every hook). - applying some kind of monkey-patching on top of Cobra. This change eliminates the necessity for such workarounds by allowing to set a global variable EnableTraverseRunHooks. Tickets: - spf13#216 - spf13#252 Signed-off-by: Volodymyr Khoroz <[email protected]> * Fix linter errors (spf13#2052) When using golangci-lint v1.55.0 some new errors were being reported. Signed-off-by: Marc Khouzam <[email protected]> * Don't complete --help flag when flag parsing disabled (spf13#2061) Fixes spf13#2060 When a command sets `DisableFlagParsing = true` it requests the responsibility of doing all the flag parsing. Therefore even the `--help/-f/--version/-v` flags should not be automatically completed by Cobra in such a case. Without this change the `--help/-h/--version/-v` flags can end up being completed twice for plugins: one time from cobra and one time from the plugin (which has set `DisableFlagParsing = true`). Signed-off-by: Marc Khouzam <[email protected]> * Add tests for flag completion registration (spf13#2053) Different problems have been reported about flag completion registration. These two tests are the cases that were not being verified but had been mentioned as problematic. Ref: - spf13#1320 - spf13#1438 (comment) Signed-off-by: Marc Khouzam <[email protected]> * Replace all non-alphanumerics in active help env var program prefix (spf13#1940) * Replace all non-alphanumerics in active help env var program prefix There are other characters besides the dash that are fine in program names, but are problematic in environment variable names. These include (but are not limited to) period, space, and non-ASCII letters. * Another change in docs to mention non-ASCII-alphanumeric instead of just dash * build(deps): bump actions/checkout from 3 to 4 (spf13#2028) * Support usage as plugin for tools like kubectl (spf13#2018) In this case the executable is `kubectl-plugin`, but we run it as: kubectl plugin And the help text should reflect the actual usage of the command. To create a plugin, add the cobra.CommandDisplayNameAnnotation: rootCmd := &cobra.Command{ Use: "plugin", Annotations: map[string]string{ cobra.CommandDisplayNameAnnotation: "kubectl plugin", } } Internally this change modifies CommandPath() for the root command to return the command display name instead of the command name. This is used for error messages, help text generation, and completions. CommandPath() is expected to have spaces and code using it already handle spaces (e.g replacing with _), so hopefully this does not break anything. Fixes: spf13#2017 Signed-off-by: Nir Soffer <[email protected]> * Improve API to get flag completion function (spf13#2063) The new API is simpler and matches the `c.RegisterFlagCompletionFunc()` API. By removing the global function `GetFlagCompletion()` we are more future proof if we ever move from a global map of flag completion functions to something associated with the command. The commit also makes this API work with persistent flags by using `c.Flag(flagName)` instead of `c.Flags().Lookup(flagName)`. The commit also adds unit tests. Signed-off-by: Marc Khouzam <[email protected]> * feat: expose GetCompletions (was getCompletions) --------- Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: David Wertenteil <[email protected]> Signed-off-by: John McBride <[email protected]> Signed-off-by: Oldřich Jedlička <[email protected]> Signed-off-by: Florent Poinsard <[email protected]> Signed-off-by: Luiz Carvalho <[email protected]> Signed-off-by: Marc Khouzam <[email protected]> Signed-off-by: Taavi Väänänen <[email protected]> Signed-off-by: Paul Holzinger <[email protected]> Signed-off-by: Volodymyr Khoroz <[email protected]> Signed-off-by: Nir Soffer <[email protected]> Co-authored-by: Brian Pursley <[email protected]> Co-authored-by: Enrico Candino <[email protected]> Co-authored-by: Norman Dankert <[email protected]> Co-authored-by: Unai Martinez-Corral <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: David Wertenteil <[email protected]> Co-authored-by: Yash Ladha <[email protected]> Co-authored-by: Seonghyeon Cho <[email protected]> Co-authored-by: Dominik Roos <[email protected]> Co-authored-by: Shihta Kuan <[email protected]> Co-authored-by: janhn <[email protected]> Co-authored-by: Ggg6542 <[email protected]> Co-authored-by: John McBride <[email protected]> Co-authored-by: Gyanendra Mishra <[email protected]> Co-authored-by: Oldřich Jedlička <[email protected]> Co-authored-by: Oldřich Jedlička <[email protected]> Co-authored-by: Florent Poinsard <[email protected]> Co-authored-by: Luiz Carvalho <[email protected]> Co-authored-by: Marc Khouzam <[email protected]> Co-authored-by: Deleplace <[email protected]> Co-authored-by: Tom Payne <[email protected]> Co-authored-by: Taavi Väänänen <[email protected]> Co-authored-by: Branch Vincent <[email protected]> Co-authored-by: Paul Holzinger <[email protected]> Co-authored-by: Martijn Evers <[email protected]> Co-authored-by: gocurr <[email protected]> Co-authored-by: Jun Nishimura <[email protected]> Co-authored-by: Nuno Adrego <[email protected]> Co-authored-by: Souma <[email protected]> Co-authored-by: Alexandru-Claudius Virtopeanu <[email protected]> Co-authored-by: Haoming Meng <[email protected]> Co-authored-by: vkhoroz <[email protected]> Co-authored-by: Ville Skyttä <[email protected]> Co-authored-by: Nir Soffer <[email protected]> Co-authored-by: Geoffrey Ragot <[email protected]>

h4ck3rk3y added 4 commits January 30, 2023 18:26

added keep order to bash, zsh, powershell & fish

a3adbf8

fix a tiny bug with bash

bc561bc

fix indentation

6e475b6

added keep order to default bash completions

87cd943

h4ck3rk3y commented Jan 30, 2023

View reviewed changes

bash_completions.go Outdated Show resolved Hide resolved

marckhouzam added kind/feature A feature request for cobra; new or enhanced behavior area/shell-completion All shell completions labels Jan 31, 2023

marckhouzam added this to the 1.7.0 milestone Jan 31, 2023

removed changes that arent necessary for new bash

3fc2de1

github-actions bot removed the area/shell-completion All shell completions label Feb 2, 2023

h4ck3rk3y requested a review from marckhouzam February 3, 2023 17:48

marckhouzam requested changes Feb 6, 2023

View reviewed changes

marckhouzam added the area/shell-completion All shell completions label Feb 6, 2023

fix fish

22ae670

github-actions bot removed the area/shell-completion All shell completions label Feb 7, 2023

h4ck3rk3y requested a review from marckhouzam February 7, 2023 09:35

marckhouzam requested changes Feb 11, 2023

View reviewed changes

h4ck3rk3y added 4 commits February 13, 2023 10:09

worked on some reviews

cbcd050

support bash3

d115eb6

added a comment

fcd84a9

added logic to reduce calls to underlying program

3b1d173

h4ck3rk3y requested a review from marckhouzam February 14, 2023 11:00

marckhouzam added the area/shell-completion All shell completions label Feb 25, 2023

marckhouzam approved these changes Feb 25, 2023

View reviewed changes

marckhouzam merged commit 3daa4b9 into spf13:main Feb 25, 2023

hoshsadiq pushed a commit to zulucmd/zulu that referenced this pull request May 16, 2023

added keep order to bash, zsh, pwsh & fish

0a524d2

Closes #1900 Merge spf13/cobra#1903

Dav-14 mentioned this pull request Mar 15, 2024

feat: update cobra formancehq/cobra#1

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added keep order to bash, zsh, pwsh & fish #1903

added keep order to bash, zsh, pwsh & fish #1903

h4ck3rk3y commented Jan 30, 2023 •

edited

Loading

CLAassistant commented Jan 30, 2023 •

edited

Loading

marckhouzam left a comment

h4ck3rk3y commented Feb 7, 2023

marckhouzam left a comment

h4ck3rk3y commented Feb 23, 2023

marckhouzam commented Feb 23, 2023

marckhouzam left a comment

added keep order to bash, zsh, pwsh & fish #1903

added keep order to bash, zsh, pwsh & fish #1903

Conversation

h4ck3rk3y commented Jan 30, 2023 • edited Loading

CLAassistant commented Jan 30, 2023 • edited Loading

marckhouzam left a comment

Choose a reason for hiding this comment

h4ck3rk3y commented Feb 7, 2023

marckhouzam left a comment

Choose a reason for hiding this comment

h4ck3rk3y commented Feb 23, 2023

marckhouzam commented Feb 23, 2023

marckhouzam left a comment

Choose a reason for hiding this comment

h4ck3rk3y commented Jan 30, 2023 •

edited

Loading

CLAassistant commented Jan 30, 2023 •

edited

Loading