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.

Sign up for GitHub

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

Jump to bottom

Allow shortcuts to directories to be used for ./x.py fmt #107948

Merged
merged 1 commit into from
Feb 14, 2023
Merged

Allow shortcuts to directories to be used for ./x.py fmt #107948

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 38 additions & 2 deletions src/bootstrap/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,46 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) {
let (tx, rx): (SyncSender<PathBuf>, _) = std::sync::mpsc::sync_channel(128);
let walker = match paths.get(0) {
Some(first) => {
let mut walker = WalkBuilder::new(first);
let find_shortcut_candidates = |p: &PathBuf| {
let mut candidates = Vec::new();
for candidate in WalkBuilder::new(src.clone()).max_depth(Some(3)).build() {
if let Ok(entry) = candidate {
if let Some(dir_name) = p.file_name() {
if entry.path().is_dir() && entry.file_name() == dir_name {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems this style is better:

 if let Ok(entry) = candidate &&
    let Some(dir_name) = p.file_name() &&
    entry.path().is_dir() && entry.file_name() == dir_name {
   ...
}

Copy link
Member Author

@jieyouxu jieyouxu Feb 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love to use that, but for src/bootstrap/format.rs if-let chains features aren't already enabled

error[E0658]: `let` expressions in this position are unstable
   --> format.rs:199:24
    |
199 |                     if let Ok(entry) = candidate
    |                        ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #53667 <https://github.com/rust-lang/rust/issues/53667> for more information

I've been using if-let chains in compiler/ and it's been really nice. Should I enable the feature or have the code remain as-is?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's out of scope for this particular PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bootstrap itself is rarely (ever?) built with nightly.
The only thing to do is maybe leave a FIXME, but do not enable nightly features.

Copy link
Member

@jyn514 jyn514 Feb 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please don't use nightly in bootstrap, if we start doing that we have to add cfg(bootstrap) to bootstrap itself and that will not be fun

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right I forgot bootstrap needed to work with stable 😄, the PR was squashed as-is without touching nightly features.

candidates.push(entry.into_path());
}
}
}
}
candidates
};

// Only try to look for shortcut candidates for single component paths like
// `std` and not for e.g. relative paths like `../library/std`.
let should_look_for_shortcut_dir = |p: &PathBuf| p.components().count() == 1;

let mut walker = if should_look_for_shortcut_dir(first) {
if let [single_candidate] = &find_shortcut_candidates(first)[..] {
jyn514 marked this conversation as resolved.
Show resolved Hide resolved
WalkBuilder::new(single_candidate)
} else {
WalkBuilder::new(first)
}
} else {
WalkBuilder::new(first)
};

for path in &paths[1..] {
walker.add(path);
if should_look_for_shortcut_dir(path) {
if let [single_candidate] = &find_shortcut_candidates(path)[..] {
walker.add(single_candidate);
} else {
walker.add(path);
}
} else {
walker.add(path);
}
}

walker
}
None => WalkBuilder::new(src.clone()),
Expand Down