Make cargo --list combine commands and their aliases

[ToBinio]

Problem

Currently cargo --list splits the command itself and its aliase in seperate lines

b                    alias: build
bench                Execute all benchmarks of a local package
build                Compile a local package and all of its dependencies
build-man            alias: run --package xtask-build-man --
c                    alias: check

Proposed Solution

i would say this should be changed to a simular / the same system as clap has.
combining the command and its aliase

build, b    Compile the current package
check, c    Analyze the current package and report errors, but don't build object files
clean       Remove the target directory
doc, d      Build this package's and its dependencies' documentation

this behavior can already be seen in cargo --help

Notes

No response

[weihanglo]

As you can see, Cargo provides the ability to define custom aliases.

build-man            alias: run --package xtask-build-man --

If we put the alias side by side on the same line of the original command name, it might be a bit confusing seeing other custom aliases shows on their own lines.

I would suggest the followings

• Move built-in aliases to the same line as you suggested.
• Add a new section “User-defined aliases” and display all custom aliases there.

Installed Commands:
    add                  Add dependencies to a Cargo.toml manifest file
    bench                Execute all benchmarks of a local package
    build, b             Compile a local package and all of its dependencies
    ...

User-defined aliases:
    build-man            alias: run --package xtask-build-man --
    ...

Some pointers:

