Add ignore flag to be used with workspace option#118
Add ignore flag to be used with workspace option#118Hoziax wants to merge 4 commits intoDevinR528:mainfrom
Conversation
|
I am not opposed to adding this although I do wonder why you want to skip it. There isn't any harm in sorting that file too, is there? What is the motivation as to why it should be skipped? |
|
The original idea/problem as to why I opened this pull request was the following. We have a CI pipeline that uses hakari to speed up builds. The idea now is to add cargo sort to that. However (our) hakari expects a very specific order for the workspace-hack module. Not doing anything would result in the two passing the So as you mentioned one option would be to define an explicit order that sorts all |
| fn parse_workspace_member(member: &str, dir: &str) -> IoResult<Vec<String>> { | ||
| let base_path = format!("{dir}/{member}"); | ||
| if member.contains('*') || member.contains('?') { | ||
| let parsed_members = glob::glob(&base_path)? |
There was a problem hiding this comment.
This hits the filesystem for each member passed on the command line, right? Isn't all we have to do conceptually wildcard matching? The wildmatch crate offers that, though without ** support – which I think is okay to not have here?
There was a problem hiding this comment.
It is not easy to see from the commit history but this code mostly got moved and it was already this way before.
|
Thank you for explaining your usecase. That makes sense to me, tools fighting each other is no fun :) I'll give this a review in the upcoming days. |
| If your workspace contains a Cargo.toml that should not be sorted, for example if it has been automatically generated, it can | ||
| be ignored with the ignore flag: |
There was a problem hiding this comment.
This file doesn't use hard-wraps from what I can see.
| If your workspace contains a Cargo.toml that should not be sorted, for example if it has been automatically generated, it can | |
| be ignored with the ignore flag: | |
| If your workspace contains a Cargo.toml that should not be sorted, for example if it has been automatically generated, it can be ignored with the ignore flag: |
| /// workspace section or that are specified in the `ignore` parameter will be removed. | ||
| /// Returns an error if [`parse_workspace_member`] failed to parse a glob pattern in any workspace | ||
| /// member. | ||
| fn parse_and_filter_workspace_members( |
There was a problem hiding this comment.
Please define (utility) functions below their usage (roughly in order of how they are used).
| fn parse_workspace_member(member: &str, dir: &str) -> IoResult<Vec<String>> { | ||
| let base_path = format!("{dir}/{member}"); | ||
| if member.contains('*') || member.contains('?') { | ||
| let parsed_members = glob::glob(&base_path)? |
There was a problem hiding this comment.
It is not easy to see from the commit history but this code mostly got moved and it was already this way before.
| if let Some(Item::Table(ws)) = workspace { | ||
| // The workspace excludes, used to filter members by | ||
| let excludes: Vec<&str> = | ||
| ws.get("exclude").map_or_else(Vec::new, array_string_members); |
There was a problem hiding this comment.
@Hoziax Can't we just append our values from ignore to this vector? Why did we have to change so much of the code to support the feature? It makes it hard to review as it is not clear what is old and what is new code.
3 participants
Sometimes it can be useful to exclude certain modules/
Cargo.tomlfiles when runningcargo sorton a complete workspace. For example if you use aworkspace-hackmodule.This pull requests adds a ignore flag (
-i/--ignore) that can be used in combination with the workspace flag (-w/--workspace) that lets a user specify modules that should be skipped when runningcargo sort. Example usage:cargo sort -w -i "workspace-hack" my_projectThe ignore flag also supports a list of values and the
?and*glob patterns: