Skip to content
This repository has been archived by the owner on Nov 18, 2024. It is now read-only.

Consider following the Cargo.toml conventions #181

Closed
huachaohuang opened this issue Dec 10, 2021 · 11 comments
Closed

Consider following the Cargo.toml conventions #181

huachaohuang opened this issue Dec 10, 2021 · 11 comments
Labels
help wanted Extra attention is needed

Comments

@huachaohuang
Copy link
Owner

There is a Cargo.toml style guide here. We can consider following it.

@huachaohuang huachaohuang added the help wanted Extra attention is needed label Dec 10, 2021
@tisonkun
Copy link
Contributor

tisonkun commented Dec 10, 2021

As mentioned in #183, I'm curious whether there is a checker for these rules; otherwise, even if we create a patch to follow them, it's easy to be broken later.

@huachaohuang
Copy link
Owner Author

I surely don't mean to do that mannually 🤣 . I think we can configure Taplo to follow the conventions somehow.

@ygf11
Copy link

ygf11 commented Dec 10, 2021

I surely don't mean to do that mannually.

Sorry for misunderstanding your thoughts.
But I'm not sure Taplo can do all rule check in conventions.

@tisonkun
Copy link
Contributor

tisonkun commented Dec 10, 2021

@ygf11 that's what a volunteer of this issue should do. Figure out whether taplo can do it or not, and if it can, submit a patch; if not, discuss for alternatives or close the issue as won't do.

@ygf11
Copy link

ygf11 commented Dec 10, 2021

that's what an assignee of this issue should do. Figure out whether taplo can do it or not, and if it can, submit a patch; if not, discuss for alternatives or close the issue as won't do.

Ok, It's interesting. let me have a try:)

@ygf11
Copy link

ygf11 commented Dec 11, 2021

@huachaohuang @tisonkun I found taplo can do some checks, like following:

  1. Sorting key-value pairs by key name alphabetically in a section(exception of package section).
  2. Put a single space both before and after the = between a key and value.(Taplo default behavior)
  3. For array values, expand arrays to multiple line if it exceed the maximum column width(default 80).
  4. For array values, collapse arrays that don't exceed the maximum column width.

conventions can not do:

  1. Put a blank line between the last key-value pair in a section and the header of the next section.
  2. Key-value pairs sorting style in package section, which puts name and version in the top level, then other key-value pairs(alphabetical order), and put description at the end of that section.
  3. When a key-value pair is too long in dependency section , modify it like following:
[dependencies]
crate1 = { path = "crate1", version = "1.2.3" }

[dependencies.extremely_long_crate_name_goes_here]
path = "extremely_long_path_name_goes_right_here"
version = "4.5.6"
  1. Other trivial conventions, like author, homepage,description check etc.

I also found another Cargo.toml formatter, dprint.
Other than four rules which Taplo supports, it also supports key-value pairs sorting style in package section.
But it has an exception, sorting key-value pairs alphabetically only take affect on dependency section, other sections will not , like features.

what do you think?

@tisonkun
Copy link
Contributor

@ygf11 sorry to reply late. Last week we're preparing the release and this comment was missed from my schedule. I'm looking into it now.

@huachaohuang
Copy link
Owner Author

Oh, I didn't recall this either. @ygf11 @tisonkun thanks for helping.

@tisonkun
Copy link
Contributor

tisonkun commented Dec 21, 2021

@ygf11 thanks for your investigation. I think you can submit a pull request turn on relative checks in taplo's config file, and fix violations correspondingly.

For those conventions cannot do, you can try to create an issue in taplo community (e.g., tamasfe/taplo#196) and see if you can cooperate somehow. This one is a suggestion only, not what you're asked to do.

@xxchan
Copy link

xxchan commented Dec 30, 2021

After seeing the style guild, I'm wondering whether rustfmt supports it, and I found this has been discussed rust-lang/rustfmt#4091. They will support this, but no progress yet. So I think we can just turn on some taplo check now and use rustfmt solely when it's ready.

@tisonkun
Copy link
Contributor

tisonkun commented Jan 6, 2022

@xxchan sorry for reply late. As @ygf11 doesn't response, would you mind to submit a pull request to turn on related options?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants