refactor(cli): traversal #3563
Merged
refactor(cli): traversal #3563
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
A-CLI
Conaclos
approved these changes
Aug 2, 2024
| @@ -483,11 +505,16 @@ pub(crate) struct TraversalOptions<'ctx, 'app> { | |||
| /// The approximate number of diagnostics the console will print before | |||
| /// folding the rest into the "skipped diagnostics" counter | |||
| pub(crate) remaining_diagnostics: &'ctx AtomicU32, | |||
|
|
|||
| /// List of paths that should be processed | |||
| pub(crate) evaluated_paths: RwLock<FxHashSet<EvaluatedPath>>, | |||
There was a problem hiding this comment.
Just to be sure: why do we need a RWLock here? Have we several processes that “evaluate” files? Or it is just for toggling if a file was fixed? In this last case, we could use an atomic bool?
There was a problem hiding this comment.
TraversalOptions is sent as a reference across multiple threads. So we need RwLock for two reasons:
- enable interior mutability when having a
&self - lock the resource when multiple threads want to update the list
crates/biome_cli/tests/snapshots/main_cases_diagnostics/max_diagnostics_verbose.snap
Outdated
Show resolved
Hide resolved
crates/biome_cli/tests/snapshots/main_cases_diagnostics/max_diagnostics_verbose.snap
Outdated
Show resolved
Hide resolved
| .map(|pattern| pattern as &dyn Display) | ||
| .collect(); | ||
|
|
||
| visitor.record_list(&pattern_list) |
There was a problem hiding this comment.
In another PR we could certainly accept an impl Iterator in order to avoid the allocation of a Vec.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Part of #3307
The PR breaks down the traversal into two parts:
The logic of the evaluation is pretty much the same we have today, and at the end instead of calling
handle_path, we can a new function calledstore_path.Once the evaluation finishes, we call
traversalagain, but we can a new function calledhandle, which essentially calls the existing functionprocess_file.Now that we have this new information, I took the opportunity to add two new verbose diagnostics that show the evaluated paths, and the paths that were changed.
Test Plan
Updated the current snapshosts.