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

command: add extensive autocompletion support #500

Merged
merged 38 commits into from
Sep 16, 2022
Merged

command: add extensive autocompletion support #500

merged 38 commits into from
Sep 16, 2022

Conversation

kucukaslan
Copy link
Contributor

@kucukaslan kucukaslan commented Aug 25, 2022

Adds extensive auto-completion support to s5cmd for zsh, bash and pwsh. Including:

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

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

Fixes #207
Fixes #372

kucukaslan and others added 15 commits August 12, 2022 16:52
add escape colon method for autocomplete parameters.
except the "version" and "mb", for which autocompletion of remote buckets/keys does not make sense.
-  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.
- 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
@kucukaslan
Copy link
Contributor Author

kucukaslan commented Aug 26, 2022

  • command completion
  • flag completion
  • local file/directory completion

s5cmd cp

  • bucket name completion

s5cmd cp s3://

  • s3 key completion

s5cmd cp s3://bucket/

Note
I am not sure about the rest

In bash

  • Completions can handle: :, !, *
  • Completions CANNOT handle space characters,
  • Can handle &, @ and % if the argument is quoted, that is
    s5cmd cp "s3://bucket/ke@<TAB>

In zsh

  • Completions can handle: :, ?, *
    • Can handle space characters if the argument is quoted, that is
      s5cmd cp "s3://bucket/sp <TAB>

ps. <TAB> given above always means pressing when the cursor is at that point.

@kucukaslan kucukaslan mentioned this pull request Aug 26, 2022
@kucukaslan
Copy link
Contributor Author

kucukaslan commented Sep 1, 2022

General Notes

We are using the urfave/cli's autocompletion feature.
s5cmd has a special flag (--generate-bash-completion) which can only be the last parameter.
When this flag is given, s5cmd executes the appropriate BashCompleteFunc to prepare autocompletion suggestions.

We provide autocompletion scripts for bash, pwsh, and zsh. The essence of all three script is that:
When a user pressed to TAB

  • preserve current arguments,
  • execute s5cmd with generate-bash-completion flag
  • process/filter the autocompletion suggestions (and create local file/dir suggestions when appropriate)
  • provide the autocompletion suggestions to shell

Common problems (incomplete)

If the argument that is going to be completed contains $ then suggestions don't work. Similarly if some part of the argument is given via shell variable it doesn't work either.

zsh

Its autocompletion behavior uses : as a seperator for suggestion:description pairs. So when SHELL1 is zsh, s5cmd escapes2 colon charactes.

bash

Colon

In bash autocompletion behaviour uses : as word seperator ("break"), unless the colon is removed from COMP_WORDBREAKS global variable. So the autocompletion suggestions should not include the colon and the text preceding the colon. That when s3:// is going to be autocompleted to s3://bucket the suggestion should be //bucket instead of s3://bucket.
If the COMP_WORDBREAKS does not contain colon, then the autocompletion suggestion should remain as is, i.e. as s3://bucket.
But s5cmd is unaware of the value of COMP_WORDBREAKS. So when SHELL is bash, s5cmd prepares and gives both styles.
Then our custom autocompletion script filters the appropriate one and forwards it to shell.

Using COMP_LINE & COMP_POINT

When an autocompletion request is made BASH sets calls the autocompletion script with some environment variables. COMP_WORDS is one of them. It is an array of strings created from the original command to be completed. Normally it is used in urfave/cli's autocompletion. However it splits words according to notorious COMP_WORDBREAKS and colon characters are considered to be word breaks and seperate words. It might be still tempting to use it and 'just concat when there is a colon'.
But it is not possible for s5cmd to discern whether {'a', ':', 'b'} is originally a:b or a :b or a: b or a : b by only using COMP_WORDBREAKS. So instead of using the array then combining them, we will directly use the raw COMP_LINE string3 which is the original command string written by user to the terminal.
This, however, has some side effects. One of them is that it is not possible to discern whether TAB is pressed to complete the last argument or a new argument.
That is, it cannot discern  s5cmd cp s3://buck/obj<TAB> and  s5cmd cp s3://buck/obj <TAB>, so it tries to complete s3://buck/obj in both cases when we expect it to suggest a new argument in the second case. To overcome this, we have a simple hack in bash script, if there is a space just before the then we pass an empty string argument to s5cmd. But this time it doesn't respect the actual space character in the s5cmd cp "s3://buck/obj <TAB> despite the left quotation mark. I don't have a solution to it, and don't intent to solve that case.

ineffective completion

For mb command if argument is empty suggests s3:// otherwise suggests the argument itself. But in bash there is no suggestions made if argument is empty.

