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 bash completion script #68

Closed
vlrpl opened this issue Jan 27, 2023 · 4 comments · Fixed by #329
Closed

add bash completion script #68

vlrpl opened this issue Jan 27, 2023 · 4 comments · Fixed by #329
Labels
good first issue Good for newcomers

Comments

@vlrpl
Copy link
Contributor

vlrpl commented Jan 27, 2023

There's a chance we'll end up having a decent number of options and adding a bash completion file could be a nice to have

@vlrpl vlrpl self-assigned this Jan 27, 2023
@atenart atenart added this to the v1 milestone Feb 21, 2023
@atenart
Copy link
Contributor

atenart commented Mar 10, 2023

As discussed in #75, it would be worth looking at https://docs.rs/clap_complete/4.1.1/clap_complete/index.html.

@atenart atenart added the good first issue Good for newcomers label Mar 10, 2023
@atenart atenart removed this from the v1 milestone May 16, 2023
@atenart atenart added this to the v1.1 milestone Jun 13, 2023
@atenart atenart removed this from the v1.2 milestone Aug 1, 2023
@liuhangbin
Copy link
Contributor

Hi @vlrpl @atenart

I checked this feature. clap_complete has 3 ways to generate the shell completion.

The 1st way is using generate_to, which will generate the shell completion file at compile time. With this way we need to put the generator in build.rs. But this way is very hard to implement as we need to include!(src/cli/cli.rs), which also depends on a lot of modules.

The 2nd way is using generate, which could generate the completions file for a specified shell at runtime. But we need to add a generate parameter. e.g.

# retis -h

Usage: retis [OPTIONS] <COMMAND>

Commands:
  collect  Collect network events
  print    Print stored events to stdout
  sort     Sort stored events in series based on tracking id
  pcap     Generate a PCAP file from stored events
  profile  Manage Profiles

Options:
      --log-level <LOG_LEVEL>  Log level [default: info] [possible values: error, warn, info, debug, trace]
  -p, --profile <PROFILE>      Comma separated list of profile names to apply
      --kconf <KCONF>          Path to kernel configuration (e.g. /boot/config-6.3.8-200.fc38.x86_64; default: auto-detect)
  -g, --generate <generator>   [possible values: bash, elvish, fish, powershell, zsh]
  -h, --help                   Print help (see more with '--help')
  -V, --version                Print version

# retis -g bash > bash_complete
# source bash_complete

The 3rd way is using the unstalbe dynamic feature. With this way we can hide the generate parameter and user will not see it normally. e.g.

# retis -h
A subcommand definition to `flatten` into your CLI

Usage: retis [OPTIONS] <COMMAND>

Commands:
  collect  Collect network events
  print    Print stored events to stdout
  sort     Sort stored events in series based on tracking id
  pcap     Generate a PCAP file from stored events
  profile  Manage Profiles

Options:
      --log-level <LOG_LEVEL>  Log level [default: info] [possible values: error, warn, info, debug, trace]
  -p, --profile <PROFILE>      Comma separated list of profile names to apply
      --kconf <KCONF>          Path to kernel configuration (e.g. /boot/config-6.3.8-200.fc38.x86_64; default: auto-detect)
  -h, --help                   Print help (see more with '--help')
  -V, --version                Print version

# retis complete -h
Register shell completions for this program

Usage: retis complete --shell <SHELL> --register <REGISTER> [-- <COMP_WORDS>...]

Options:
      --shell <SHELL>        Specify shell to complete for [possible values: bash, fish]
      --register <REGISTER>  Path to write completion-registration to
  -h, --help                 Print help (see more with '--help')

# retis complete --shell bash --register bash_complete
# source bash_complete

Note the 2nd way supports bash, elvish, fish, powershell, zsh while the 3rd way only supports bash, fish. So which way do you want?

The other question is how to use the completion file after generating? We can't force user to source it every time. Should we add an alise file to source the completion file and execuate retis with all parameters ? e.g.

#!/bin/bash

source ${the_install_path}/shell_completion
retis $@

@atenart
Copy link
Contributor

atenart commented Jan 5, 2024

I checked this feature. clap_complete has 3 ways to generate the shell completion.

Thanks for looking into this, it'll improve the UX by a lot!

The 1st way is using generate_to, which will generate the shell completion file at compile time. With this way we need to put the generator in build.rs. But
this way is very hard to implement as we need to include!(src/cli/cli.rs), which also depends on a lot of modules.

Agreed, while generating completion scripts at build time would be best, due to how we handle cli options this is not possible.

The 2nd way is using generate, which could generate the completions file for a specified shell at runtime. But we need to add a generate parameter. e.g.

# retis -h

Usage: retis [OPTIONS] <COMMAND>

Commands:
  collect  Collect network events
  print    Print stored events to stdout
  sort     Sort stored events in series based on tracking id
  pcap     Generate a PCAP file from stored events
  profile  Manage Profiles

Options:
      --log-level <LOG_LEVEL>  Log level [default: info] [possible values: error, warn, info, debug, trace]
  -p, --profile <PROFILE>      Comma separated list of profile names to apply
      --kconf <KCONF>          Path to kernel configuration (e.g. /boot/config-6.3.8-200.fc38.x86_64; default: auto-detect)
  -g, --generate <generator>   [possible values: bash, elvish, fish, powershell, zsh]
  -h, --help                   Print help (see more with '--help')
  -V, --version                Print version

# retis -g bash > bash_complete
# source bash_complete

Nit: if we go this way I think a complete sub-command would be better than a top level parameter.

The 3rd way is using the unstalbe dynamic feature. With this way we can hide the generate parameter and user will not see it normally. e.g.

Note the 2nd way supports bash, elvish, fish, powershell, zsh while the 3rd way only supports bash, fish. So which way do you want?

IIUC the 3rd way would be best, as the completion feature would be built-in and would not require our or the user involvement. However I'm not sure how complete is the unstable-dynamic feature. Do you know if this is unstable because not all shells are supported or because some features are not there yet?

With that, it looks like the best option for now would be to have a retis generate command. @vlrpl @amorenoz WDYT?

The other question is how to use the completion file after generating? We can't force user to source it every time. Should we add an alise file to source the completion file and execuate retis with all parameters ? e.g.

#!/bin/bash

source ${the_install_path}/shell_completion
retis $@

Please, no. We should either install it as part of the rpm or in the container image, or let the user source it (should be documented in docs).

To install the completion script in the rpm or image we would need to generate it though. Maybe this could be done alongside #237 (eg. by calling the resulting binary and generating the completion files once available)? (Not that this is mandatory to get a first implementation in). Or maybe it's possible to call retis generate while building an rpm / image and install the resulting completion file.

@liuhangbin
Copy link
Contributor

Do you know if this is unstable because not all shells are supported or because some features are not there yet?

From the maintainer's comment, the dynamic completions is still under development and will replace static completions in the future. So I'd suggest use this feature directly.

Please, no. We should either install it as part of the rpm or in the container image, or let the user source it

OK, let's do it later. When the dynamic completions got stable. I think the completion file should be stable too as it doesn't depend on the parameter list. Here is an example file for bash completion.

_clap_complete_retis() {
    export IFS=$'\013'
    export _CLAP_COMPLETE_INDEX=${COMP_CWORD}
    export _CLAP_COMPLETE_COMP_TYPE=${COMP_TYPE}
    if compopt +o nospace 2> /dev/null; then
        export _CLAP_COMPLETE_SPACE=false
    else
        export _CLAP_COMPLETE_SPACE=true
    fi
    COMPREPLY=( $("retis" complete --shell bash -- "${COMP_WORDS[@]}") )
    if [[ $? != 0 ]]; then
        unset COMPREPLY
    elif [[ $SUPPRESS_SPACE == 1 ]] && [[ "${COMPREPLY-}" =~ [=/:]$ ]]; then
        compopt -o nospace
    fi
}
complete -o nospace -o bashdefault -F _clap_complete_retis retis

liuhangbin added a commit to liuhangbin/retis that referenced this issue Jan 8, 2024
The clap supports static and unstable-dynamic completion. The static
completion supports more shells. But it needs to add a `generate` parameter
specifically. While the unstable-dynamic completion could hide the parameter
for normal use. This should help if we want to build the complete file in
the release package and not disturb normal users.

The clap maintainer also said that dynamic completion will replace static
completion in future. So I chose to use dynamic completion, although it only
supports bash and fish right now.

