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

Auto completion #490

Closed
wants to merge 13 commits into from
Closed

Auto completion #490

wants to merge 13 commits into from

Conversation

kucukaslan
Copy link
Contributor

@kucukaslan kucukaslan commented Aug 17, 2022

Adds extensive auto-completion support to s5cmd. Including:

  • command completion
  • flag completion
  • local file/directory completion
  • bucket name completion
  • s3 key completion

Fixes #207
Fixes #372

@kucukaslan
Copy link
Contributor Author

kucukaslan commented Aug 23, 2022

Updates

Background

The last version that supported the autocompletion was v7.0.0. It used posener/complete and correctly handled s3 key completions. However it didn't completed local files/directories.

I've implemented1 autocompletion with both urfave/cli and posener/complete/v2.

In zsh it worked accurately -except the need to programmatically2 escape colons.

In bash however the colon's need to be handled more specially. Because, in bash colons are considered to be word breaks. So we should handle the colons specially.

Since v7.0.0 worked correctly, at the time, I've examined its implementation. It turned out that the completion script removes : from global COMP_WORDBREAKS array. That change also fixed the problem but it is not a good practice since changing COMP_WORDBREAKS may break some other program (completions).

As an alternative solution we could use the bash-completion package to handle colons without changing COMP_WORDBREAKS. But this time the user need to have her bash source the corresponding file by herself. A slight inconvenience to user.

As a third option we can also modify s5cmd's autocompletion suggestions to match the expectations of bash. It can check the SHELL environment variable to determine the SHELL and left trim the autocompletion for bash.

Current Status of Alternatives

with urfave/cli using bash-completion

  • Implemented in Kucukaslan:auto-completion branch
  • Works in zsh, bash and pwsh3
  • In bash it needs _init_completion and __ltrim_colon_completions functions from bash-completion package to be declared/imported beforehand.

with urfave/cli without bash-completion package

Note
I've made some changes today (2022.08.24). This might actually work.

with posener/complete/v2.

  • Implemented in Kucukaslan:forward-to-past branch
  • Works in zsh, but fails to handle : in bash, does not support pwsh and theoretically works in fish too4.
  • Flags and commands need to be adapted from urfave.cli's interfaces to posener.complete's respective interfaces

ps: First two of them have a little (?) problem:

$ s5cmd cp s3://mcks5cmd/key:has:co/ <TAB> 
produces

s5cmd cp s3://mcks5cmd/key:has:co/ s3://mcks5cmd/key:has:co/lon

but expected behaviour would be listing local files ( beware that TAB is pressed after a space)

Footnotes

  1. imperfectly

  2. That means the code of the s5cmd not the shell script.

  3. Powershell on MacOs

  4. That means there is a script generated for fish but I didn't tried it.

- It used to complete full s3 keys at once, but now it only completes till next s3 seperator (slash)
- bucket completion now appends slash (/) to the end of the bucket
-  autocomplete used to fall back to default bash behaviour after the  autocompletion suggestions are filtered. So the default bash completion completed "s5cmd ls s3://b" to "s3://bin/" since there is  a directory named "/bin/" in the local. Default behavior ignores "s3:" since : is a COMP_WORDBREAK. by moving the completion fallback to compgen function we ensure that local completions are also filtered properly.
@kucukaslan
Copy link
Contributor Author

kucukaslan commented Aug 26, 2022

I'm closing this PR in favour of #500.

Among the autocompletion solutions:

Posener/complete

it supports zsh, bash and fish
it has two relevant1 advantages :

  • it is possible to specify the autocompletion suggestions for flag arguments
  • the library handles the (un)installation of completion scripts

it has 3 disadvantages

  • it cannot handle the colons in bash. the notorious COMP_WORDBREAKS must be changed, unacceptable.
  • all of the commands and flags should be also defined for it, at least 'adapted' to it. This problem also avoids the "flag argument suggestion" advantage
  • I forgot the 3rd one2

using urfave/cli

it supports zsh, bash and pwsh

Changing COMP_WORDBREAKS

straightforward and easy but unacceptable.

using well-known bash-completion

relatively easy and feels more reliable since it was being used by many applications for years.
But it is not preferable to tell the user "please also import this other library written in bash".

write your own script and delegate most of the logic into app

  • it took a lot of time,
  • it required to write shell script, deal with COMP_WORDBREAKS on your own
  • ineffective use of time
  • slightly better for user experience, not ideal.
  • did I mentioned the time spent on it?

Decision

For obvious reasons, I've prefered using urfave/cli with our own bash completion script.
The user will be given the instructions and the completion script when she called s5cmd with install-completion flag. The instructions and script will be written according to value of SHELL environment variable.

Footnotes

  1. An example of "irrelavant" advantages is that you can use posener/complete to write completion for any cli application.

  2. https://images.app.goo.gl/pxCKgdQ61oDYtvRa6

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.

Bucket/key completion not supported? Auto completion for local files
2 participants