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

bash generated completions invalid when command name contains spaces #4822

Open
2 tasks done
mjpieters opened this issue Apr 3, 2023 · 4 comments
Open
2 tasks done
Labels
A-builder Area: Builder API C-bug Category: Updating dependencies M-breaking-change Meta: Implementing or merging this will introduce a breaking change.
Milestone

Comments

@mjpieters
Copy link

mjpieters commented Apr 3, 2023

Please complete the following tasks

Rust Version

rustc 1.67.0 (fc594f156 2023-01-24)

Clap Version

4.2.1

Minimal reproducible code

Cargo.toml

[package]
name = "mcre_clap_generate"
version = "0.1.0"
edition = "2021"
[dependencies]
clap = { version = "4.2.1", features = ["derive"] }
clap_complete = "4.2.0"

main.rs:

use std::path::PathBuf;
use clap::{CommandFactory, Parser, ValueHint, error::Result};
use clap_complete::{generate_to, shells};

#[derive(Parser)]
pub enum SubCommand {
    Completions {
        /// The output directory to which the file should be written.
        #[arg(value_hint = ValueHint::DirPath)]
        output_directory: PathBuf,
    },
}

#[derive(Parser)]
#[command(
    name = "MCRE for clap generate output",
    about = "This is minimal, complete reproducible example for clap generate output producing invalid Bash completions",
)]
struct Cli {
    #[command(subcommand)]
    pub cmd: Option<SubCommand>,
}

fn main() -> Result<()> {
    let opt = Cli::parse();
    if let Some(SubCommand::Completions {
        output_directory,
    }) = &opt.cmd
    {
        generate_to(shells::Bash, &mut Cli::command(), "mcre_clap_generate", output_directory)?;
        generate_to(shells::Zsh, &mut Cli::command(), "mcre_clap_generate", output_directory)?;
    }
    Ok(())
}

Steps to reproduce the bug with the above code

$ cargo build
...
$ mkdir -p /tmp/sample_output; ./target/debug/mcre_clap_generate completions /tmp/sample_output

Actual Behaviour

The generated bash and zsh completions files are not valid, because either generator uses cmd.get_name() instead of cmd.get_bin_name() as the parent command.

You can see this most easily when trying to execute the generated bash output:

$ bash /tmp/sample_output/mcre_clap_generate.bash
/tmp/sample_output/mcre_clap_generate.bash: line 15: syntax error near unexpected token `for'
/tmp/sample_output/mcre_clap_generate.bash: line 15: `            MCRE for clap generate output,completions)'
$ head -n30 /tmp/sample_output/mcre_clap_generate.bash
_mcre_clap_generate() {
    local i cur prev opts cmd
    COMPREPLY=()
    cur="${COMP_WORDS[COMP_CWORD]}"
    prev="${COMP_WORDS[COMP_CWORD-1]}"
    cmd=""
    opts=""

    for i in ${COMP_WORDS[@]}
    do
        case "${cmd},${i}" in
            ",$1")
                cmd="mcre_clap_generate"
                ;;
            MCRE for clap generate output,completions)
                cmd="MCRE for clap generate output__completions"
                ;;
            MCRE for clap generate output,help)
                cmd="MCRE for clap generate output__help"
                ;;
            MCRE for clap generate output__help,completions)
                cmd="MCRE for clap generate output__help__completions"
                ;;
            MCRE for clap generate output__help,help)
                cmd="MCRE for clap generate output__help__help"
                ;;
            *)
                ;;
        esac
    done

The Zsh generated output is similarly broken; using the shellcheck linter:

$ shellcheck /tmp/sample_output/_mcre_clap_generate 

In /tmp/sample_output/_mcre_clap_generate line 23:
    case $state in
    ^-- SC1009 (info): The mentioned syntax error was in this case expression.


In /tmp/sample_output/_mcre_clap_generate line 24:
    (MCRE for clap generate output)
    ^-- SC1073 (error): Couldn't parse this case item. Fix to allow more checks.
          ^-- SC1072 (error): Expected ) to open a new case item. Fix any mentioned problems and try again.
          ^-- SC1085 (error): Did you forget to move the ;; after extending this case item?

For more information:
  https://www.shellcheck.net/wiki/SC1085 -- Did you forget to move the ;; aft...
  https://www.shellcheck.net/wiki/SC1072 -- Expected ) to open a new case ite...
  https://www.shellcheck.net/wiki/SC1073 -- Couldn't parse this case item. Fi...

Expected Behaviour

The parent name for a completion subcommand should be cmd.get_bin_name() (in the MRCE: mrce_clap_generate), and not cmd.get_name() (MCRE for clap generate output in the MRCE).

If I manually replace MCRE for clap generate output with mcre_clap_generate in the generated output, the completions work as expected.

Additional Context

No response

Debug Output

[clap_builder::builder::command]        Command::_do_parse
[clap_builder::builder::command]        Command::_build: name="MCRE for clap generate output"
[clap_builder::builder::command]        Command::_propagate:MCRE for clap generate output
[clap_builder::builder::command]        Command::_check_help_and_version:MCRE for clap generate output expand_help_tree=false
[clap_builder::builder::command]        Command::long_help_exists
[clap_builder::builder::command]        Command::_check_help_and_version: Building default --help
[clap_builder::builder::command]        Command::_check_help_and_version: Building help subcommand
[clap_builder::builder::command]        Command::_propagate_global_args:MCRE for clap generate output
[clap_builder::builder::debug_asserts]  Command::_debug_asserts
[clap_builder::builder::debug_asserts]  Arg::_debug_asserts:help
[clap_builder::builder::debug_asserts]  Command::_verify_positionals
[clap_builder::parser::parser]  Parser::get_matches_with
[clap_builder::parser::parser]  Parser::get_matches_with: Begin parsing '"completions"'
[clap_builder::parser::parser]  Parser::possible_subcommand: arg=Ok("completions")
[clap_builder::parser::parser]  Parser::get_matches_with: sc=Some("completions")
[clap_builder::parser::parser]  Parser::parse_subcommand
[ clap_builder::output::usage]  Usage::get_required_usage_from: incls=[], matcher=false, incl_last=true
[ clap_builder::output::usage]  Usage::get_required_usage_from: unrolled_reqs=[]
[ clap_builder::output::usage]  Usage::get_required_usage_from: ret_val=[]
[clap_builder::builder::command]        Command::_build_subcommand Setting bin_name of completions to "mcre_clap_generate completions"
[clap_builder::builder::command]        Command::_build_subcommand Setting display_name of completions to "MCRE for clap generate output-completions"
[clap_builder::builder::command]        Command::_build: name="completions"
[clap_builder::builder::command]        Command::_propagate:completions
[clap_builder::builder::command]        Command::_check_help_and_version:completions expand_help_tree=false
[clap_builder::builder::command]        Command::long_help_exists
[clap_builder::builder::command]        Command::_check_help_and_version: Building default --help
[clap_builder::builder::command]        Command::_propagate_global_args:completions
[clap_builder::builder::debug_asserts]  Command::_debug_asserts
[clap_builder::builder::debug_asserts]  Arg::_debug_asserts:output_directory
[clap_builder::builder::debug_asserts]  Arg::_debug_asserts:help
[clap_builder::builder::debug_asserts]  Command::_verify_positionals
[clap_builder::parser::parser]  Parser::parse_subcommand: About to parse sc=completions
[clap_builder::parser::parser]  Parser::get_matches_with
[clap_builder::parser::parser]  Parser::get_matches_with: Begin parsing '"/tmp/sample_output"'
[clap_builder::parser::parser]  Parser::possible_subcommand: arg=Ok("/tmp/sample_output")
[clap_builder::parser::parser]  Parser::get_matches_with: sc=None
[clap_builder::parser::parser]  Parser::get_matches_with: Positional counter...1
[clap_builder::parser::parser]  Parser::get_matches_with: Low index multiples...false
[clap_builder::parser::parser]  Parser::resolve_pending: id="output_directory"
[clap_builder::parser::parser]  Parser::react action=Set, identifier=Some(Index), source=CommandLine
[clap_builder::parser::parser]  Parser::remove_overrides: id="output_directory"
[clap_builder::parser::arg_matcher]     ArgMatcher::start_custom_arg: id="output_directory", source=CommandLine
[clap_builder::builder::command]        Command::groups_for_arg: id="output_directory"
[clap_builder::parser::arg_matcher]     ArgMatcher::start_custom_arg: id="Completions", source=CommandLine
[clap_builder::parser::parser]  Parser::push_arg_values: ["/tmp/sample_output"]
[clap_builder::parser::parser]  Parser::add_single_val_to_arg: cur_idx:=1
[clap_builder::parser::arg_matcher]     ArgMatcher::needs_more_vals: o=output_directory, pending=0
[clap_builder::parser::arg_matcher]     ArgMatcher::needs_more_vals: expected=1, actual=0
[clap_builder::parser::parser]  Parser::react not enough values passed in, leaving it to the validator to complain
[clap_builder::parser::parser]  Parser::add_defaults
[clap_builder::parser::parser]  Parser::add_defaults:iter:output_directory:
[clap_builder::parser::parser]  Parser::add_default_value: doesn't have conditional defaults
[clap_builder::parser::parser]  Parser::add_default_value:iter:output_directory: doesn't have default vals
[clap_builder::parser::parser]  Parser::add_defaults:iter:help:
[clap_builder::parser::parser]  Parser::add_default_value: doesn't have conditional defaults
[clap_builder::parser::parser]  Parser::add_default_value:iter:help: doesn't have default vals
[clap_builder::parser::validator]       Validator::validate
[clap_builder::builder::command]        Command::groups_for_arg: id="output_directory"
[clap_builder::parser::validator]       Conflicts::gather_direct_conflicts id="output_directory", conflicts=[]
[clap_builder::parser::validator]       Conflicts::gather_direct_conflicts id="Completions", conflicts=[]
[clap_builder::parser::validator]       Validator::validate_conflicts
[clap_builder::parser::validator]       Validator::validate_exclusive
[clap_builder::parser::validator]       Validator::validate_conflicts::iter: id="output_directory"
[clap_builder::parser::validator]       Conflicts::gather_conflicts: arg="output_directory"
[clap_builder::parser::validator]       Conflicts::gather_conflicts: conflicts=[]
[clap_builder::parser::validator]       Validator::validate_required: required=ChildGraph([Child { id: "output_directory", children: [] }])
[clap_builder::parser::validator]       Validator::gather_requires
[clap_builder::parser::validator]       Validator::gather_requires:iter:"output_directory"
[clap_builder::parser::validator]       Validator::gather_requires:iter:"Completions"
[clap_builder::parser::validator]       Validator::gather_requires:iter:"Completions":group
[clap_builder::parser::validator]       Validator::validate_required: is_exclusive_present=false
[clap_builder::parser::parser]  Parser::add_defaults
[clap_builder::parser::parser]  Parser::add_defaults:iter:help:
[clap_builder::parser::parser]  Parser::add_default_value: doesn't have conditional defaults
[clap_builder::parser::parser]  Parser::add_default_value:iter:help: doesn't have default vals
[clap_builder::parser::validator]       Validator::validate
[clap_builder::parser::validator]       Validator::validate_conflicts
[clap_builder::parser::validator]       Validator::validate_exclusive
[clap_builder::parser::validator]       Validator::validate_required: required=ChildGraph([])
[clap_builder::parser::validator]       Validator::gather_requires
[clap_builder::parser::validator]       Validator::validate_required: is_exclusive_present=false
[clap_builder::parser::arg_matcher]     ArgMatcher::get_global_values: global_arg_vec=[]
[clap_builder::builder::command]        Command::_build: name="MCRE for clap generate output"
[clap_builder::builder::command]        Command::_propagate:MCRE for clap generate output
[clap_builder::builder::command]        Command::_check_help_and_version:MCRE for clap generate output expand_help_tree=true
[clap_builder::builder::command]        Command::long_help_exists
[clap_builder::builder::command]        Command::_check_help_and_version: Building default --help
[clap_builder::builder::command]        Command::_check_help_and_version: Building help subcommand
[clap_builder::builder::command]        Command::_propagate_global_args:MCRE for clap generate output
[clap_builder::builder::debug_asserts]  Command::_debug_asserts
[clap_builder::builder::debug_asserts]  Arg::_debug_asserts:help
[clap_builder::builder::debug_asserts]  Command::_verify_positionals
[clap_builder::builder::command]        Command::_build: name="completions"
[clap_builder::builder::command]        Command::_propagate:completions
[clap_builder::builder::command]        Command::_check_help_and_version:completions expand_help_tree=true
[clap_builder::builder::command]        Command::long_help_exists
[clap_builder::builder::command]        Command::_check_help_and_version: Building default --help
[clap_builder::builder::command]        Command::_propagate_global_args:completions
[clap_builder::builder::debug_asserts]  Command::_debug_asserts
[clap_builder::builder::debug_asserts]  Arg::_debug_asserts:output_directory
[clap_builder::builder::debug_asserts]  Arg::_debug_asserts:help
[clap_builder::builder::debug_asserts]  Command::_verify_positionals
[clap_builder::builder::command]        Command::_build: name="help"
[clap_builder::builder::command]        Command::_propagate:help
[clap_builder::builder::command]        Command::_check_help_and_version:help expand_help_tree=true
[clap_builder::builder::command]        Command::long_help_exists
[clap_builder::builder::command]        Command::_propagate_global_args:help
[clap_builder::builder::debug_asserts]  Command::_debug_asserts
[clap_builder::builder::debug_asserts]  Command::_verify_positionals
[clap_builder::builder::command]        Command::_build: name="completions"
[clap_builder::builder::command]        Command::_propagate:completions
[clap_builder::builder::command]        Command::_check_help_and_version:completions expand_help_tree=true
[clap_builder::builder::command]        Command::long_help_exists
[clap_builder::builder::command]        Command::_propagate_global_args:completions
[clap_builder::builder::debug_asserts]  Command::_debug_asserts
[clap_builder::builder::debug_asserts]  Command::_verify_positionals
[clap_builder::builder::command]        Command::_build: name="help"
[clap_builder::builder::command]        Command::_propagate:help
[clap_builder::builder::command]        Command::_check_help_and_version:help expand_help_tree=true
[clap_builder::builder::command]        Command::long_help_exists
[clap_builder::builder::command]        Command::_propagate_global_args:help
[clap_builder::builder::debug_asserts]  Command::_debug_asserts
[clap_builder::builder::debug_asserts]  Command::_verify_positionals
[clap_builder::builder::command]        Command::_build_bin_names
[ clap_builder::output::usage]  Usage::get_required_usage_from: incls=[], matcher=false, incl_last=true
[ clap_builder::output::usage]  Usage::get_required_usage_from: unrolled_reqs=[]
[ clap_builder::output::usage]  Usage::get_required_usage_from: ret_val=[]
[clap_builder::builder::command]        Command::_build_bin_names:iter: bin_name set...
[clap_builder::builder::command]        Command::_build_bin_names:iter: Setting usage_name of completions to "mcre_clap_generate completions"
[clap_builder::builder::command]        Command::_build_bin_names:iter: Setting bin_name of completions to "mcre_clap_generate completions"
[clap_builder::builder::command]        Command::_build_bin_names:iter: Setting display_name of completions to "MCRE for clap generate output-completions"
[clap_builder::builder::command]        Command::_build_bin_names
[ clap_builder::output::usage]  Usage::get_required_usage_from: incls=[], matcher=false, incl_last=true
[ clap_builder::output::usage]  Usage::get_required_usage_from: unrolled_reqs=["output_directory"]
[ clap_builder::output::usage]  Usage::get_required_usage_from:iter:"output_directory" arg is_present=false
[ clap_builder::output::usage]  Usage::get_required_usage_from: ret_val=[StyledStr("<OUTPUT_DIRECTORY>")]
[clap_builder::builder::command]        Command::_build_bin_names:iter: bin_name set...
[clap_builder::builder::command]        Command::_build_bin_names:iter: Setting usage_name of help to "mcre_clap_generate help"
[clap_builder::builder::command]        Command::_build_bin_names:iter: Setting bin_name of help to "mcre_clap_generate help"
[clap_builder::builder::command]        Command::_build_bin_names:iter: Setting display_name of help to "MCRE for clap generate output-help"
[clap_builder::builder::command]        Command::_build_bin_names
[ clap_builder::output::usage]  Usage::get_required_usage_from: incls=[], matcher=false, incl_last=true
[ clap_builder::output::usage]  Usage::get_required_usage_from: unrolled_reqs=[]
[ clap_builder::output::usage]  Usage::get_required_usage_from: ret_val=[]
[clap_builder::builder::command]        Command::_build_bin_names:iter: bin_name set...
[clap_builder::builder::command]        Command::_build_bin_names:iter: Setting usage_name of completions to "mcre_clap_generate help completions"
[clap_builder::builder::command]        Command::_build_bin_names:iter: Setting bin_name of completions to "mcre_clap_generate help completions"
[clap_builder::builder::command]        Command::_build_bin_names:iter: Setting display_name of completions to "MCRE for clap generate output-help-completions"
[clap_builder::builder::command]        Command::_build_bin_names
[ clap_builder::output::usage]  Usage::get_required_usage_from: incls=[], matcher=false, incl_last=true
[ clap_builder::output::usage]  Usage::get_required_usage_from: unrolled_reqs=[]
[ clap_builder::output::usage]  Usage::get_required_usage_from: ret_val=[]
[clap_builder::builder::command]        Command::_build_bin_names:iter: bin_name set...
[clap_builder::builder::command]        Command::_build_bin_names:iter: Setting usage_name of help to "mcre_clap_generate help help"
[clap_builder::builder::command]        Command::_build_bin_names:iter: Setting bin_name of help to "mcre_clap_generate help help"
[clap_builder::builder::command]        Command::_build_bin_names:iter: Setting display_name of help to "MCRE for clap generate output-help-help"
[clap_builder::builder::command]        Command::_build_bin_names
[ clap_builder::output::usage]  Usage::get_required_usage_from: incls=[], matcher=false, incl_last=true
[ clap_builder::output::usage]  Usage::get_required_usage_from: unrolled_reqs=[]
[ clap_builder::output::usage]  Usage::get_required_usage_from: ret_val=[]
@mjpieters mjpieters added the C-bug Category: Updating dependencies label Apr 3, 2023
@epage
Copy link
Member