pwsh

Not doing anything special. It is the Powershell, BTW.

Manjaro KDE: zsh vs bash

s5cmd reads the SHELL environment variable and prepares the autocompletions according to its value.
Normally the SHELL is same with the default shell of the terminal so it works fine. But, anecdotally, I know that in Manjaro KDE/Plasma the Konsole starts with the zsh, but the SHELL is still set to bash.
I don't handle this case nor similar cases that shell is different then the SHELL.

autocompletion suggestions don't respect to the number of arguments

Autocompletions currently don't check number of arguments. For example cat command accepts only a single argument, but autocompletion continues to complete no matter how many args has been provided so far.
Although, it is possible to check number of arguments in s5cmd, I'm afraid that this won't stop the local file/dir suggestions. Since local completion is the fallback behaviour when s5cmd doesn't provide suggestions.

Number of suggestions provided by s5cmd

I set it to 20 to avoid excessive API calls. But we can increase or remove the limit.

Bug-like behaviour in bash

if s3://bucket/ can be completed by both some prefix(es) (s3 directory) and key then autocompletion seems to show only the prefix in bash. I know this was too incomprehensible, so let say there is a bucket named bucket having objects with keys dir/f, die2/mc, boo and dododo. Then dododo and boo are filtered out from the suggestions and the only suggestions became dir/ and die2/. This happens only in root level that is when completion requested for s3://bucket/. This behaviour does not occur in other shells.

Note
You may refer to #490 for the past discussions

Footnotes

  1. The environment variable SHELL. More precisely the name of the binary that SHELL points to. e.g. when SHELL is set to /bin/bash, I say SHELL is bash. Also beware that, I use shell to mean the shell application used in terminal.

  2. That is to replace every : with \:

  3. its substring up to COMP_POINT (cursor) to be more precise

@kucukaslan kucukaslan marked this pull request as ready for review September 1, 2022 14:21
@kucukaslan kucukaslan requested a review from a team as a code owner September 1, 2022 14:21
@kucukaslan kucukaslan requested review from aykutfarsak and seruman and removed request for a team September 1, 2022 14:21
command/auto_complete.go Outdated Show resolved Hide resolved
command/auto_complete.go Outdated Show resolved Hide resolved
Co-Authored-By: Aykut Farsak <aykutfarsak@gmail.com>
command/app.go Outdated Show resolved Hide resolved
command/auto_complete.go Outdated Show resolved Hide resolved
command/auto_complete.go Outdated Show resolved Hide resolved

// it returns a complete function which prints the argument, itself, which is to be completed.
// If the argument is empty string it uses the defaultCompletions to make suggestions.
func ineffectiveCompleteFnWithDefault(cmd *cli.Command, defaultCompletions ...string) func(ctx *cli.Context) {
Copy link
Member

Choose a reason for hiding this comment

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

WDYM by ineffective? Please give it a name that indicates what it does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean it doesn't actually predict anything but merely returns the argument itself (or some default values) to override shell's default autocompletion. May be nonpredictiveCompleteFnWithDefault can be a better name (or worse?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the record with https://github.com/Kucukaslan/s5cmd/commit/578f64c79aa9d76206c5fe90e8435a6afad91fdb this function is split to two parts and the auto-completion function is named constantCompleteWithDefault.

command/auto_complete.go Outdated Show resolved Hide resolved
command/auto_complete.go Outdated Show resolved Hide resolved
e2e/util_test.go Show resolved Hide resolved
e2e/util_test.go Outdated Show resolved Hide resolved
e2e/auto_complete_test.go Outdated Show resolved Hide resolved
storage/url/url.go Outdated Show resolved Hide resolved
@kucukaslan kucukaslan marked this pull request as draft September 13, 2022 07:50
@kucukaslan kucukaslan marked this pull request as ready for review September 14, 2022 07:08
command/auto_complete.go Outdated Show resolved Hide resolved
command/auto_complete.go Outdated Show resolved Hide resolved
command/auto_complete.go Outdated Show resolved Hide resolved
kucukaslan and others added 2 commits September 15, 2022 09:23
Co-authored-by: Selman Kayrancioglu <seruman@users.noreply.github.com>
Copy link
Member

@seruman seruman left a comment

Choose a reason for hiding this comment

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

🚀 Thank you!

@kucukaslan
Copy link
Contributor Author

kucukaslan commented Sep 15, 2022

🚀 Thank you!

My pleasure. Thanks a lot for your help and suggestions!

@sonmezonur sonmezonur merged commit 5e867c9 into peak:master Sep 16, 2022
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
4 participants