• The code collecting commands and aliases. We may split it into two functions, list_commands and list_aliases.

  cargo/src/bin/cargo/main.rs

  Line 87 in e2d1488

  fn list_commands(config: &Config) -> BTreeMap<String, CommandInfo> {

• Constants of built-in aliases. This may need some refactors if we merge their displays into their built-in command counterparts. Nee

  cargo/src/bin/cargo/main.rs

  Line 43 in e2d1488

  const BUILTIN_ALIASES: [(&str, &str, &str); 6] = [

[weihanglo]

git help --all is a good reference btw.

[ToBinio]

what do you think about also spitting commands installed through cargo install into their own separate section?

then there would be 'Built-In commands', 'User-defined aliases', and 'Installed commands'

[weihanglo]

Sounds good to me!

[ToBinio]

I would like to try implementing this my self.

is there anything else I should do first / pay attention to?

[weihanglo]

You can use @rustbot claim to assign this issue to yourself. See https://github.com/rust-lang/triagebot/wiki/Assignment.

Feel free to hack it. If you have any question, ask here, open a topic on T-cargo, or ping me on Zulip.

[ToBinio]

@rustbot claim

[ehuss]

Just beware that there are tools which parse the output of --list. For example, shell completions (bash) will use it to know which commands exist. Although it is probably ill-advised to parse a human-readable output, since it has had such a simple and well-structured form for so long, and no alternatives with a machine-readable format, there will likely be various things that try to parse it.

I think we need to be careful with making too extreme of changes. Some suggestions:

1. Test things like the bash/zsh/fish completions. How badly will they be broken? Can they be easily fixed?
2. Consider keeping the changes minimal to reduce the impact.
3. Are there use cases other than shell-completions? Try some code searches to see if you can find other examples.
4. Consider what it would take to support a machine-readable format to give an alternative to tools which need to discover this information. How hard would it be to create a format that we could agree to use? It seems like an extensible format shouldn't be too hard to make.

[weihanglo]

I just recall that built-in aliases are able to get overridden by custom aliases. It will be a bit weird to put build, b Compile a package… in one line if b got overridden. See this test case:

cargo/tests/testsuite/cargo_alias_config.rs

Lines 362 to 385 in e2d1488

	#[cargo_test]
	fn alias_override_builtin_alias() {
	let p = project()
	.file("Cargo.toml", &basic_bin_manifest("foo"))
	.file("src/main.rs", "fn main() {}")
	.file(
	".cargo/config",
	r#"
	[alias]
	b = "run"
	"#,
	)
	.build();
	p.cargo("b")
	.with_stderr(
	"\
	[COMPILING] foo v0.5.0 ([..])
	[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
	[RUNNING] `target/debug/foo[EXE]`
	",
	)
	.run();
	}

Given it needs more inputs than I thought, I'll label as E-medium to reflect the effort level.

[ToBinio]

ok, I just finished creating a basic working version. which does not handle the consistency issue called out by ehuss or the overwriting behavior.

should I make a draft so somebody can have a look at it?

how should we handle the consistency issue? maby making the new --list format optional so you use e.g --list-formated?

[weihanglo]

A draft PR seems good to me, though I would highly recommend looking into ehuss's suggestions first.

Looks like fish is immune from adding new sections. However bash and zsh are not. I wonder if we can make bash and zsh do a similar filter-and-replace way. Then we ship it first. Wait it until hitting stable, and then release our --list output format change. I feel like this could reduce the inevitable impact for most bash/zsh users.

We should also check some major shell framework (ohmyzsh, bash-completions, etc) if they vendor our shell completions, and inform them about the change upfront, or just submit patches to them.

how should we handle the consistency issue? maby making the new --list format optional so you use e.g --list-formated?

Personally, I'd like to see cargo help becoming a better help command like git help. For instance, we could have cargo help --alias to output aliases only. cargo help --external-subcommands --message-format=json to output 3rd-party command in JSON format. Probably we can slowly phase out cargo --list and suggest cargo help --list instead. However, I think this deserves another new issue.

[ToBinio]

what would you think about not doing any changes to the current cargo --list and instead starting the migration to cargo help --list and its subcommands?
It could also be nice to replace --list with --command or something in this direction to make its usage clearer.

we should then make it clear that cargo --list got deprecated and will be removed at some point. Maybe in the 2024 release?

this should enable the cleanest migration... I think.

[weihanglo]

That would be really plausible! I believe it is highly possible to accept the proposal as an unstable feature. You can follow the instruction here and code-level guide here to make new unstable flags. I bet -Zunstable-options would fit this case.

In order to continue the discussion before hacking, I would recommend making a tiny semiformal design proposal (not RFC), similar to #11099. It should list what is going to be added and what is the expected behaivour. Prior arts from other CLI tools, etc. It doesn't need to be that long but informative.

[ToBinio]

should I open a new issue for this proposal because the scope slightly changed?
or would I be ok to keep it in this issue?

did I understand correctly that this proposal should explain which commands/flags would be added and how there would work? it is not supposed to explain how the code itself should look.

would be ok to change some of the current cargo --list code without changing the behavior of cargo --list? Or would it be better to have some more or less duplicated code for the time? To give some context I am talking about the change to use multiple functions for receiving what commands do exist.

The code collecting commands and aliases. We may split it into two functions, list_commands and list_aliases.

[epage]

what do you think about also spitting commands installed through cargo install into their own separate section?

Some quick thoughts

• Some commands are in between, like cargo clippy and cargo fmt
• We should probably define why we should treat these two cases differently before doing it

To add to ehuss' comments, my suggestion would be to prioritize what you want to see changed and do it incrementally (at least one change per PR) so we can evaluate each one and decide whether to keep it or not or so we can easily revert the parts of it later.

It'd also be good for us to have #11912 merged first but its stuck in limbo

[weihanglo]

should I open a new issue for this proposal because the scope slightly changed? or would I be ok to keep it in this issue?

If it is a completely different stuff. I'd suggest open a new one and maybe close this if it is not needed anymore.

did I understand correctly that this proposal should explain which commands/flags would be added and how there would work? it is not supposed to explain how the code itself should look.

Yes. We can discuss starting at high level but you may want to take epage's advice: Do it incrementally. The proposal needn't be a gigantic one.

would be ok to change some of the current cargo --list code without changing the behavior of cargo --list? Or would it be better to have some more or less duplicated code for the time? To give some context I am talking about the change to use multiple functions for receiving what commands do exist.

If you find any refactor making sense, helping the code more structured and easy to reason, feel free to do so!