App name argument propogated to child app

[anowell]

The child process is being called with the clap app name as the first arg. I printed env::args() when the process starts so you can see that.

At quick glance, I see the app name is added to the arg list in Klask::execute (here) which didn't seem right. As a quick test, it appeared to work for me if I replaced that line and kill the subsequent validation check:

    fn execute(&mut self) -> Result<ChildApp, ExecuteError> {
        // let args = self.state.get_cmd_args(vec![self.app.get_name().into()])?;
        let args = self.state.get_cmd_args(vec![])?;

        // Check for validation errors - disabled since the validation expects the app name arg
        // self.app.clone().try_get_matches_from(args.iter())?;

But even though that works for me, I feel like I'm missing something since it seems like there is an intention to use the app name argument to distinguish some "outer" subcommand, but I didn't completely grok how that works (why wouldn't run_app always start the GUI and just put the logic on the calling code to decide when to call run_app?

[MichalGniadek]

The idea is that I wrap clap app in another clap app and treat it as a subcommand. And then when someone calls prog.exe I show a GUI and when prog.exe prog <options> is called I pass it to the user code.

So, just right-clicking the program opens up a GUI. But also if someone has a clap app with only optional arguments, it still works.

#[derive(Debug, Clap)]
struct Test {
    #[clap(long)]
    opt1: Option<String>,
    #[clap(long)]
    opt2: Option<String>,
}

For example, without this subcommand trick the above code launches new GUIs every time Run is pressed with no arguments. I hope that clears things up. If you have any ideas on how to improve this (admittedly kinda janky) setup, I'm open to suggestions!

[anowell]

Ah. I see now. I was still using clap to parse args explicitly and only calling klask when I wanted GUI rendered. So it was clap's Opt::parse() here that exited:

fn main() -> Result<()> {
    println!("{:?}", env::args());
    let opts = Opts::parse();

    match opts.issue {
        None => {
            let settings = klask::Settings { enable_env: Some("Env Vars".to_string()), ..Default::default() };
            klask::run_derived::<Opts, _>(settings, |_opts| println!("Welcome to JEM"));
        },
        Some(issue) => {
            // Handle --issue

While it's your library to take in the direction you want, reasons against hijacking args include:

• Clap exposes parse for convenience (exits on parsing error) but also try_parse for more custom handling of parsing errors.
• It may break code that checks env::arg or env::args directly (e.g. to count args) - or in my case, using clap directly.
• It makes it slightly more complicated to conditionally compile in support for klask (one path calls clap to parse, the other klask)

Possible alternatives:

• Environment variable; set a var (e.g. KLASK_CHILD=true) on the child process to detect child processes; Downside is that all subsequent child process inherit this variable by default (e.g. meaning an app would need to call cmd.env_remove("KLASK_CHILD") if they wanted one klask app to launch a different klask app). Is hijacking env::var less janky then env::arg?
• Parent PID: Have klask figure out the path associated with the parent pid, and if it matches the path of the current process pid, then you know the the app was launched by itself. For unix, see parent_id and pidpath. Windows seems a bit more complicated, but possible: see Process::path and process32first (msdn)

Anyhow, given that I figured out that I was just using klask wrong, feel free to close this if you want to stick with the arg-injection approach. Though, it might be at least worth documenting that usage of klask::run_* should replace any existing arg parsing since klask injects an argument that it silently consumes. Cheers!

[MichalGniadek]

Thanks for the feedback! I think switching to an environment variable is a good idea. I could probably use std::env::remove_var to remove it automatically (at least when someone is using klask::run_*) so recursive apps shouldn't be a problem. Using parent pid seems a bit too complicated.
Also, just as a note, try_parse should always return Ok because I'm doing the validation with try_get_matches_from before I start the child process.

EDIT: I have implemented environment vars in #16, so if you want, you can check if it works for your use case.