Could rustfmt format Cargo.toml ?

[Stargateur]

I actually use a very very small tool toml-fmt, I currently use it like:

toml-fmt < Cargo.toml > Cargo.toml.new && mv Cargo.toml.new Cargo.toml

That not very practical and I must remember to use it. Could rustfmt handle it too ? I think Cargo.toml is also a part of rust file so maybe it's ok to add it.

[calebcartwright]

That's an interesting idea @Stargateur

Formatting toml files is outside rustfmt's scope (at least currently), though I can see how having the optional/opt-in ability to format toml files would be beneficial. prettier also supports formatting json and yaml files in addition to JavaScript and TypeScript, so it's not completely unheard of.

How would you envision the behavior when running cargo fmt in a multi-crate workspace? Would the expectation be that cargo fmt would support formatting all the detected Cargo.toml files in the workspace?

[Stargateur]

How would you envision the behavior when running cargo fmt in a multi-crate workspace? Would the expectation be that cargo fmt would support formatting all the detected Cargo.toml files in the workspace?

I guess the same that the actual behaviour, if the actual behaviour is to format all detected rust file in a workspace than yes, if no than no.

[calebcartwright]

It looks like the Style Guide actually already includes a spec for Cargo.toml, so IMO formatting Cargo.toml files accordingly is something that rustfmt should indeed support.

There's a few things to think through (default behavior, cli args/opts, configurable via rustfmt.toml, etc.) but it should be a fun project if anyone is interested in working on it!

[gilescope]

You could start small and gradually crank it up. Alphabetical ordering of blocks of dependencies (with newlines demarcating different blocks of dependencies) would be good. Adding spaces around dependencies. I.e. foo = "1.2" rather than foo="1.2".

Those two would probably be 80% of the file sorted out nicely - the dependencies are going to have the most change so they are the areas that benefit the most from consistency.

[srinikhil-07]

@calebcartwright Can I try this one following yours and @gilescope directions?

[calebcartwright]

@Nikhil0487 - absolutely! I suspect this may end up being a sizeable effort, but can likely be developed iteratively. I still don't have a feel for what the end state would look like from an experience/usability perspective, nor have I really had the bandwidth to think on it.

As such I'd advise starting with the formatting logic (something that takes in an unformatted string and returns a formatted one) and we'll figure out the rest later on. Feel free to put this in a new mod under src/formatting

[thomaseizinger]

For people watching this issue, I've developed a dprint plugin that does some formatting of Cargo.toml files: https://github.com/thomaseizinger/dprint-plugin-cargo-toml

dprint also has a rustfmt plugin which means you can have a single tool for all your formatting needs :)

[gilescope]

I notice that vscode is formatting Cargo.toml if I do format on save. But I'm not sure how - is that rust-analyzer or the better-toml extension or something else?

[mati865]

@gilescope it's "Even Better TOML" extension who does that. Link to the GitHub project: https://github.com/tamasfe/taplo

[gilescope]

Great. How do we sneak that into rustfmt? ^_^

On Fri, 13 Aug 2021 at 01:47, Mateusz Mikuła @.***> wrote:

@gilescope https://github.com/gilescope it's "Even Better TOML" extension who does that. Link to the GitHub project: https://github.com/tamasfe/taplo

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rust-lang/rustfmt/issues/4091#issuecomment-898046047, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGEJCBI2ISDDTV5DK3GHS3T4RMQBANCNFSM4LQYIZGA .

[gilescope]

Have just been playing with that. I think all my dreams have come true with respect to tidy Cargo.toml files.

I could not want for more. I just put a .taplo.toml file in the root dir and away I go: https://taplo.tamasfe.dev/configuration/#formatting-options https://taplo.tamasfe.dev/

(align_entries = true in particular is most excellent)

[thomaseizinger]

The functionality of my dprint plugin has been integrated into the official dprint toml plugin.

The plugin is based on taplo and automatically formats your Cargo.toml according to the conventions.

Have just been playing with that. I think all my dreams have come true with respect to tidy Cargo.toml files.

If you are happy with placing a .taplo.toml file in your root dir, you will likely be pleased with dprint:

One configuration file for formatting TOML, Rust, TypeScript, JSON and many more to come :) Kudos to @dsherret for making it!

[calebcartwright]

I absolutely appreciate how beneficial it can be to share alternatives on a feature request like this, and thank everyone for posting. However, I do think alternatives have been more than sufficiently covered at this point and I don't want this thread to continue digressing on discussions drilling into those alternatives which has drifted markedly off topic.

This issue is first and foremost a feature request for rustfmt that needs to be implemented in a rustfmt way, including adherence to the Style Guide prescriptions which dictate rustfmt's formatting, as well as honoring reasonably expected rustfmt config behavior, e.g. hard_tabs and tab_spaces. As such I'd ask that any additional discussion about alternatives be taken to a more appropriate channel, like the user forums, discord/zulip, etc. so that this thread can remain focused on the rustfmt implementation.

As to the actual rustfmt implementation, the steps for incorporation remain the same as outlined earlier in this thread. I do not think deferring the actual formatting logic to an external library is likely to be a viable candidate for the reasons that I've outlined above. We've got a formatting spec we'd have to follow, and even if one or more versions of said external lib happen to emit the same formatting, there's no guarantee they'd continue to do so (and we couldn't use a pinned version either due to risk of update blockage)

[xxchan]

I implemented the formatting logic https://github.com/rust-lang/rustfmt/pull/5240, review welcomed!

[xxchan]

For those who are watching this issue, the current status is that: The implementation in #5240 basically looks good. The next step is that we need to add more tests, and investigate how to integrate the logic into rustfmt.

But I'm a little bit busy these days, so I will try but cannot ensure to find some time recently to continue working on #5240. I'm OK if someone wants to join force and drive it to complete! 😊

[robertbastian]

Shouldn't this be part of Cargo, not rustfmt?

[clarfonthey]

Rustfmt can run on individual files, so, being able to format a Cargo.toml file seems reasonable imho.

[xxchan]

The question is already discussed:

It looks like the Style Guide actually already includes a spec for Cargo.toml, so IMO formatting Cargo.toml files accordingly is something that rustfmt should indeed support.

[clarfonthey]

I believe that the point here was whether the formatting should be part of the code for cargo fmt rather than part of rustfmt, since it'll be pretty separate from the usual rust formatting machinery anyway. I don't think it's a big deal to include it in rustfmt and allow formatting without knowledge of a crate itself, since that code would live in cargo itself.

[ytmimi]

@clarfonthey cargo fmt is just a thin wrapper around rustfmt so it's best if the feature is implemented by rustfmt.

rustfmt's main goal is to be an implementation of the Style Guide, and since there's a prescription in the Style Guide for Cargo.toml formatting I also agree with the comment made in https://github.com/rust-lang/rustfmt/issues/4091#issuecomment-612333467 and @xxchan.

[TimonPost]

In case some people are interested, I made a toml-formatting library that I have been carefully using on a large set of TOML files and also composited a best-practice-style guide which I maintain. https://github.com/TimonPost/cargo-toml-format. It works relatively decent if you execute the steps in a certain order. See tests. Just reposting here

[tcoratger]

Is there a plan here to integrate .toml file formatting directly into the rustfmt library or is it out of the scope?

[xxchan]

It’s out of scope.

[ytmimi]

@tcoratger if you mean all .toml files then I agree with @xxchan that it's out of scope, but if you're specifically referring to Cargo.toml files then the answer is yes, it's in scope. https://github.com/rust-lang/rustfmt/issues/4091#issuecomment-612333467

[tcoratger]

