Support --workspace flag in cargo-fmt

[epage]

In Rust 1.39, the --all flag was renamed to --workspace but cargo-fmt still does not provide --workspace.

This is a frequent papercut for me as I write CI pipelines where I try to use the new flag everywhere but forget that cargo-fmt doesn't support it.

[epage]

There a reason that cargo isn't being consumed a crate to ensure consistency?

I know in some of my tools, I've found had problems with

• build times
• Being locked into the Cargo.toml parser that I built against, leading to warnings when new features are used
  • I'm assuming this isn't a concern since this ships with cargo.

Part of my curiosity is for better understanding the needs people have for cargos API.

[calebcartwright]

Thanks for reaching out, but I'm going to close this as it has been discussed a few times and is not a matter of a simple rename (see #3911, #4441 and other workspace related issues).

I appreciate the perspective and the desire for consistency, but this is a case where we'd introduce breaking behavioral changes if we were to rename flags to match what cargo does.

There a reason that cargo isn't being consumed a crate to ensure consistency?

Because it's way too powerful and heavy for what we need. cargo fmt is a glorified rustfmt command builder, and all it really needs is the edition and entry point files for the package targets which is readily available via cargo metadata

[epage]

Supporting --workspace isn't necessarily a breaking change; it can be added in addition to --all which can warn people to use --workspace instead, like cargo does.

I saw mention in #3911 that the behavior is different. Unfortunately, cargo fmts documentation doesn't make this clear and looks like it'd behave like cargo. Digging into the source, it looks like the difference is how the root workspace is handled, cargo only operating on all if the root is virtual (no package), while cargo fmt will operate on all always when in the root. Is that correct? I wonder if people would consider that distinction a defining feature that needs to remain compatible or a bug that to be fixed. Probably both :)

[calebcartwright]

As stated previously, I get the desire for consistency and alignment with cargo and other subcommands, and if I had the luxury of starting with a completely clean slate that would definitely be the north star. However, we have to work within the constraints and realities of the present, for better or worse.

While the changes, and/or additions, you are proposing to something in the rustfmt ecosystem would indeed increase alignment with something outside that ecosystem (cargo), they'd simultaneously muddy the waters/create confusion and contradiction within that ecosystem. That's not good either, and in my opinion has the potential to be marginally worse than where we are today.

Supporting --workspace isn't necessarily a breaking change it can be added in addition to --all which can warn people to use --workspace instead, like cargo does.

No, we definitely wouldn't want to do that. You're still equating cargo fmt's --all with meaning --workspace but that's incorrect. cargo fmt's --all behavior has existed for a long time, and exists for a reason. It may not be relevant or useful for every single user, but there's definitely users that utilize the current behavior of --all.

• Renaming the option from --all to --workspace would be specious because --all is not constrained to a workspace.
• Renaming the option from --all to --workspace would introduce confusion because in some scenarios/locations in a directory hierarchy cargo fmt will format the entire workspace even without a --workspace flag
• Leaving --all as is (or renaming it to something more glaringly obvious to avoid cognitive overlap with prior cargo options) and adding a new --workspace option that is constrained to the workspace is slightly more viable in that it avoids the first issue, and I've proposed doing this very type of thing in the past

Unfortunately, cargo fmts documentation doesn't make this clear and looks like it'd behave like cargo

Quoting myself a bit again but regardless of what is/isn't documented we're going to be dealing with folks that make the same understandable, but incorrect, assumption. Though not yet released, we have already made some adjustments to the help text; I'm just skeptical it'll move the needle much.

Digging into the source, it looks like the difference is how the root workspace is handled, cargo only operating on all if the root is virtual (no package), while cargo fmt will operate on all always when in the root. Is that correct?

Feel like this has now been answered with the preceding text/links to past discussion on the topic (particularly #4247 (comment) and #3911)

I wonder if people would consider that distinction a defining feature that needs to remain compatible or a bug that to be fixed. Probably both :)

As I'm sure you already know, stability and consistency are very real factors in the Rust ecosystem 😄 The combination of Rust's general posture on stability and the stability guarantee of rustfmt provide very little wiggle room to make breaking changes of any kind, which is why to this very day rustfmt's default emits some formatting that's very silly here in 2021 but has to continue to do so because once upon a time it was required (e.g. an extra space in nested tuple accesses)

This absolutely applies to command line behavior an options as well. I understand that folks don't like the papercut of remembering to use --all with cargo fmt (assuming they actually need it, even though 90% of the time they don't), but we can't just chalk current, utilized behavior up as a "bug" and break that behvaior.

[epage]

Leaving --all as is (or renaming it to something more glaringly obvious to avoid cognitive overlap with prior cargo options) and adding a new --workspace option that is constrained to the workspace is slightly more viable in that it avoids the first issue, and I've proposed doing this very type of thing in the past

This is basically what I was intending with my comment but I'm not seeing a reason here or in the link for why this is being rejected.