epage commented Apr 3, 2023

bin_name would not work for this

  • Its for usage and includes all parents
  • It can be affected by what the current process is called

What we should do is add an assert that name does not have a space.

@epage epage added M-breaking-change Meta: Implementing or merging this will introduce a breaking change. A-builder Area: Builder API labels Apr 3, 2023
@epage epage added this to the 5.0 milestone Apr 3, 2023
@mjpieters
Copy link
Author

mjpieters commented Apr 3, 2023

bin_name would not work for this

  • Its for usage and includes all parents
  • It can be affected by what the current process is called

Then why is the second half of the generated output using get_bin_name(), via utils::subcommands()? Because those are valid:

$ tail -n75 /tmp/sample_output/mcre_clap_generate.bash
    case "${cmd}" in
        mcre_clap_generate)
            opts="-h --help completions help"
            if [[ ${cur} == -* || ${COMP_CWORD} -eq 1 ]] ; then
                COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
                return 0
            fi
            case "${prev}" in
                *)
                    COMPREPLY=()
                    ;;
            esac
            COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
            return 0
            ;;
        mcre_clap_generate__completions)
            opts="-h --help <OUTPUT_DIRECTORY>"
            if [[ ${cur} == -* || ${COMP_CWORD} -eq 2 ]] ; then
                COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
                return 0
            fi
            case "${prev}" in
                *)
                    COMPREPLY=()
                    ;;
            esac
            COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
            return 0
            ;;
        mcre_clap_generate__help)
            opts="completions help"
            if [[ ${cur} == -* || ${COMP_CWORD} -eq 2 ]] ; then
                COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
                return 0
            fi
            case "${prev}" in
                *)
                    COMPREPLY=()
                    ;;
            esac
            COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
            return 0
            ;;
        mcre_clap_generate__help__completions)
            opts=""
            if [[ ${cur} == -* || ${COMP_CWORD} -eq 3 ]] ; then
                COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
                return 0
            fi
            case "${prev}" in
                *)
                    COMPREPLY=()
                    ;;
            esac
            COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
            return 0
            ;;
        mcre_clap_generate__help__help)
            opts=""
            if [[ ${cur} == -* || ${COMP_CWORD} -eq 3 ]] ; then
                COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
                return 0
            fi
            case "${prev}" in
                *)
                    COMPREPLY=()
                    ;;
            esac
            COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
            return 0
            ;;
    esac
}

