rustfmt
rustfmt copied to clipboard
rust-lang
Could rustfmt format Cargo.toml ?
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.
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?
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.
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!
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.
@calebcartwright Can I try this one following yours and @gilescope directions?
@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
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 :)
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?
@gilescope it's "Even Better TOML" extension who does that. Link to the GitHub project: https://github.com/tamasfe/taplo
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 .
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)
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!
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)
I implemented the formatting logic https://github.com/rust-lang/rustfmt/pull/5240, review welcomed!
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! 😊
Rustfmt can run on individual files, so, being able to format a Cargo.toml file seems reasonable imho.
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.
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.
@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.
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
Is there a plan here to integrate .toml file formatting directly into the rustfmt library or is it out of the scope?
@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
@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
@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.
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.
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.
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.