Skip to content

Restore cargo fmt behavior in workspaces #2513

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

Merged
merged 3 commits into from
Mar 11, 2018
Merged

Restore cargo fmt behavior in workspaces #2513

merged 3 commits into from
Mar 11, 2018

Conversation

rtsuk
Copy link

@rtsuk rtsuk commented Mar 6, 2018

Previously, cargo fmt invoked without parameters would
only format the crate in the current directory, even if
the crate was part of a workspace. This patch restores
that behavior.

Copy link
Member

@nrc nrc left a comment

Choose a reason for hiding this comment

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

Thanks, looks good, just want to check about the Path equality check

let current_dir = env::current_dir()?;
let current_dir_manifest = current_dir.join("Cargo.toml");
let workspace_root_path = PathBuf::from(&metadata.workspace_root);
let in_workspace_root = workspace_root_path == current_dir;
Copy link
Member

Choose a reason for hiding this comment

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

You should probably canonicalise the paths before comparing them (although tbh, I don't know how equality for paths work so it might not be necessary).

@nrc
Copy link
Member

nrc commented Mar 9, 2018

Thanks! Could you rebase please?

Rob Tsuk added 3 commits March 9, 2018 07:24
Previously, cargo fmt  invoked without parameters would
only format the crate in the current directory, even if
the crate was part of a workspace. This patch restores
that behavior.
@rtsuk
Copy link
Author

rtsuk commented Mar 9, 2018

Done!

@nrc nrc merged commit f5ebcd9 into rust-lang:master Mar 11, 2018
@nrc
Copy link
Member

nrc commented Mar 11, 2018

Thank you!

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

Successfully merging this pull request may close these issues.

2 participants