@mjpieters
Copy link
Author

I've understood the structure of the bash completion function to be roughly this:

    for i in ${COMP_WORDS[@]}
    do
        case "${cmd},${i}" in
            # expand cmd to cmd__possible_part2_word
        esac
    done

    case "${cmd}" in
        # provide completion words for each group and subcommand
    esac

The first part is generated by shells::bash::Bash::all_subcommands(), the second part mostly by shells::bash::Bash::subcommand_details() (the first option uses cmd.get_bin_name()). The latter uses crate::generator::utils::all_subcommands(), a method that produces a vector of vec![sc.get_name().to_string(), sc.get_bin_name().unwrap().to_string()] two-value vectors, and the details are generated using only the second element from those (so sc.get_bin_name().unwrap().to_string()).

Hence my assumption that the first part should also use those exact same strings.

@tv42
Copy link

tv42 commented Nov 12, 2023

I triggered something very similar by having a crate named foo_bar (because foo was taken), with the binary in src/bin/foo/main.rs and thus named simply foo. I called clap_complete::generate("bash", &mut command, "foo", &mut std::io::stdout()); and the completion looked like

    for i in ${COMP_WORDS[@]}
    do
        case "${cmd},${i}" in
            ",$1")
                cmd="foo"
                ;;
            foo__bar,quux)
                cmd="foo__bar__quux"
                ;;

Once I added #[command(name = "foo")] into my derived clap::Parser struct, the completion script switched from foo__bar to foo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-builder Area: Builder API C-bug Category: Updating dependencies M-breaking-change Meta: Implementing or merging this will introduce a breaking change.
Projects
None yet
Development

No branches or pull requests

3 participants