Usage:
$ retis complete -h
Register shell completions for this program

Usage: retis complete --shell <SHELL> --register <REGISTER> [-- <COMP_WORDS>...]

Options:
      --shell <SHELL>        Specify shell to complete for [possible values: bash, fish]
      --register <REGISTER>  Path to write completion-registration to
  -h, --help                 Print help (see more with '--help')

$ retis complete --shell bash --register bash_complete
$ source bash_complete

Close: retis-org#68
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
liuhangbin added a commit to liuhangbin/retis that referenced this issue Jan 8, 2024
The clap supports static and unstable-dynamic completion. The static
completion supports more shells. But it needs to add a `generate` parameter
specifically. While the unstable-dynamic completion could hide the parameter
for normal use. This should help if we want to build the complete file in
the release package and not disturb normal users.

The clap maintainer also said that dynamic completion will replace static
completion in future. So I chose to use dynamic completion, although it only
supports bash and fish right now.

Usage:
$ retis complete -h
Register shell completions for this program

Usage: retis complete --shell <SHELL> --register <REGISTER> [-- <COMP_WORDS>...]

Options:
      --shell <SHELL>        Specify shell to complete for [possible values: bash, fish]
      --register <REGISTER>  Path to write completion-registration to
  -h, --help                 Print help (see more with '--help')

$ retis complete --shell bash --register bash_complete
$ source bash_complete

Close: retis-org#68
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
liuhangbin added a commit to liuhangbin/retis that referenced this issue Jan 17, 2024
As the clap_complete dynamic module is unstable and hard-codes the
subcommand as `complete`, which conflicts with the current `collect`
subcommand. Let's still use the legacy static way to generate the
completion file.

Usage:
$ retis generate -h
Generate completions file for a specified shell

Usage: retis generate [OPTIONS]

Options:
      --shell <SHELL>        Specify shell to complete for [possible values: bash, elvish, fish, powershell, zsh]
      --register <REGISTER>  Path to write completion-registration to
  -h, --help                 Print help

$ retis generate --shell bash --register .
$ source retis.bash

Close: retis-org#68
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
liuhangbin added a commit to liuhangbin/retis that referenced this issue Jan 18, 2024
As the clap_complete dynamic module is unstable and hard-codes the
subcommand as `complete`, which conflicts with the current `collect`
subcommand. Let's still use the legacy static way to generate the
completion file.

Usage:
$ retis sh-complete -h
Generate completions file for a specified shell

Usage: retis sh-complete [OPTIONS]

Options:
      --shell <SHELL>        Specify shell to complete for [possible values: bash, elvish, fish, powershell, zsh]
      --register <REGISTER>  Path to write completion-registration to
  -h, --help                 Print help

$ retis sh-complete --shell bash --register .
$ source retis.bash

Close: retis-org#68
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
liuhangbin added a commit to liuhangbin/retis that referenced this issue Jan 30, 2024
As the clap_complete dynamic module is unstable and hard-codes the
subcommand as `complete`, which conflicts with the current `collect`
subcommand. Let's still use the legacy static way to generate the
completion file.

Usage:
$ retis sh-complete -h
Generate completions file for a specified shell

Usage: retis sh-complete [OPTIONS]

Options:
      --shell <SHELL>        Specify shell to complete for [possible values: bash, elvish, fish, powershell, zsh]
      --register <REGISTER>  Path to write completion-registration to
  -h, --help                 Print help

$ retis sh-complete --shell bash --register .
$ source retis.bash

Close: retis-org#68
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
liuhangbin added a commit to liuhangbin/retis that referenced this issue Feb 1, 2024
As the clap_complete dynamic module is unstable and hard-codes the
subcommand as `complete`, which conflicts with the current `collect`
subcommand. Let's still use the legacy static way to generate the
completion file.

Usage:
$ retis sh-complete -h
Generate completions file for a specified shell

Usage: retis sh-complete [OPTIONS]

Options:
      --shell <SHELL>        Specify shell to complete for [possible values: bash, elvish, fish, powershell, zsh]
      --register <REGISTER>  Path to write completion-registration to
  -h, --help                 Print help

$ retis sh-complete --shell bash --register .
$ source retis.bash

Close: retis-org#68
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants