Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Line grouping doesn't seem to work #27

Open
nanocryk opened this issue Nov 15, 2021 · 6 comments
Open

Line grouping doesn't seem to work #27

nanocryk opened this issue Nov 15, 2021 · 6 comments

Comments

@nanocryk
Copy link

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.

@DevinR528
Copy link
Owner

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 = ""

@nanocryk
Copy link
Author

nanocryk commented Nov 16, 2021

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.

@DevinR528
Copy link
Owner

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 (#26 (comment) 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.

@nanocryk
Copy link
Author

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 :)

@DevinR528
Copy link
Owner

Sweet! I did use some of yours for #29 (the new decore system is a lot of working with Options which is unfortunate 🤷) 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?).

@nanocryk
Copy link
Author

nanocryk commented Nov 24, 2021

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants