cargo-sort icon indicating copy to clipboard operation
cargo-sort copied to clipboard

Line grouping doesn't seem to work

Open nanocryk opened this issue 3 years ago • 6 comments

I'm trying to run cargo sort -g on this Cargo.toml : https://github.com/PureStake/moonbeam/blob/master/node/cli/Cargo.toml

However it seems to completely ignore the groupings/newlines. It is also not idempotent as running the command multiple times in a row keeps giving different outputs (no longer changes after 3 steps for this TOML).

Finally, it reorder lines only containing comments by looking at the text of the comment as if it was a key name. While it could make sense for a commented entry, it prevents to use comments to name each group.

nanocryk avatar Nov 15 '21 17:11 nanocryk

Ok so although this behavior is REALLY odd it does seem "correct" (the --group thing is kinda just a heuristic or a weak best effort).

Although I will look into why items seem to migrate up groups, that does seem wrong.

Finally, it reorder lines only containing comments by looking at the text of the comment as if it was a key name. While it could make sense for a commented entry, it prevents to use comments to name each group.

Do you have an example of this?

The main problem when doing anything with whitespace/comments is that you have to either associate it with the thing before or the thing after, and you end up guessing what the meaning is that's why you get weird groups like

log = "0.4.8"
structopt = "0.3.8"
parity-scale-codec = '2.2'

sp-core= ""
sc-cli = ""
sc-service= ""
sc-tracing = ""
sp-runtime = ""
sc-telemetry = ""
frame-benchmarking-cli = ""
try-runtime-cli = ""

turning into

log = "0.4.8"
parity-scale-codec = '2.2'
structopt = "0.3.8"
frame-benchmarking-cli = ""
sc-cli = ""
sc-service = ""
sc-telemetry= ""
sc-tracing = ""

sp-core = ""
sp-runtime = ""
try-runtime-cli = ""

DevinR528 avatar Nov 15 '21 21:11 DevinR528

Hmm I think I get the issue. The empty line is detected and the groups are made, but then the empty line is grouped with sp-core and moved with it when sorting the second group.

If empty lines are used to make groups they should be be detached and inserted back at the start of the group after the sort. (this line containing a comment could work too)

I'll take a look in the code if I can find something.

nanocryk avatar Nov 16 '21 09:11 nanocryk

If empty lines are used to make groups they should be be detached and inserted back at the start of the group after the sort. (this line containing a comment could work too)

This would be ideal! It is probably possible, it's been a while since I've really worked on this but I think that would work. I need to spend a weekend updating everything (https://github.com/DevinR528/cargo-sort/issues/26#issuecomment-953926497 and see what can be done about this too) it just might be a bit before I can get to it :(

What works for https://github.com/ruma/ruma/blob/main/crates/ruma/Cargo.toml is to use cargo-sort as a check instead of the fixer. Obviously, this isn't really an acceptable answer but for the time being that's how you can get it to act consistently.

DevinR528 avatar Nov 18 '21 14:11 DevinR528

Hey !

I experimented a bit and implemented my own crate :) https://github.com/PureStake/toml_sort

It sorts the entries by sections, with custom lists of priority keys. Feel free to take a look and borrow some ideas :)

nanocryk avatar Nov 22 '21 10:11 nanocryk

Sweet! I did use some of yours for #29 (the new decore system is a lot of working with Options which is unfortunate :shrug:) but having what you did guide helped a lot. Also once I get it back to working like the old version then I can go in and correct the grouping code (I'll add you as an author on the commits if that ok since you will have helped it along majorly, or I could get it back to working like the old version and if you wanted to PR I could do that too?).

DevinR528 avatar Nov 23 '21 17:11 DevinR528

I'll add you as an author on the commits if that ok since you will have helped it along majorly

I'm good with that :)

nanocryk avatar Nov 24 '21 12:11 nanocryk