Feel like this has now been answered with the preceding text/links to past discussion on the topic (particularly #4247 (comment) and #3911)

To be clear, I was trying to confirm my understanding since parts remained unclear. With your reply, I've picked up an additional piece (implicit workspaces), since I hadn't found that comment yet.

As I'm sure you already know, stability and consistency are very real factors in the Rust ecosystem smile The combination of Rust's general posture on stability and the stability guarantee of rustfmt provide very little wiggle room to make breaking changes of any kind, which is why to this very day rustfmt's default emits some formatting that's very silly here in 2021 but has to continue to do so because once upon a time it was required (e.g. an extra space in nested tuple accesses)

To be clear, before the "implicit workspace" explanation, this was sounding more like undefined behavior being treated as requiring compatibility which can be a murkier for what can be changed, depending on several factors. Understanding there is an intentional use case is different.

Supporting a cargo aligned --workspace though would still be a big help. For anyone "not in the know" of cargo fmt (which I imagine is most Rust developers), they will use cargo fmt --all without reading the docs, expecting it to behave like anything else in cargo. This can then start re-formatting code that they wouldn't expect to be formatted because this concept of "implicit workspace" doesn't exist anywhere else. If I then go to that other crate and commit, assuming I'm the only one making changes, I'll have to revert my change and rewrite it from scratch if the re-format changes were noisy enough that I can't find what I modified.

[calebcartwright]

No one's saying the current state is perfect. What I've said is that there's no obvious improvements nor perfect state. It's a classic case of competing tradeoffs that's been exacerbated by changes to factors outside our control. Any change in this area that improves one side is going to cause degradation elsewhere.

That doesn't mean we'll never make any changes, but it's a significantly more involved and complicated topic that we're not going to track/resolve as a simple issue like this one and those that preceded it.

This is basically what I was intending with my comment but I'm not seeing a reason here or in the link for why this is being rejected.

I never said my proposal was rejected, in fact I said it was likely the only viable option, so I'm not really sure what this is in reference to (if it was regarding the state of the issue then I think my response above should address that)

Digging into the source, it looks like the difference is how the root workspace is handled, cargo only operating on all if the root is virtual (no package), while cargo fmt will operate on all always when in the root. Is that correct?

Feel like this has now been answered with the preceding text/links to past discussion on the topic (particularly #4247 (comment) and #3911)

To be clear, I was trying to confirm my understanding since parts remained unclear. With your reply, I've picked up an additional piece (implicit workspaces), since I hadn't found that comment yet.

Understood, was just noting that I felt in responding to/answering other questions in the same comment that this question happened to have gotten covered as well.

To be clear, before the "implicit workspace" explanation, this was sounding more like undefined behavior being treated as requiring compatibility which can be a murkier for what can be changed, depending on several factors. Understanding there is an intentional use case is different.

Supporting a cargo aligned --workspace though would still be a big help. For anyone "not in the know" of cargo fmt (which I imagine is most Rust developers), they will use cargo fmt --all without reading the docs, expecting it to behave like anything else in cargo. This can then start re-formatting code that they wouldn't expect to be formatted because this concept of "implicit workspace" doesn't exist anywhere else. If I then go to that other crate and commit, assuming I'm the only one making changes, I'll have to revert my change and rewrite it from scratch if the re-format changes were noisy enough that I can't find what I modified.

To be honest, I see rapidly diminishing returns in continuing to spend cycles going back and forth this. I feel like everything worth saying has already been said a few times in a few different ways, and I've got too big a backlog to continue debating something I think we actually largely agree on 😄

It's a topic that's been covered at length, and as I've already noted there's downsides/tradeoffs to the current behavior. Most folks seem to forget or overlook that it's pretty common for users to structure their projects with multiple crates that have non-strictly hierarchical path based dependencies, and do so without explicitly defining a workspace.

It is (or at least was) common for the "implicit" phraseology to be used with various tools including and beyond rustfmt, and there is some etymology that ties back to cargo itself. However, I don't think it really matters what exactly it's called, nor is it worthwhile to get sidetracked by one label or another.

cargo fmt --all was always intended to be a "format my entire project", which has to account for the fact that there's some different definitions of "project" which includes this structure. This type of structure is a very real, and common scenario and those users expect cargo-fmt to continue to have a way to format their entire "project" regardless of which directory they happen to be in that moment.

[epage]

I think one take away from this is that this is a recurring question and that the information about it is spread far part. In being introduced to it, it took several back and forths, reading of the code, and links out to threads and specific comments in threads. It might be worth finding a clearer way to communicate this so to keep things more positive on boths sides of the conversation and reducing load on boths sides.

[calebcartwright]

Yeah I hear ya, good documentation can be of great utility. I'm all for getting something like that together, but it still boils down to a matter of priorities and is unlikely to happen in the near future (unless someone else is feeling particularly motivated). This has only come up maybe four or five times in the past two years, so unfortunately it probably doesn't even crack the list of the top ten recurring things that would benefit from some docs or a dedicated post.