Skip to content
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

Add short flag for workspace #11549

Closed
wants to merge 1 commit into from
Closed

Add short flag for workspace #11549

wants to merge 1 commit into from

Conversation

SUPERCILEX
Copy link

I have no idea what the feature roadmap is, so adding a short flag might conflict with future plans. If it doesn't though, it'd be nice to have.

@rustbot
Copy link
Collaborator

rustbot commented Jan 6, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @weihanglo (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 6, 2023
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about my own stance on this. It seems not harmful, but people may find it a bikeshedding problem. Could you provide more sources to support this change? Such as

  • How -w is used for in other command line tools. This is to avoid future conflicts for a well-recognized meaning over -w.
  • What the most common shorthand flag is for workspace-like operation from other package manager.
  • How frequent this flag is used and people are tired of typing it out.

Thank you anyway for posting this.

@SUPERCILEX
Copy link
Author

How frequent this flag is used and people are tired of typing it out.

In my workspaces, basically always since I'm running cargo t --workspace constantly.

What the most common shorthand flag is for workspace-like operation from other package manager.

I don't think pip or bundler have workspaces, but npm does and uses the -w flag:

$ npm run --help
Run arbitrary package scripts

Usage:
npm run-script <command> [-- <args>]

Options:
[-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
[-ws|--workspaces] [--include-workspace-root] [--if-present] [--ignore-scripts]
[--foreground-scripts] [--script-shell <script-shell>]

aliases: run, rum, urn

Run "npm help run-script" for more info

How -w is used for in other command line tools. This is to avoid future conflicts for a well-recognized meaning over -w.

Honestly I don't think random tool X's flag naming is relevant as they're different domains.

@epage
Copy link
Contributor

epage commented Jan 8, 2023

First off, this is a lot of requirements / design discussion in a PR when typically we ask for this to happen in an Issue before a PR is ever created, see

I don't think pip or bundler have workspaces, but npm does and uses the -w flag:

For an example list of other tools we sometimes consider for prior art, see cargo-add's PR

Honestly I don't think random tool X's flag naming is relevant as they're different domains.

Its very relevant on two fronts

  • There are common conventions that could confuse people if we appropriated one. -f is nearly synonymous with --force that if we used it for something else, it would cause confusion.
  • We have a lot of overlap in functionality with various domains, so looking to other commands for flags they use could be useful to give ideas for potential flags in cargo. For example, say this was before cargo install -f was added, we would want to look at flags and consider "would we ever have a --force flag? Maybe we should follow conventions for it?

One resource I commonly look to is https://www.gnu.org/prep/standards/html_node/Option-Table.html#Option-Table

Some other questions

  • Do we use the -w short flag in any existing command such that a new command using both would run into conflicts?
  • Are there any other existing flags that a -w short flag would be useful for? This is both an extension of the above question and it is important because a user might confuse the two as well.

@epage
Copy link
Contributor

epage commented Jan 8, 2023

Regarding the implementation of this, it'd be worth checking the PR of the last major short flag we added (sadly, most of the research for what we considered when adding it was not captured in that PR or the one it references).

@SUPERCILEX
Copy link
Author

Maybe we should follow conventions for it?

I don't recall using a -w flag in recent memory and the gnu resource you shared uses -w for a bunch of esoteric options I'd never heard of, so I'm fairly confident in saying it's not a common flag name.

Do we use the -w short flag in any existing command such that a new command using both would run into conflicts?

Is there a command reference somewhere? Otherwise I can grep help pages.

Are there any other existing flags that a -w short flag would be useful for?

More useful than --workspace? Highly doubtful since workspaces are a core feature of cargo.

@epage
Copy link
Contributor

epage commented Jan 8, 2023

Is there a command reference somewhere? Otherwise I can grep help pages.

https://doc.rust-lang.org/cargo/commands/index.html

Are there any other existing flags that a -w short flag would be useful for?

More useful than --workspace? Highly doubtful since workspaces are a core feature of cargo.

I did not say "more useful" but "useful". Like I said, if -w would be meaningful enough in another context, then it could be confusing as to what -w means.

@weihanglo
Copy link
Member

How -w is used for in other command line tools. This is to avoid future conflicts for a well-recognized meaning over -w.

Honestly I don't think random tool X's flag naming is relevant as they're different domains.

Not exactly. -w is often a shorthand of --watch. Here is a summary of tools with -w in JavaScript land (and many more for newer tools). Outside of that, kubectl has -w as a shorthand of --watch. dune, a build system for OCaml, also got the same -w for watch mode. So does tsc, or TypeScript Compiler.

Just try providing some data points. Some of them are relevant tools I feel like, though that doesn't imply Cargo is going to use -w as watch mode.

@SUPERCILEX
Copy link
Author

Is there a command reference somewhere? Otherwise I can grep help pages.

doc.rust-lang.org/cargo/commands/index.html

Boom: https://doc.rust-lang.org/cargo/commands/cargo-update.html. We're already using -w for --workspace, so making -w universal gets extra points for consistency.

I did not say "more useful" but "useful". Like I said, if -w would be meaningful enough in another context, then it could be confusing as to what -w means.

Sure, but I can't predict the future and don't know the roadmap. That's on you guys.

-w is often a shorthand of --watch

Great point, that's a good possible future direction. So then it's up to you guys: do you think cargo will ever have a watch feature? I personally don't see much value for Rust and we already have https://crates.io/crates/cargo-watch.

@epage
Copy link
Contributor

epage commented Jan 8, 2023

Boom: https://doc.rust-lang.org/cargo/commands/cargo-update.html. We're already using -w for --workspace, so making -w universal gets extra points for consistency.

Huh, never noticed that had a --workspace flag.

However, I will not that text communication can be hard and starting with "boom" like this can come across as trying to "win an argument" when there is no argument here but trying to gather the relevant information for making the best decision for cargo (as we can't change this due to compatibility).

-w is often a shorthand of --watch

Great point, that's a good possible future direction. So then it's up to you guys: do you think cargo will ever have a watch feature? I personally don't see much value for Rust and we already have https://crates.io/crates/cargo-watch.

Watch support is being discussed at #9339

Again, to be clear, this doesn't mean that this will necessarily be rejected or that it is that one is more important than the other but that we need to weigh in potential ambiguity.

@SUPERCILEX
Copy link
Author

when there is no argument here but trying to gather the relevant information for making the best decision

Right, and the fact that a -w flag already exists is a huge seller. There's a precedent.

potential ambiguity

I think this is the key question then: if watch were to become a thing, would it be a flag or a command (like cargo-watch)? If it would be a flag, then this PR has to wait for #9339 to be resolved. If it'd be added as a command, then I think -w can coexist just fine. I took a look at the other linked --watch tools, and they all appear to use booleans which suggests watch as a command is fine.

@SUPERCILEX
Copy link
Author

the fact that a -w flag already exists is a huge seller

Actually, thinking about this a bit more, it's not unreasonable to think someone would want to run update automatically as they're editing the Cargo.toml. This further suggests that watch should be a command since otherwise it would be confusing to add to cargo update as a flag. Though I don't know the downsides of making it command.

@epage
Copy link
Contributor

epage commented Jan 8, 2023

I think this is the key question then: if watch were to become a thing, would it be a flag or a command (like cargo-watch)? If it would be a flag, then this PR has to wait for #9339 to be resolved. If it'd be added as a command, then I think -w can coexist just fine. I took a look at the other linked --watch tools, and they all appear to use booleans which suggests watch as a command is fine.

imo I would see watch support being a flag on select commands, like cargo check.

Actually, thinking about this a bit more, it's not unreasonable to think someone would want to run update automatically as they're editing the Cargo.toml. This further suggests that watch should be a command since otherwise it would be confusing to add to cargo update as a flag. Though I don't know the downsides of making it command.

Generally, watch mode is used during long-running tasks, like as part of an edit-build-test cycle. I can't quite picture a long running Cargo.toml workflow for a cargo update --watch flag.

@epage
Copy link
Contributor

epage commented Jan 8, 2023

For anyone curious -w was added at the same time as --workspace to `cargo update as part of #8725.

EDIT: Looked it up, hoping there was a note on why it wasn't applied to other commands but there isn't.

@SUPERCILEX
Copy link
Author

Generally, watch mode is used during long-running tasks, like as part of an edit-build-test cycle. I can't quite picture a long running Cargo.toml workflow for a cargo update --watch flag.

Fair enough, so I think this is blocked on #9339.

@SUPERCILEX
Copy link
Author

Filed #11554

@SUPERCILEX SUPERCILEX closed this Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants