-
Notifications
You must be signed in to change notification settings - Fork 931
fix sorting of use statements with raw identifiers #3795
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
fix sorting of use statements with raw identifiers #3795
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late review. The code LGTM, but as you mentioned I think that we need to version gate this. If that would require a lot of work, then I will create a branch for 2.0 and change the base to that branch.
The challenge from my perspective is how to make the config version value accessible in the A relatively quick and easy way I can see to do that would be to include the Something like: pub(crate) enum UseSegment {
Ident(String, Option<String>, Version),
Slf(Option<String>, Version),
Super(Option<String>, Version),
Crate(Option<String>, Version),
Glob,
List(Vec<UseTree>),
} And then in match (self, other) {
(&Slf(ref a), &Slf(ref b))
| (&Super(ref a), &Super(ref b))
| (&Crate(ref a), &Crate(ref b)) => match (a, b) {
(Some(sa), Some(sb)) => {
if self.version == Version::One {
sa.cmp(sb)
} else {
sa.trim_start_matches("r#").cmp(sb.trim_start_matches("r#"))
}
}
(_, _) => a.cmp(b),
},
.... Thoughts? |
Thank you for the elaboration. Given the amount of work and the fact that we will remove the version gating code once we prepare for 2.0, I would rather merge it to the branch for 2.0 and worry about merging-disaster later 😛 Thank you again for your work! |
SGTM! |
Hey @calebcartwright, thinking about how we could backport this and include the version gates. Maybe we could update pub(crate) enum UseSegmentKind {
Ident(String, Option<String>),
Slf(Option<String>),
Super(Option<String>),
Crate(Option<String>),
Glob,
List(Vec<UseTree>),
}
pub(crate) struct UseSegment {
kind: UseSegmentKind,
version: Version
} Let me know if you think this is the approach we should take or if you think passing the version to each enum variant would be a better solution. |
I'm open to just about anything, but the cleaner/simpler the better! |
Resolves #3791
Does this need to be version gated? It looks more like a sorting bug fix IMO, but I can certainly add a version gate if this is indeed a formatting change.