Skip to content
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

Add shell completion generation #5311

Merged
merged 7 commits into from Mar 29, 2023
Merged

Conversation

anihm136
Copy link
Contributor

Fixes #5104. Currently still WIP - completions work correctly for a command named 'git-lfs', but not for 'git lfs'. Need to figure out how to modify the generated script to complete the subcommand correctly.

@anihm136 anihm136 requested a review from a team as a code owner March 14, 2023 16:49
@anihm136 anihm136 changed the title Add shell completion generation [WIP] Add shell completion generation Mar 14, 2023
@bk2204
Copy link
Member

bk2204 commented Mar 15, 2023

Hey,

Is this still a WIP, or are you ready for review?

@anihm136
Copy link
Contributor Author

Still WIP, apologies if it notified you for a review. Should I convert it to a draft PR until it's ready?

@bk2204
Copy link
Member

bk2204 commented Mar 16, 2023

Sure, converting to draft would be a good idea. Since we've already been notified, please just mention in the PR body when this is ready for review and we'll try to get to it reasonably quickly.

Thanks for taking the time to send a pull request for this; I know a bunch of people want this feature.

@anihm136 anihm136 marked this pull request as draft March 16, 2023 14:11
@anihm136 anihm136 marked this pull request as ready for review March 18, 2023 04:56
@anihm136
Copy link
Contributor Author

@bk2204 this is ready for review now. I do not have access to PowerShell, so I am unable to manually verify if completions are working. Do you expect tests for this?

Copy link
Member

@bk2204 bk2204 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch!

I noticed one thing we should fix for localization purposes, but otherwise I think this looks good. If you think you can write a few tests for bash (we don't use zsh, fish, or PowerShell in the testsuite, so we can't test them), that would be great, but if it's not possible, then we can skip it.

commands/run.go Outdated Show resolved Hide resolved
@anihm136
Copy link
Contributor Author

Getting the list of completions for a command in bash seems to be tricky, so I'm leaving out the test for now. Please let me know if anything else is needed. Thanks!

@bk2204
Copy link
Member

bk2204 commented Mar 22, 2023

It looks like this is failing CI because the files aren't formatted properly. Could you fix that by running goimports to clean this up?

Copy link
Member

@bk2204 bk2204 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Thanks again for the patch!

@bk2204 bk2204 merged commit 5dbee4f into git-lfs:main Mar 29, 2023
10 checks passed
@chrisd8088 chrisd8088 changed the title [WIP] Add shell completion generation Add shell completion generation Jun 27, 2023
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Jul 17, 2023
In commit 1572300 of PR git-lfs#5311 the
"git lfs completion" command was introduced, utilizing the support
provided by the spf13/cobra package to generate tab-completion scripts
for a number of shells, including Bash, fish, Zsh, and PowerShell.

For the shells other than PowerShell, the generated completion scripts
are intended to work with the tab-completion scripts for Git commands
that are available from either the Git project (for Bash) or as an
implementation native to the shell.  This coordination permits users
to receive Git LFS command suggestions when they type "git lfs [Tab]",
for instance, instead of only when they type "git-lfs [Tab]".

In the case of PowerShell, Git command tab-completion is provided by a
third-party project (https://github.com/dahlbyk/posh-git), and while
it may eventually be possible to integrate that project's scripts with
the ones generated by the spf13/cobra package, at the present time
the Git LFS core team lacks the time and expertise to develop such
a solution.  Thus the script currently generated for Git LFS command
completion in PowerShell would only provide suggestions when users
enter the "git-lfs" program name, rather than "git lfs".

As we do not want to offer a subpar user experience and one which the
Git LFS core team is unable to properly support, for the present time we
remove the PowerShell option from the "git lfs completion" command.

Note that PowerShell is primarily (but not exclusively) used on Windows,
where most Git LFS users will have installed Git LFS as part of the
Git for Windows project.  Git for Windows also provides an emulation
of the Bash shell, where the Bash tab-completion script generated by
the "git lfs completion" command should function as expected.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Jul 17, 2023
In commit 1572300 of PR git-lfs#5311 the
"git lfs completion" command was introduced, utilizing the support
provided by the spf13/cobra package to generate tab-completion scripts
for a number of shells.  Short and long help text was provided at
the same time, in the formats offered by the spf13/cobra package.

However, the Git LFS client explicitly overrides the default help
functions of the spf13/cobra package, and replacing them with functions
which output versions of the Git LFS manual pages that are pre-rendered
by our build process.  Thus the long-format help text included in
PR git-lfs#5311 can not be viewed by end users.

As this text is also somewhat terse, having been copied from the
example text provided by the spf13/cobra package, we remove it now
with the expectation that in a subsequent commit in this PR we will
provide a more comprehensive set of instructions in a new manual page
for the "git lfs completion" command.

We also remove the short help text for the "git lfs completion" command,
as the majority of other Git LFS commands (with the exception of the
"git lfs help" command) do not define these text strings, and in a
subsequent commit in this PR we will suppress their inclusion in the
command lists output by the completion scripts, so as not to make
the formatting of those lists irregular and uneven.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Jul 17, 2023
In commit 1572300 of PR git-lfs#5311 the
"git lfs completion" command was introduced, utilizing the support
provided by the spf13/cobra package to generate tab-completion scripts
for a number of shells, including the Bash, fish, and Zsh shells.

We now reorder the list of these shells and their relevant case switches
so they are in alphabetical order.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Jul 17, 2023
In commit 1572300 of PR git-lfs#5311 the
"git lfs completion" command was introduced, utilizing the support
provided by the spf13/cobra package to generate tab-completion scripts
for a number of shells, including the Bash shell.  The completion
script for the Bash shell is currently generated using the spf13/cobra
package's legacy implementation, which is returned by the
(*Command).GenBashCompletion() method.

However, the spf13/cobra package's maintainers advise using their
newer implementation of tab-completion for the Bash shell, which is
returned by the (*Command).GenBashCompletionV2() method, as this
version of the script is shorter and aligned with the tab-completion
scripts now generated for other shells.

In particular, the new version of the Bash tab-completion script has
support for the program to return custom, dynamic completion lists using
per-command functions.  While the current Git LFS client does not
implement any of these ValidArgsFunction() or RegisterFlagCompletionFunc()
functions, we anticipate doing so in the future so as to be able to
provide lists of Git-specific values such as ref and remote names,
when appropriate for a particular Git LFS command.

Note that in order for the new Bash completion script for Git LFS to
integrate properly with the Bash completion script from the Git project,
we have to revise the script generated by the spf13/cobra package so it
rewrites the set of command words when the user types "git lfs ...[Tab]"
such that the initial two words are replaced with the single word
"git-lfs".  This requires a more substantial replacement of a portion
of the generated script than was the case with the legacy implementation.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Jul 17, 2023
In a prior commit in this PR we revised the "git lfs completion" command
so it would generate a tab-completion script for the Bash shell using
the newer implementation offered by the spf13/cobra package's
(*Command).GenBashCompletionV2() method, and in doing so also added
a search-and-replace operation to revise the generated script such that
it would integrate with Git's own tab-completion script for Bash.

We already perform a similar search-and-replace operation to revise
the script generated for the Zsh shell by the spf13/cobra package's
(*Command).GenZshCompletion() method.  These script revisions were
introduced in commit 4c9987d of PR git-lfs#5311
so that the tab-completion script for Git LFS would integrate with the
Zsh shell's native tab-completion for Git.

We now refactor the script generation and modification for Zsh to more
closely align with that used for the Bash shell, which should also
slightly improve the clarity of the code and make its purpose more
obvious.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Jul 17, 2023
In commit 1572300 of PR git-lfs#5311 the
"git lfs completion" command was introduced, utilizing the support
provided by the spf13/cobra package to generate tab-completion scripts
for a number of shells, including the Bash, fish, and Zsh shells.
These scripts make use of "hidden"  __complete and __completeNoDesc
commands, also implemented by the spf13/cobra package, which the
shell completion functions may query to retrieve dynamic lists of
command names and flags from the Git LFS client.

At present, the __complete command is used, which also returns any
short help text defined each command.  This additional descriptive
text is then presented to the user if they are running a shell like
Zsh whose completion system supports the display of such hints.

However, as we only define short help text for a single Git LFS
command, namely the "git lfs help" command, the display of this
one text string causes the columnar display of available command
names to be prefaced with a single "help" line when the user
types "git lfs [Tab]":

  help             -- Help about any command
  checkout         fsck             post-checkout    status
  clean            install          post-commit      track
  ...              ...              ...              ...

This irregularity makes the display output less helpful and more
difficult to parse than if we simply suppress the inclusion of the
per-command descriptions entirely, so we do so by setting the
appropriate flags or using a different script generation method
of the spf13/cobra package.

Note that we then also need to update the name of the __complete
command to __completeNoDesc in the search-and-replace operation we
perform on the script generated for the Zsh shell.

We can always revisit this choice in the future should we choose
to add short help text to all our command definitions.  This would
require refactoring our NewCommand() and RegisterCommand() functions
to accept the per-command text strings as extra parameters.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Jul 17, 2023
In commit 1572300 of PR git-lfs#5311 the
"git lfs completion" command was introduced, utilizing the support
provided by the spf13/cobra package to generate tab-completion scripts
for a number of shells, including the Bash, fish, and Zsh shells.
No tests were added at the same time, however, so we do so now.

The new tests are simple and just confirm that the "git lfs completion"
command generates the expected error messages if it is not given an
expected shell name as an argument, and that when one of the supported
shell names is provided, the command generates the expected script.

We do not attempt to test the functionality of the generated scripts.

Although these tests are quite basic, they should help alert us to
any potential regressions caused by an upgrade of the spf13/cobra
package.

In particular, if the scripts generated by that package for the Bash
or Zsh shells change such that the search-and-replace operations
we perform on the scripts cease to function, the "git lfs completion"
command will still succeed but may produce a script which no longer
integrates well with the tab-completion scripts for Git itself.

Since we have now have archived copies of the current scripts as test
fixtures, the tests will fail when the generated scripts are different.
This should alert us to the fact that we need to check the new
script output and confirm that it still works with Git tab-completion
on the affected shells, and if not, revise our search-and-replace
operations accordingly.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Jul 17, 2023
In commit 1572300 of PR git-lfs#5311 the
"git lfs completion" command was introduced, utilizing the support
provided by the spf13/cobra package to generate tab-completion scripts
for a number of shells, including the Bash, fish, and Zsh shells.

However, no manual page was added at the same time, although help text
was included using the spf13/cobra package's format for long command
descriptions.  Unfortunately, as described in an earlier commit in this
PR, the Git LFS client overrides the default help functions defined by
the spf13/cobra package and so there was no way for a user to access
this long-format help text for the "git lfs completion" command.

We therefore removed the original help text for the command, and now
reintroduce a revised and expanded version of the text in the form of
a manual page for the git-lfs-completion(1) command.

The full text of this new manual page attempts to explain the features
and limitations of the current tab-completion scripts, especially the fact
that the Git LFS client does not yet offer suggested lists of terms
specific to Git, such as ref or remote names, but that the tab-completion
scripts for Git LFS should integrate well with the tab-completion
scripts for Git itself, provided the correct one is used for a given
shell.

We also try to include examples of the configuration of tab-completion
for each type of shell and for both a user's current session as well
as for future sessions.  Complete documentation of all such per-shell
configuration options is beyond the scope of the Git LFS project,
of course, but we aim to provide a sufficient overview so as to guide
users to a working solution and forestall support requests as much
as possible.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Jul 18, 2023
In commit 1572300 of PR git-lfs#5311 the
"git lfs completion" command was introduced, utilizing the support
provided by the spf13/cobra package to generate tab-completion scripts
for a number of shells, including the Bash, fish, and Zsh shells.

However, no manual page was added at the same time, although help text
was included using the spf13/cobra package's format for long command
descriptions.  Unfortunately, as described in an earlier commit in this
PR, the Git LFS client overrides the default help functions defined by
the spf13/cobra package and so there was no way for a user to access
this long-format help text for the "git lfs completion" command.

We therefore removed the original help text for the command, and now
reintroduce a revised and expanded version of the text in the form of
a manual page for the git-lfs-completion(1) command.

The full text of this new manual page attempts to explain the features
and limitations of the current tab-completion scripts, especially the fact
that the Git LFS client does not yet offer suggested lists of terms
specific to Git, such as ref or remote names, but that the tab-completion
scripts for Git LFS should integrate well with the tab-completion
scripts for Git itself, provided the correct one is used for a given
shell.

We also try to include examples of the configuration of tab-completion
for each type of shell and for both a user's current session as well
as for future sessions.  Complete documentation of all such per-shell
configuration options is beyond the scope of the Git LFS project,
of course, but we aim to provide a sufficient overview so as to guide
users to a working solution and forestall support requests as much
as possible.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Jul 18, 2023
In commit 1572300 of PR git-lfs#5311 the
"git lfs completion" command was introduced, utilizing the support
provided by the spf13/cobra package to generate tab-completion scripts
for a number of shells, including the Bash, fish, and Zsh shells.

However, no manual page was added at the same time, although help text
was included using the spf13/cobra package's format for long command
descriptions.  Unfortunately, as described in an earlier commit in this
PR, the Git LFS client overrides the default help functions defined by
the spf13/cobra package and so there was no way for a user to access
this long-format help text for the "git lfs completion" command.

We therefore removed the original help text for the command, and now
reintroduce a revised and expanded version of the text in the form of
a manual page for the git-lfs-completion(1) command.

The full text of this new manual page attempts to explain the features
and limitations of the current tab-completion scripts, especially the fact
that the Git LFS client does not yet offer suggested lists of terms
specific to Git, such as ref or remote names, but that the tab-completion
scripts for Git LFS should integrate well with the tab-completion
scripts for Git itself, provided the correct one is used for a given
shell.

We also try to include examples of the configuration of tab-completion
for each type of shell and for both a user's current session as well
as for future sessions.  Complete documentation of all such per-shell
configuration options is beyond the scope of the Git LFS project,
of course, but we aim to provide a sufficient overview so as to guide
users to a working solution and forestall support requests as much
as possible.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature] Shell completion
2 participants