@ytmimi Yes that was my question if the Cargo.toml files were in scope. I saw the comment mentioned (https://github.com/rust-lang/rustfmt/issues/4091#issuecomment-612333467) but since it is 3 years old I was wondering if it was still relevant/ongoing. On most Rust repos I see this is a real pain point, especially since the other parts of the formatting are very well handled by rustfmt, but this one is really the missing piece.

As I understand this is a project to do, is there a starting point? Work already done on it? Discussions on how the community wants to proceed?

Maybe using some existing external library as example could be the way to go, what do you think? Like the one mentioned here for example https://github.com/rust-lang/rustfmt/issues/4091#issuecomment-1472645913

[xxchan]

5240

[calebcartwright]

@epage responding to https://github.com/rust-lang/rustfmt/pull/5240#issuecomment-1856120291 here as I'd rather discuss if/when/etc. a feature should be implemented in the issue as opposed to a PR.

I'm really trying to avoid getting into broader discussion as well as the specifics of what/why/etc. because I think discussion on PRs should be largely centered review of the "how", the specifics of proposed implementation changes.

To be clear, I was trying to highlight what could be dropped to make this easier to move forward, from a cargo team member perspective, not to get into the broader discussion.

Fair, though to clarify from my end, I was commenting in general, not to you in particular as this is a topic that tends to solicit a lot of commentary that can get really noisy really quickly, and I wanted to try to keep @xxchan's PR as free from that as possible so that it's easier for it to stay focused on review/implementation details

Ultimately, this is not something I'm comfortable moving ahead with at this time, neither in whole nor in part,

Please reconsider this. Focusing on a more incremental approach should make your life much easier and gets people benefits now, rather than waiting for some future date. We can pick less churn-heavy areas to move forward.

Suffice to say I disagree about the impact of this on my life, but this feels orthogonal so I'll leave it there.

Ultimately the ask here would be tantamount to implementing a non-trivial new feature in rustc without any form of RFC on a topic for which there's known areas of contention. I really don't think that's common practice elsewhere in the Project, and I don't see why folks want to try to persuade us to operate differently.

All the reasoning I'm hearing for why it's better to do something now, and for the first time, centers around (a) not having toml formatting being bad and (b) having some/any kind of partial formatting being improvement.

My response to that would be to reiterate that there's existing options available today that deliver just that, several of which are described and discussed above.

However, I'm not going to reconsider moving forward under the current set of contexts and constraints. When those change, then we can as well. I'd personally prefer spending available time and energy trying to resolve those items so that we can proceed forward on good footing, as opposed to debating

including moving some of the subcommands (like cargo-fmt) into Cargo and large unanswered questions around things like sorting.

I thought you said "no" to moving it?

I'd heard indirectly that it's something the Cargo team wanted to take ownership of, then I saw your ping and provided my input, but I never received a response nor did I hear anything else from anyone. In my mind this was a pending, unresolved matter

As for sorting, if we leave that out, then it isn't a problem, right?

I do not know the specifics of the Cargo team's objections to the existing rules, nor what sort of rules you all may want. Those specifics haven't been communicated to the Rustfmt team nor the Style team as far as I'm aware.

That's why I personally think our time would be better spent trying to hammer out those details, and focus on trying to get everything solidified before the 2024 style edition has to ship.

[epage]

Ultimately the ask here would be tantamount to implementing a non-trivial new feature in rustc without any form of RFC on a topic for which there's known areas of contention. I really don't think that's common practice elsewhere in the Project, and I don't see why folks want to try to persuade us to operate differently.

Teams have actually been moving in this direction. The idea is there are times where its worth trying things out before doing the RFC to see what you didn't know and to find out if things worked as well as you predicted. This also makes the RFC easier because you can point to something concrete for feedback.

Examples from cargo

• cargo's cache GC started with a PR. Unsure if this will end with an RFC or MCP
• MSRV resolver started with PRs that were merged and is currently an RFC.
• We've asked people to make an MCP to experiment with code coverage, rather than focusing on the existing RFC
• cargo script had an eRFC to approve moving forward with an implementation and is now in the RFC stage with some implementation waiting on approval

libs-api has shifted focus away from RFCs to ACPs

It was either compiler or lang that has people start with an MCP for unstable features which can then lead to an RFC.

One of the biggest problems I've seen in the Rust community is a focus on big, heroic improvements rather than breaking things down into smaller, achievable improvements (hence why I'm pushing back here). I see the shift to implement-first-RFC-later a a big improvement in that direction.

I'd heard indirectly that it's something the Cargo team wanted to take ownership of, then I saw your ping and provided my input, but I never received a response nor did I hear anything else from anyone. In my mind this was a pending, unresolved matter

We were looking for whether and where it would make sense. Without you being open to the idea, I assumed it was dead in the water.

I do not know the specifics of the Cargo team's objections to the existing rules, nor what sort of rules you all may want. Those specifics haven't been communicated to the Rustfmt team nor the Style team as far as I'm aware.

See https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/.60Cargo.2Etoml.60.20style.20guide

Unsure why that hadn't had made its way back.

[epage]

btw if you want help in reviewing manifest-related formatting, I would be willing to do it. Involving people more specialized in Rust vs Cargo was part of the inspiration for the "should manifest formatting be owned by Cargo" though I do know there are other concerns related to formatting that apply to both.

[calebcartwright]

Formatting the contents of a Cargo.toml file is significantly less complex than formatting the entire set of Rust syntax.

The real question I'd argue is what that toml file should look like, and how the feature should be surfaced up within the rustfmt ecosystem.

If we aren't even approaching consensus on things like what the order of tables should be, what the order of keys should be, how some of the key values should be formatted, etc. then I'm not sure I see how much formatting value is really left to be realized other than basic whitespace modifications, and again I'd point to the bevy of existing tooling that can assist on that front.

I reject the notion that I'm insisting on some "heroic" big bang approach, and I'd have hoped that's self-evident in my commentary and the guidance we've provided in trying to advance this feature (however slowly) (e.g. https://github.com/rust-lang/rustfmt/issues/4091#issuecomment-730716157).

Fundamentally, I think this entire feature is exceedingly small but the basics of the formatting along the axes I mentioned constitute a "Minimum Viable Product".

Until we have that, I'm not going to move forward with this; pressure/repeated requests aren't going to change that.

Instead, I'd like to work together on establishing that direction, on determining/confirming that the intent is to have the Style Guide to maintain codified prescriptions for Cargo.toml files and if so what those prescriptions are.

We don't have to have everything fully finalized to potentially move forward, but we need a lot more clarity than what currently exists.

[NickLarsenNZ]

The real question I'd argue is what that toml file should look like, and how the feature should be surfaced up within the rustfmt ecosystem.

If we aren't even approaching consensus on things like what the order of tables should be, what the order of keys should be, how some of the key values should be formatted, etc. then I'm not sure I see how much formatting value is really left to be realized other than basic whitespace modifications, and again I'd point to the bevy of existing tooling that can assist on that front.

@calebcartwright, I guess I'm confused by this part, as you originally posted the link to the style guide (new link to the Cargo.toml conventions) which makes a lot of that clear already. Or are you suggesting there be a new round of opinions on style to replace that style guide?

[calebcartwright]

This was a topic we discussed at length in yesterday's T-Style meeting so I wanted to provide an update. Additionally, I recognize that from the outside this all seems really simple and that the Nike execution strategy (i.e. "just do it") seems perfectly viable. However, there's a lot more at play than meets the eye.

My phrasing of the sequence of steps that need to occur:

1. The Project needs to determine which team will hold Authority/ownership of the Cargo.toml style (see https://github.com/rust-lang/cargo/issues/13313)
2. Cargo.toml style rules need to be determined by whichever Project team holds that authority (see https://github.com/rust-lang/style-team/issues/188)
3. Presuming the rustfmt ecosystem of tooling will provide the automated formatting for those rules, an implementation needs to be completed and released (see this very issue, and associated PRs)

As I've stated previously, not all of these items need to be fully 100% resolved before we could proceed with some form of experimental support in rustfmt. However, more clarity is needed, particularly on those first two items before it makes sense to take additional steps on the rustfmt side

Fortunately, it now seems (or at least appears to me) that the Cargo team's concerns with the original rules were not as broad as I'd previously understood, but are perhaps more about a depth of concern with a smaller number of elements which should make it possible to make progress more quickly.

[NickLarsenNZ]

Thanks for the clarity @calebcartwright .

As an aside, should the current formatting doc mention that it is in flux in the meantime?

[calebcartwright]

Thanks for the clarity @calebcartwright .

As an aside, should the current formatting doc mention that it is in flux in the meantime?

Potentially, I'll mention it on https://github.com/rust-lang/rust/pull/120072

[clarfonthey]

Going to reshare this comment here since the PR got closed:

My particular concern here, speaking of someone who understands what it's like to contribute changes to large projects on my own time, is that it's incredibly frustrating to leave PRs in limbo like this. I'm fine with stripping out changes that could be changed/cause churn, I'm fine with deciding that you just don't want this change before design is done. What I'm not okay with is a change being open for almost two years in limbo.

It takes a lot of effort to rebase and review a change like this as it gets scrutinised, and it's especially frustrating to continue to have a change left open for months because it's in this weird limbo of not prioritised but not undesired.

The reason why I proposed merging under an unstable option is because it would mean this PR is closed and then additional work can happen in another PR.

Like, I understand the situation this puts you in as a maintainer, but I'm speaking from experience; either:

1. Merge it as-is
2. Provide criteria for what form it would need to be, to be merged
3. Close it

None of this "we don't know what the criteria is, don't want to work on it right now, but like some of what this is right now." That just makes everyone feel worse about it, both maintainers and contributors. IMHO, either decide if there's some portion of this you'd be comfortable merging now (and request those specific changes, likely involving removing pieces) or close this and tell people where they can follow the discussion in the meantime.

I'm sure that the person who actually locked the PR wasn't thinking too hard about it, but this is honestly just insult to injury for the situation. Instead of actually performing one of these three things, you decided to prevent all further discussion on the matter, keeping it more in limbo, effectively making it both impossible to merge and impossible to close.

Please reconsider.

[calebcartwright]

@clarfonthey i appreciate you sharing your perspectives, but we don't need the same comment copied across multiple places.

There's not actually any limbo here. Things are blocked, we've stated it explicitly, with links to relevant upstream information (e.g. https://github.com/rust-lang/rustfmt/issues/4091#issuecomment-1898987293)

I understand you're directing us to take one of the three options you've proposed, but we're the ones that make that decision and we've already stated we're not going to do those.

It's blocked, we've provided information explaining why it's blocked, and we're leaving the PR open.

[clarfonthey]

I'm not just copying the comment here, though; I'm continuing the discussion of it since I can no longer continue that discussion on the PR.

There's not actually any limbo here. Things are blocked

I think we have differing opinions of what these terms mean. The PR is in bureaucratic limbo: it's not clear what portion is owned by the style team and which portion is owned by the cargo team, and so, no exact decision can be made about what portion is okay to include, and what portion isn't. There is no clear timeline on when it will be unblocked, and rustfmt will continue to undergo massive changes as the language adds more features, etc. The two issues you linked do show the path forward: the discussion is on Cargo's side and there are explicit forums to track this, and if you want updates, you should ask there.

Regardless, the answer to this is simple: close the PR until the point where it's clear what should be done. The code will not vanish once the PR is closed, and it can even be re-opened if it turns out that the code hasn't gone stale by the time the blockers are fixed. Instead, it was kept open and the discussion was blocked, which demonstrates a particular unwillingness to deal with the problem that I'd like to interrogate.

Closing the PR would have had the same effect of quelling the discussion, in my opinion, since it points out that the PR is not a place for talking about how much you want the feature. Instead, keeping it open and locking discussion says "I'm tired of you pointing this out to me when I don't want to deal with it" which… again, why? There's a clear solution here.

I understand that I am effectively speaking for the PR author and they could have just decided to close it on their behalf instead. The reason why I want a definitive decision made on the PR, not the stuff necessary to unblock it, is because it represents an incredibly unhealthy relationship with contributors to rustfmt itself. Yes, there are lots of problems with the "hit-and-run" model of contributions where someone just dumps a PR and expects it to get merged, although hopefully, there's a bit more collaboration involved between providing the initial PR and merging it. Right now, the collaboration is effectively "we like the work you've done on this for us, but want you to pause for a couple years while we work out our bureaucratic issues, after which point you are expected to fix the change for us" which is kind of toxic.

I doubt that anyone explicitly feels this way about changes, and is extremely understanding if PR authors don't want to rebase their changes after years, but the point still stands that this is the opinion that's presented by the actions being performed. And I think that even the explicit problem mentioned by the most recent action, locking the topic, would be much more easily resolved if everyone had a better picture of what's actually needed to make these changes.

By keeping the PR open, you're inviting discussion there: that's what open PRs are for! Honestly, linking the comment you just linked in the PR probably does the most help letting people know what the path to unblocking this is, and I think that honestly, the content of the comment you linked here should be added to the beginning of this issue so people have the easiest access to it.

Sorry that I'm mostly just rambling here, but I'm just upset at the choices made here and would like to interrogate them because I think that doing that would be worthwhile. I know that ultimately, your hands are about as tied as mine here, so, it's easy for this to just feel like nitpicking.

[calebcartwright]

Instead, it was kept open and the discussion was blocked, which demonstrates a particular unwillingness to deal with the problem that I'd like to interrogate.

I was multitasking when I locked it and didn't post the explanatory comment right away. That was a mistake but it wasn't malicious.

The discussion on the PR had long been too sprawling and off topic, and I'd repeatedly tried to redirect it back over here (e.g. https://github.com/rust-lang/rustfmt/pull/5240#issuecomment-1856122730) without avail. I'll also note the obvious that the discussion remains open here, intentionally.

no exact decision can be made about what portion is okay to include, and what portion isn't. The reason why I want a definitive decision made on the PR,

This is where I strongly, emphatically disagree. We've made a decision, I've said "no, we're blocked. we're not going to move forward at this time" repeatedly on this issue and on the PR

https://github.com/rust-lang/rustfmt/issues/4091#issuecomment-1856447234 https://github.com/rust-lang/rustfmt/issues/4091#issuecomment-1898987293 https://github.com/rust-lang/rustfmt/pull/5240#issuecomment-1856101922

I strongly suspect some people commenting haven't read that decision. Others have read and don't like that decision, but that's typical and not the same as us not having conveyed a decision.

To date people have been solely focused on trying to pressure us to change that decision. I've also said this a few times, but that's not a constructive path and it's not going to deliver any outcomes other than negative emotions on both sides.

This particular feature is something we'd like to support (assuming it remains within the style/rustfmt scope), but it's blocked on things owned by other teams. We're not actively working on trying to get those blockers removed ourselves, and we've decided and communicated we're not going to proceed until those are resolved.

I do not know how to state it any more clearly than that, and if anyone is going to gauge current state based off a PR being open or closed while not reading such definitive statements and/or labels and as a result has an inaccurate impression, well then I can accept that.

I feel like the rest of your feedback has been some general criticism of me and our approach here which I feel is starting to border on a "lecture".

I think that feedback is partly a difference of opinion over a decision to close vs. label as blocked. I've read and understand your perspective on the various implications and inferences that can be drawn from a PR being left open, but I'll just add that there are also implications and inferences associated with a PR being closed. Everyone is different, I've had previous experiences of closing in somewhat similar situations and it having the effect of deflating the author, and/or users presuming the associated feature was permanently rejected.

I think the feedback is also partly based on a number of presumptions I'd dispute (no one has asked for a rebase since I stated work was blocked, we'd absolutely never make a after which point you are expected to fix the change for us demand, etc.)

Furthermore old PRs being left open (with or without being explicitly noted as blocked) is pretty common in my experience, and while I agree that's suboptimal, I genuinely don't understand why there's such intensity directed at us for this pretty common situation

https://github.com/rust-lang/cargo/pulls?q=is%3Apr+is%3Aopen+sort%3Acreated-asc https://github.com/rust-lang/rust/pulls?q=is%3Apr+is%3Aopen+sort%3Acreated-asc https://github.com/rust-lang/rustup/pulls?q=is%3Aopen+is%3Apr+sort%3Acreated-asc

Certainly feel free to respond here, but if this is indeed a conversation you want to continue at length then I'm also around on Zulip and suspect that might be a better forum (at least for some of the items you indicated wanting to interrogate)

[epage]

but it's blocked on things owned by other teams.

If that is the Cargo team, I don't think we know that. The only thing I know the Cargo team has formally been approached on is whether decision making is wholly owned by the style team or if the Cargo team should be involved, see https://github.com/rust-lang/cargo/issues/13313. I do not see that as blocking for moving forward.

Furthermore old PRs being left open (with or without being explicitly noted as blocked) is pretty common in my experience, and while I agree that's suboptimal, I genuinely don't understand why there's such intensity directed at us for this pretty common situation

Cargo isn't a good example. I'm actively working to close out old work (PRs, RFCs) as part of an effort to make next steps for work clearer. This is something I've cared about more generally but was reminded of the importance of it when some of the Edition 2024 work was held up for months because the author and the relevant team each thought they were blocked on the other.

[calebcartwright]

Cargo isn't a good example. I'm actively working to close out old work (PRs, RFCs) as part of an effort to make next steps for work clearer. This is something I've cared about more generally but was reminded of the importance of it when some of the Edition 2024 work was held up for months because the author and the relevant team each thought they were blocked on the other.

My point is that old PRs are not a phenomenon unique to rustfmt, and that a lot of the critique is generally applicable to open source projects in general. I'm not saying it's a good thing, and at some point we need to go through a similar exercise.

I only listed a couple quick examples from within the project, but I'm confident there's a large number of open source projects with similar characteristics

[clarfonthey]

Instead, it was kept open and the discussion was blocked, which demonstrates a particular unwillingness to deal with the problem that I'd like to interrogate.

I was multitasking when I locked it and didn't post the explanatory comment right away. That was a mistake but it wasn't malicious.

Just to be clear, I'm not specifically dinging you for that bit; it's very easy to forget to do all the steps and I don't fault you for that. My main issue was the decision to lock the PR and keep it open, not that you didn't do so without explaining your justification for that in the first place.

The discussion on the PR had long been too sprawling and off topic, and I'd repeatedly tried to redirect it back over here (e.g. #5240 (comment)) without avail. I'll also note the obvious that the discussion remains open here, intentionally.

I strongly suspect some people commenting haven't read that decision. Others have read and don't like that decision, but that's typical and not the same as us not having conveyed a decision.

I definitely sympathise with the frustration that comes from a lack of reading, and while I personally try to go through everything, explicitly unhide the comments GitHub chooses to hide, etc., I think that it's very difficult to blame people for this, given the circumstances. The GitHub UI makes it incredibly difficult to follow long discussions like this and the simplest way to show that a discussion should not be somewhere is to just close the PR, as I said. Closing it does not delete the code or indicate that it cannot be merged later, but it does tell people that you shouldn't be discussing on that PR. Closing it with a comment that points out that discussion should continue here doubly pushes that message, since the first comment people will likely see is the last one if they try and comment there.

To date people have been solely focused on trying to pressure us to change that decision. I've also said this a few times, but that's not a constructive path and it's not going to deliver any outcomes other than negative emotions on both sides.

I should add, I don't think that you're making the wrong decision here in terms of merging the PR or what specifically is blocking it. My entire issue is with leaving the PR open, which encourages people to continue discussing it specifically. It sends the same message I thought was inappropriate, which is "we like this code, but would prefer to leave it hanging than do anything about it." Rustfmt is an active project and while this code is relatively separated from everything else and thus less prone to becoming stale, it still will become stale over time, and I think that it is just overall more helpful to close the PR and re-evaluate once the blockers are removed than to keep it hanging.

I do not know how to state it any more clearly than that, and if anyone is going to gauge current state based off a PR being open or closed while not reading such definitive statements and/or labels and as a result has an inaccurate impression, well then I can accept that.

As I said, I agree that people shouldn't be adding random "+1" comments to PRs without reading the entire thing, but GitHub's UI specifically encourages pushing away comments in the PR in favour of the very first and very last ones, neither of which properly demonstrates the blockers to the PR. Closing the PR is a guaranteed way to make sure your message is visible, at which point you can be certain that everyone making a comment asking when it can be merged just isn't reading well enough.

I feel like the rest of your feedback has been some general criticism of me and our approach here which I feel is starting to border on a "lecture".

To be clear, while you made the latest decision, I don't think that this is all on you. I think it's a decision of the team and I'm explaining why I think that decision should be reconsidered. My impression is that more than just you were responsible for this general decision, and more people than you can read these comments and respond to them. If I'm wrong about that, I'm sorry that I'm singling you out, but my impression is that I wasn't.

Furthermore old PRs being left open (with or without being explicitly noted as blocked) is pretty common in my experience, and while I agree that's suboptimal, I genuinely don't understand why there's such intensity directed at us for this pretty common situation

It's common, but I don't think it should be. PRs that are blocked should only be left open if they have value to being left open: either the change can continue to be worked on, or it's likely that the blocker will be removed soon. Neither of these applies here, since both work on the change and merging the change depend on decisions which are yet to be made.

---

Also just adding another thing as I'm rereading my own comments: overall, I think that leaving the PR open also gives people the general impression that this is something that is being actively worked on, and it's not. Sure, you can do whatever you want to dissuade people of this: leave it open but leave a comment, leave a tag that says it's blocked, etc., but I don't know why there's a specific decision here against just closing the PR since that feels like the simplest option.

And also, my particular frustration here is that, to me, locking comments should be a last-resort on any project, since it feels generally like punishing the people who are taking the discussion seriously because a few people are not willing to do so. I've accepted that it's okay for things like tracking issues now that it's been clarified what you should actually do instead of commenting in these threads, but I think it's inappropriate here because it just further muddies up the messaging of an open PR that is unable to be merged.

And maybe that frustration is just a me problem that I should get over, but my specific goal with all these comments is to unpack the reasoning behind the decision which I feel is flawed and can lead to more suboptimal decisions in the future, rather than just being specifically salty about this one moderation decision which largely doesn't matter.

[calebcartwright]

@epage

If that is the Cargo team, I don't think we know that. The only thing I know the Cargo team has formally been approached on is whether decision making is wholly owned by the style team or if the Cargo team should be involved, see https://github.com/rust-lang/cargo/issues/13313. I do not see that as blocking for moving forward.

yes I've linked to that issue in the cargo repo a few times myself, including when I summarized current state as we understood it back in https://github.com/rust-lang/rustfmt/issues/4091#issuecomment-1898987293

rustfmt's bound by a few things, the most salient of which here being that its default behavior (regardless of stable vs. nightly) is to reformat things according to the Style Guide and for that formatting to be static (within the confines of a Style Edition now as per RFC 3338)

Fundamentally, our issue on the rustfmt side is that there's Cargo.toml formatting prescriptions in the stable version of the Style Guide: https://doc.rust-lang.org/stable/style-guide/cargo.html and there's some subset of those stable prescriptions that, as we understand it, the Cargo team objects to.

So anything rustfmt would do today, regardless of whether it's behind an unstable option or not, is either going to contradict the Cargo team's wishes or contradict the rustfmt/Style Guide dynamic. That's a problem for us, and it's one we cannot and will not just ignore.

Additionally, we're still not sure what formatting the Cargo team wants. Based on your comment in https://github.com/rust-lang/style-team/issues/188#issuecomment-1897022513 as well as https://github.com/rust-lang/style-team/issues/188#issuecomment-1917193778 my impression is that there are still some outstanding decisions to be made as to what a Cargo.toml file should look like after formatting.

As alluded to in some of those same comments, there are also unanswered process questions (and I'd add a few e.g. would the stability guarantee for Rust code be extended to toml? would iteration be bound to style editions? would there be any equivalent to "nightly only syntax" where the t-style/t-rustfmt process around nightly only syntax be applicable?, etc.)

Many of these aren't questions we can just kick down the road to our future selves. From a rustfmt perspective they are things that we need to know, even for something analogous to an "eRFC"

---

---

I think T-Style and T-Cargo need to get together to get some decisions made and outputs documented.

However, in the spirit of being transparent and pragmatic, Cargo.toml formatting is not something that was able to be pulled in to the 2024 Edition, and as such I know this isn't something that the rustfmt nor style teams will have capacity to dig into until the Edition work is complete, and I'd guess that the Cargo team is pretty busy with edition work as well.

As such I'd suggest we get t-style and t-cargo to reconvene (maybe mid/late October based on the current edition schedule?), identify and enumerate the steps forward with each having a clear owner, and ensure that all teams are on the same page with regards to what's needed to proceed.

[calebcartwright]

@clarfonthey

Generally responding to: https://github.com/rust-lang/rustfmt/issues/4091#issuecomment-2319072401

I understand that you think it would be better to close a PR instead of labeling it as blocked and leaving it open. I understand that if the decision were yours that's what you would've done.

I think you've explained your case, it's been acknowledged and received a response, but we're not going to close it at this time. I am going to have to leave it at that, and I sincerely hope we can chalk that up to an "agree to disagree" because we've now got walls of text about meta topics like issue/PR management and I don't think there's any additional value we're going to gain by continuing the topic.

but my specific goal with all these comments is to unpack the reasoning behind the decision which I feel is flawed and can lead to more suboptimal decisions in the future, rather than just being specifically salty about this one moderation decision which largely doesn't matter.

My .02

open source maintainers are also burdened with having to try to steer the conversations and keep them on track, and on topic. That inherently includes them being the decision makers for determining whether a discussion has been resolved, whether it's constructive, whether it's on or off topic, etc. Obviously we're all human and inevitably there will be differing opinions about such matters, but the onus is still on the maintainers to make both the technical and conversational decisions.

I believe that the makers of the issue tracking systems provide features like being able to hide comments or temporarily stop new comments from coming in to aid them in that responsibility, and GitHub's own documentation states:

https://docs.github.com/en/communities/moderating-comments-and-conversations/locking-conversations

It's appropriate to lock a conversation when the entire conversation is not constructive or violates your community's code of conduct or GitHub's Community Guidelines. When you lock a conversation, you can also specify a reason, which is publicly visible.

As the maintainer team we determined the work was blocked, I communicated it which functionally resolved the discussion of that PR. I asked for a halt of unhelpful/off topic comments on the thread because they were not only lacking constructive/helpful characteristics but they were actively making things more difficult, and tried to redirect conversation to a more constructive location, all of which was repeatedly ignored.

So from my perspective the discussion was both resolved and the additional comments were entirely unconstructive. Given that, and that there'd be more than 8 months of opportunity for such comments to be posted since we'd provided a definitive decision, I decided to use one of the tools at my disposal and locked it, with an additional explanation in https://github.com/rust-lang/rustfmt/pull/5240#issuecomment-2316332796

I will again note that perspectives on what meets the threshold for "resolved" and "constructive" will vary, but I'll also draw a distinction between an open, active discussion versus a communicated decision that people don't like and want to either complain about or try to pressure to reverse.

I firmly believe that in order to steward the projects they are responsible for that open source maintainers still have to have the autonomy to be able to say "no" and "maybe, but not now" in addition to "yes" when responding requests/proposals, even when users don't want to receive/accept a "no" response.

[epage]

Additionally, we're still not sure what formatting the Cargo team wants. Based on your comment in https://github.com/rust-lang/style-team/issues/188#issuecomment-1897022513 as well as https://github.com/rust-lang/style-team/issues/188#issuecomment-1917193778 my impression is that there are still some outstanding decisions to be made as to what a Cargo.toml file should look like after formatting.

I wonder if Josh wearing multiple hats, which can help with cross-team interaction, is causing confusion here. I took this as we provided feedback to the style team, in the zulip stream and summarized in https://github.com/rust-lang/style-team/issues/188#issuecomment-1897022513 and thought the next steps were in the style teams hands.

As alluded to in some of those same comments, there are also unanswered process questions (and I'd add a few e.g. would the stability guarantee for Rust code be extended to toml? would iteration be bound to style editions? would there be any equivalent to "nightly only syntax" where the t-style/t-rustfmt process around nightly only syntax be applicable?, etc.)

I never knew these were in question. I would expect Cargo to operate similarly to Rust.