balena-io-modules / jellyschema

JellySchema - data validation, UI form generation
Apache License 2.0
4 stars 2 forks source link

Re-enable managing imports by rustfmt #57

Closed cyplo closed 5 years ago

cyplo commented 5 years ago

As we're using VSCode mainly now - it's hard for me to rely on 'organize imports' from CLion to keep the imports clean thus making rustfmt responsible for that.

cyplo commented 5 years ago

needs https://github.com/balena-io-modules/balena-temen/pull/48

zrzka commented 5 years ago

@cyplo I'd like to have this one closed without merging and changing rustfmt behavior. Reasons:

In other words, this order still stands:

Example before:

use std::collections::HashMap;
use rand::{self, RngCore};
use serde_json::Value;
use uuid::Uuid;
use super::Result;
use crate::context::Context;
use crate::error::Result;
use self::uuidv4 as uu;

Example after:

use std::collections::HashMap;

use rand::{self, RngCore};
use serde_json::Value;
use uuid::Uuid;

use crate::context::Context;
use crate::error::Result;

use super::Result;

use self::uuidv4 as uu;
zrzka commented 5 years ago

Not mentioning, that sometimes, this rule can be skipped and import can be organised by a "function" / ... like:

// JSON related crates
...

// YAML related crates
...
cyplo commented 5 years ago

Alrighty, probably worth taking a look into making rustfmt smarter as you said then - I wrote this down in the list of things to do later :)

zrzka commented 5 years ago

@cyplo what happens when you split import to groups manually (empty lines between groups) and then run rustfmt? Does rustfmt keep groups and reorders items in each group individually?

zrzka commented 5 years ago

Maybe this is a way, like semi automate it. Groups manually, but keep them sorted.

cyplo commented 5 years ago

will take a look based on this code in this PR, with default rustfmt settings and let you know

cyplo commented 5 years ago

It seems that I can introduce arbitrary groups by putting in new lines between them. Then within the group rustfmt keeps the group sorted. But it does not reshuffle/resort between the groups themselves nor it does not seem to change the order of the groups introduced by me. All of this is with the default settings - tested on the branch for this PR.

Would that work @zrzka ? Or do you see a place where this would break or want to experiment with it yourself ?

zrzka commented 5 years ago

@cyplo then I think you can reopen it ... I'd like to stick with ...

* std
* extern crates
* crate
* super
* self

... order. And if rustfmt doesn't regroup them, I'm ok with rustfmt touching / ordering them.

Also it's not a hard rule, it's about readability only, so, if there will be a mistake, we can always PR it.

cyplo commented 5 years ago

Will play with how CLion and rustfmt interact and if they don't fight I will reopen this one and ping you :)