generate_to ignores given argument and derives its own

[alerque]

Trying to get shell completion generators working on my app (porting from StructOpt) I ran into trouble using the generate_to() function.

clap/clap_generate/src/lib.rs

Lines 124 to 139 in 306dc83

	pub fn generate_to<G, S, T>(app: &mut clap::App, bin_name: S, out_dir: T)
	where
	G: Generator,
	S: Into<String>,
	T: Into<OsString>,
	{
	let out_dir = PathBuf::from(out_dir.into());
	let file_name = G::file_name(app.get_bin_name().unwrap());
	let mut file = match File::create(out_dir.join(file_name)) {
	Err(why) => panic!("couldn't create completion file: {}", why),
	Ok(file) => file,
	};
	generate::<G, S>(app, bin_name, &mut file)
	}

Even with code pretty much copied out of the docs (marked as ignore):

let outdir = match env::var_os("OUT_DIR") {
    None => return,
    Some(outdir) => outdir,
};

let mut app = build_cli();
generate_to::<Bash, _, _>(
    &mut app,     // We need to specify what generator to use
    "myapp",      // We need to specify the bin name manually
    outdir,       // We need to specify where to write to
);

This code explicitly passes a value for bin_name, yet it is unused. L130 of the function above is fetching the bin name out of the app definition and using that to build a file path, ignoring the value we passed.

Furthermore if the app doesn't happen to have a bin_name set already, the code will panic!

[pksunkara]

Thanks for reporting. This actually ties into #1764. We might want just one file name but different bin names.

[kbknapp]

I'm trying to remember exactly, but I think the reason I originally had the user pass in the binary name was because the App's bin_name doesn't get determined until runtime, however many shell completion script generations happen in a build.rs at compile time. So we need a way to tell the generating code what the binary would be called.

Of course this can be fixed by making App::bin_name or App::set_bin_name mandatory (side note we should fix the naming of those methods so they're less confusing. I know they one is &self and returns Sefl while other is just &mut self...I think the Rust idiom here is to make the &mut self's method name end in _mut...but I digress). But that should be well documented, or this invariant should be checked inside genereate_* methods via assert!s.

[saecki]

So just a follow up as I encountered this myself. How about using the passed bin_name as a file name if the app doesn't have one. Or providing a more helpful error message with expect instead of unwrap that says "App is missing a bin_name, consider adding one by using App::bin_name" or something similar.