-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
display aliases in cargo --list
#8486
Comments
Hi @ehuss I would like to work on this (if possible) but I might need some help. As far as I've seen, the only place where the aliases are listed is in Therefore, I'm not sure if the aliases should pass to be handled more consistently (so if more of them get added, it's easier to make it happen without needing to touch strings and functions in different places) or instead, we should just edit the Another option is to create a list with the aliases and make the functions that interact with them point to that list. Line 92 in f84f3f8
Avaliable aliases section which will collect the data from the aliases list.
Personally, I belive it would be nice to get something like:
|
Thanks @CPerezz! I would probably try to extract the hard-coded table (with b, c, r, t) into a separate function (or constant), so that it can be shared with the list function. As for the output, the different headings might be nice, but since this tends to get consumed by scripts, I don't think we can add them at this time. Perhaps some time in the future a flag could be added to specify the output style, and a specifically machine-readable format could be added, giving more flexibility. |
Hey @ehuss I have it working. There's only one thing I'm not sure about. Since we use a Line 77 in aa68721
we get them on this way actually:
So the aliases and the commands are both mixed since the I can think about a couple of things now:
for cmd in commands::builtin() {
commands.insert(CommandInfo::BuiltIn {
name: cmd.get_name().to_string(),
about: cmd.p.meta.about.map(|s| s.to_string()),
});
}
for command in &BUILTIN_ALIASES {
commands.insert(CommandInfo::BuiltIn {
name: command.0.to_string(),
about: Some(command.1.to_string()),
});
} in: Line 74 in aa68721
Line 91 in aa68721
Any suggestions? Since looks like all of this aliases stuff should get a refactor and propper struct supporting so it's less tricky to work with it and expand it on the future. But I'm new to Cargo so I'm not sure at all on which is the direction you guys want to take. |
Hm, I don't have a strong preference either way. Whichever way appeals to you. I'm fine with it being alphabetized, and I'm also fine if you want to keep them separate. (Sorry for the non-answer! 😄 ) |
The code will be easier to read and all in one file if we stay with this solution. But at some point in the future I believe a refactor will be needed in order to make easier and more readable the interaction with builtin aliases. Thanks!! Pushing the PR then! |
As stated in rust-lang#8486 it would help to the discovery of the builtin aliases the facto of printing them with the `cargo --list` command. - Extracted the builtin aliases currently implemented to a sepparated `const`. - Make all of the functions that interact with these aliases point to that function. - Refactored the `list_commands` fn in order to include with the builtin and external commands, the builtin aliases that come with cargo by defaut.
Display builtin aliases with `cargo --list` As stated in #8486 it would help to the discovery of the builtin aliases the facto of printing them with the `cargo --list` command. - Extracted the builtin aliases currently implemented to a separated `const`. - Make all of the functions that interact with these aliases point to that function. - Refactored the `list_commands` fn in order to include with the builtin and external commands, the builtin aliases that come with cargo by default. - Added a test that checks that the aliases that currently are builtin with cargo are indeed being printed with the rest of the commands when `cargo --list` is called. The output on my machine looks like this: ``` $ cargo --list Installed Commands: b alias: build bench Execute all benchmarks of a local package build Compile a local package and all of its dependencies c alias: check check Check a local package and all of its dependencies for errors clean Remove artifacts that cargo has generated in the past doc Build a package's documentation fetch Fetch dependencies of a package from the network fix Automatically fix lint warnings reported by rustc generate-lockfile Generate the lockfile for a package git-checkout This subcommand has been removed init Create a new cargo package in an existing directory install Install a Rust binary. Default location is $HOME/.cargo/bin locate-project Print a JSON representation of a Cargo.toml file's location login Save an api token from the registry locally. If token is not specified, it will be read from stdin. metadata Output the resolved dependencies of a package, the concrete used versions including overrides, in machine-readable format new Create a new cargo package at <path> owner Manage the owners of a crate on the registry package Assemble the local package into a distributable tarball pkgid Print a fully qualified package specification publish Upload a package to the registry r alias: run read-manifest Print a JSON representation of a Cargo.toml manifest. run Run a binary or example of the local package rustc Compile a package, and pass extra options to the compiler rustdoc Build a package's documentation, using specified custom flags. search Search packages in crates.io t alias: test test Execute all unit and integration tests and build examples of a local package tree Display a tree visualization of a dependency graph uninstall Remove a Rust binary update Update dependencies as recorded in the local lock file vendor Vendor all dependencies for a project locally verify-project Check correctness of crate manifest version Show version information yank Remove a pushed crate from the index clippy clippy clippy clippy flamegraph fmt fmt fmt fmt miri miri miri miri outdated tree ``` As discussed with @ehuss the `BTreeSet` enforces `Ord` therefore, the aliases get mixed with the commands since they're passed through the same function. It can be refactored to appear separately, but, the code will be more spread and now it's all in just one file (which I believe is easier to maintain and review). Closes #8486
The aliases that now are shown appart of the builtin ones are the cargo extensions that the user has installed. So for example, this is what I get when executing
What would you like to add here @ehuss ? If I understood correctly, you would also like to list the aliases added in If that's the case, I'll work on it. The only thing is that since the output can be larger now, I would probably like to make a slightly bigger refactor so that we can print the aliases in different sections or rather, just have an Arg for the aliases. WDYT? |
Yes, that is correct. If someone has this in their config:
Then
I would not expect most users to have a lot of aliases, so I wouldn't worry too much about that. Fixing #6088 would probably go a much longer way towards improving the output. I'm uncertain about changing the structure of the output. There are tools that parse the output of the command, and changing the structure would break them. It would definitely be good to add a parseable format, but I think that will require a long transition time before we could change the default output. You're welcome to work on that, but I think that is a separate concern. |
Hi @ehuss. Sorry for the long delay. I've been extremely busy and also sick for some time. It's easier to parse and this type of separation doesn't seem to provide any advantages to me. (At least that I'm aware obviously). |
No worries, please take care of yourself first. Cargo doesn't have any sort of shell parsing code, so it is unable to understand "quoted strings" or escaping or anything like that. To support arguments with spaces, the alias must use TOML's array syntax so that each argument is a separate string. |
Hi @ehuss I've been putting a bit of time into this. I have a couple of questions. if you have some time.
// Add the Manifest-defined aliases.
match config.get_file_path(config.home().as_path_unlocked(), "config", true) {
Ok(Some(config_path)) => {
let toml_contents = crate::util::toml::parse(
&util::paths::read(&config_path).expect("Error on read"),
&config_path,
&config,
)
.unwrap();
// Get aliases and add them to the BTreeSet
}
_ => (),
}
Having the following in my [alias] # command aliases
dnd = ["doc", "--no-deps"] This step returns the following (making it panic to get the print) with :
It's really weird since using TOML format I get this, which contains these On the other side, If instead I have: [alias] # command aliases
dnd = "doc --no-deps" I get
which seems a way easier to parse. I just would appreciate advice on which you consider the best approach or indeed if there's any other one that I'm not considering and would work better. It would also be helpful to know if you're only interested in Thanks a lot. |
Sorry, I'm not sure I follow the questions about loading config files. The config is already loaded. I would start by changing |
Should I take the opportunity to migrate the |
I would probably avoid major changes like that for now. |
Perfect!! |
@rustbot claim |
Include aliases with other commands The principal result is that they are now automatically included in the `cargo --list` output. Fixes #8486.
This demo file was originally from a demo I made back on July 30. I figured out how to get a sort of 'npm run dev' kind of thing working, I really like it! This kind of quick-feedback programming is how I get things done fast, as I can easily see when things go right, or wrong. https://doc.rust-lang.org/rust-by-example/custom_types/structs.html https://doc.rust-lang.org/book/ch16-01-threads.html https://www.reddit.com/r/rust/comments/9twt89/show_rrust_cargocmd_like_npm_scripts_but_for_cargo/ (I was curious if you could do something like `cargo run dev` for this kind of hot reload script, but it didn't seem to work from what I tried. It is possible it sounds like, but I don't think I'm implementing it right. Going to use a Bash script for the meantime. I want it to work cross-platform though, so you can use this on Windows too and not have to do anything special for that. I know I could make a Batch file, but I'd rather use simple tooling with Cargo than roll my own thing for that. That's the same case with npm as well, to use the tools that are available to you! :D) https://doc.rust-lang.org/cargo/reference/config.html#alias https://pseitz.github.io/toml-to-json-online-converter/ (Neat concept, finding the similarities between TOML, JSON, and SNBT/NBT) rust-lang/cargo#9061 rust-lang/cargo#8486 (comment) (Is my alias not working because of how I defined it in the config?)
It can be confusing that there are commands (aliases) available that are not displayed with
cargo --list
. I think displaying these can help with discovery, and make it clearer that certain commands are actually aliases.The tricky bit is exactly how it should be exposed. There are tools and scripts that parse the output of
cargo --list
, which unfortunately doesn't have an explicit flag for setting the output format (such as a machine-readable format). I think something like this should work:The text was updated successfully, but these errors were encountered: