clearloop / leetcode-cli

May the code be with you 👻
MIT License
308 stars 49 forks source link

fix: shell detection message always outputed when generating completion #131

Closed Subjective closed 10 months ago

Subjective commented 10 months ago

It seems like the shell detection message is always outputed when generating shell completions, regardless of whether the user provided a shell or not. The issue is that the println() is always being executed when it is inside the closure provided to the unwrap_or, event when a valid shell is provided. I'm not familiar with rust, so I don't know why this is the case.

More importantly though, even if the message is ouptuted correctly, it actually prevents the completion from detected correctly (at least for zsh) when added to a file in the fpath like so: $ leetcode completions zsh >! /usr/local/share/zsh/site-functions/_leetcode

That line has to be manually deleted since it comes before #compdef leetcode

# Since shell arg value is not provided trying to get the default shell from the environment.
#compdef leetcode
...

For these reasons, I think it would be best to just not output it at all and include that info in the README.

If you would prefer to keep the detection message, the following snippet will correctly output the message at the end of the file, but is no where near as elegant.

pub fn completion_handler(m: &ArgMatches, cmd: &mut ClapCommand) -> Result<(), Error> {
    let shell_result = m.get_one::<Shell>("shell");
    if let Some(shell) = shell_result {
        // Shell argument is provided, use it directly
        let completions = get_completions_string(*shell, cmd)?;
        println!("{}", completions);
    } else {
        // Shell argument is not provided, fall back to the default shell from the environment
        let shell = Shell::from_env().ok_or(Error::MatchError)?;
        let completions = get_completions_string(shell, cmd)?;
        println!("{}", completions);
        println!("# Since shell arg value is not provided trying to get the default shell from the environment.");
    }
    Ok(())
}

Let me know what you think. Thanks.

Subjective commented 10 months ago

Hmmm, I'm not entirely sure why the ci/cd build fails for macOS, it seems to work locally on my own system

clearloop commented 10 months ago

Hmmm, I'm not entirely sure why the ci/cd build fails for macOS, it seems to work locally on my own system

nevermind, I'll fix it later