CLI can panic on bad input

[christophermaier]

Example:

sudo hab sup ru 
thread 'main' panicked at 'internal error: entered unreachable code', components/hab/src/main.rs:269:18
note: Run with `RUST_BACKTRACE=1` for a backtrace.

I just got ahead of myself and hit Enter before I typed the "n" for "run".

This shouldn't be a panic; it should just give a usage error.

The cause of this specific panic can be found here:

habitat/components/hab/src/main.rs

Lines 261 to 269 in dc65012

	("sup", Some(m)) => match m.subcommand() {
	("depart", Some(m)) => sub_sup_depart(m)?,
	("secret", Some(m)) => match m.subcommand() {
	("generate", _) => sub_sup_secret_generate()?,
	_ => unreachable!(),
	},
	// this is effectively an alias of `hab svc status`
	("status", Some(m)) => sub_svc_status(m)?,
	_ => unreachable!(),

However, it looks like we get to that point because we failed to do anything in exec_subcommand_if_called, specifically here:

habitat/components/hab/src/main.rs

Lines 1272 to 1274 in dc65012

	// Delegate `hab sup run *` to the Launcher
	("sup", "run", _) => command::launcher::start(ui, env::args_os().skip(2).collect()),
	_ => Ok(()),

BUT NOTE! If you run hab sup runs, which is also not a command, you get an error message!

error: The subcommand 'runs' wasn't recognized
	Did you mean 'run'?

Something seems to be rather tangled up in our CLI processing here.

(This works on Habitat 0.74.0)

[baumanj]

Removing this line defining aliases fixes it. I think we should remove it, as well as all the others. If users want to define their own aliases, that's simple for them to do, whereas baking in support for commands like

$ hab r k e

doesn't seem like good UX

[christophermaier]

Ah, good find! Yeah, I kinda want to rip out most (if not all) the aliases.

[raskchanky]

I agree with @baumanj. In addition, I think the existence of exec_subcommand_if_called is an anti-pattern. That caused a fair amount of headaches when implementing a certain feature. In my opinion, the idea that we deliberately circumvent the work that clap is doing, and do it ourselves, just so that we can exec a different process a little faster, is shooting ourselves in the foot and making things harder than they need to be.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We value your input and contribution. Please leave a comment if this issue still affects you.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We value your input and contribution. Please leave a comment if this issue still affects you.

[stale]

This issue has been automatically closed after being stale for 400 days. We still value your input and contribution. Please re-open the issue if desired and leave a comment with details.