-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
chore: use rustfmt imports_granularity option #17421
Conversation
use std::io::{self, Error, Write}; | ||
use std::io::Error; | ||
use std::io::Write; | ||
use std::io::{self}; |
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.
use std::io::{self}; | |
use std::io; |
Maybe this should be reported to rustfmt 👀
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.
Note that
use crate::lexer;
anduse crate::lexer::{self};
are not actually equivalent: The latter only imports the lexer module, while the former looks in all namespaces, so it can import up to 3 items at once, e.g. a function, a module and a macro of the same name.
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.
I think just use std::io;
works in this case though?
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.
I missed that this was auto merging, but we can “fix” these as we see them in the future.
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.
👍
@@ -5,7 +5,8 @@ use std::future::Future; | |||
use std::io; | |||
use std::os::unix::io::RawFd; | |||
use std::pin::Pin; | |||
use std::task::{self, Poll}; | |||
use std::task::Poll; | |||
use std::task::{self}; |
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.
ditto
core/lib.rs
Outdated
pub use serde_v8::ByteString; | ||
pub use serde_v8::DetachedBuffer; | ||
pub use serde_v8::StringOrBuffer; | ||
pub use serde_v8::U16String; | ||
pub use serde_v8::ZeroCopyBuf; | ||
pub use serde_v8::{self}; |
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.
hmmm.. this looks really unnatural. I wonder if there's a way to avoid {self}
use serde::de::{self, SeqAccess as _, Visitor}; | ||
use serde::de::SeqAccess as _; | ||
use serde::de::Visitor; | ||
use serde::de::{self}; |
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.
ditto
@@ -1,7 +1,9 @@ | |||
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. | |||
use std::fmt::{self, Display}; | |||
use std::fmt::Display; | |||
use std::fmt::{self}; |
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.
ditto
.dprint.json
Outdated
@@ -11,7 +11,7 @@ | |||
}, | |||
"exec": { | |||
"associations": "**/*.rs", | |||
"rustfmt": "rustfmt" | |||
"rustfmt": "rustfmt --config imports_granularity=item,wrap_comments=true" |
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.
I'm surprised this works.
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.
I like imports_granularity=item and have wanted that on our code base for a while, but I'm not a fan of wrap_comments=true (just my preference, not sure how other people feel).
@dsherret Removed |
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.
LGTM with just imports_granularity.
Closes denoland#2699 Closes denoland#2347 Uses unstable rustfmt features. Since dprint invokes `rustfmt` we do not need to switch the cargo toolchain to nightly. Do we care about formatting stability of our codebase across Rust versions? (I don't)
Closes #2699
Closes #2347
Uses unstable rustfmt features. Since dprint invokes
rustfmt
we do not need to switch the cargo toolchain to nightly. Do we care about formatting stability of our codebase across Rust versions? (I don't)