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.

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

feat: apply transforms on stdin #320

Merged
merged 12 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion crates/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ path = "src/lib.rs"
anyhow = { version = "1.0.70" }
clap = { version = "4.1.13", features = ["derive"] }
indicatif = { version = "0.17.5" }
ignore = { version = "0.4.20" }
# Do *NOT* upgrade beyond 1.0.171 until https://github.com/serde-rs/serde/issues/2538 is fixed
serde = { version = "1.0.164", features = ["derive"] }
serde_json = { version = "1.0.96" }
Expand Down
144 changes: 84 additions & 60 deletions crates/cli/src/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ use tracing_opentelemetry::OpenTelemetrySpanExt as _;

use grit_cache::paths::cache_for_cwd;
use grit_util::{FileRange, Position};
use ignore::Walk;
use marzano_language::target_language::expand_paths;

use indicatif::{MultiProgress, ProgressBar, ProgressDrawTarget, ProgressStyle};
#[allow(unused_imports)]
use marzano_core::built_in_functions::BuiltIns;
Expand Down Expand Up @@ -140,10 +141,9 @@ macro_rules! emit_error {

#[allow(clippy::too_many_arguments)]
pub async fn par_apply_pattern<M>(
file_walker: Walk,
multi: MultiProgress,
compiled: Problem,
my_input: &ApplyInput,
my_input: ApplyInput,
mut owned_emitter: M,
processed: &AtomicI32,
details: &mut ApplyDetails,
Expand Down Expand Up @@ -190,65 +190,77 @@ where
let mut interactive = arg.interactive;
let min_level = &arg.visibility;

let (file_paths_tx, file_paths_rx) = channel();
let (found_count, disk_paths) = match my_input {
ApplyInput::Disk(ref my_input) => {
let (file_paths_tx, file_paths_rx) = channel();

for file in file_walker {
let file = emit_error!(owned_emitter, &arg.visibility, file);
if file.file_type().unwrap().is_dir() {
continue;
}
if !&compiled.language.match_extension(
file.path()
.extension()
.unwrap_or_default()
.to_str()
.unwrap_or_default(),
) {
processed.fetch_add(1, Ordering::SeqCst);
let path_string = file.path().to_string_lossy().to_string();
if my_input.paths.contains(&file.path().to_path_buf()) {
let log = MatchResult::AnalysisLog(AnalysisLog {
level: 410,
message: format!(
"Skipped {} since it is not a {} file",
path_string,
&compiled.language.to_string()
),
position: Position::first(),
file: path_string.to_string(),
engine_id: "marzano".to_string(),
range: None,
syntax_tree: None,
source: None,
});
let done_file = MatchResult::DoneFile(DoneFile {
relative_file_path: path_string,
has_results: Some(false),
file_hash: None,
from_cache: false,
});
emitter.handle_results(
vec![log, done_file],
details,
arg.dry_run,
min_level,
arg.format,
&mut interactive,
None,
Some(processed),
None,
&compiled.language,
);
let file_walker = emit_error!(
owned_emitter,
&arg.visibility,
expand_paths(&my_input.paths, Some(&[(&compiled.language).into()]))
);

for file in file_walker {
let file = emit_error!(owned_emitter, &arg.visibility, file);
if file.file_type().unwrap().is_dir() {
continue;
}
if !&compiled.language.match_extension(
file.path()
.extension()
.unwrap_or_default()
.to_str()
.unwrap_or_default(),
) {
processed.fetch_add(1, Ordering::SeqCst);
let path_string = file.path().to_string_lossy().to_string();
if my_input.paths.contains(&file.path().to_path_buf()) {
let log = MatchResult::AnalysisLog(AnalysisLog {
level: 410,
message: format!(
"Skipped {} since it is not a {} file",
path_string,
&compiled.language.to_string()
),
position: Position::first(),
file: path_string.to_string(),
engine_id: "marzano".to_string(),
range: None,
syntax_tree: None,
source: None,
});
let done_file = MatchResult::DoneFile(DoneFile {
relative_file_path: path_string,
has_results: Some(false),
file_hash: None,
from_cache: false,
});
emitter.handle_results(
vec![log, done_file],
details,
arg.dry_run,
min_level,
arg.format,
&mut interactive,
None,
Some(processed),
None,
&compiled.language,
);
}
continue;
}
file_paths_tx.send(file.path().to_path_buf()).unwrap();
}
continue;
}
file_paths_tx.send(file.path().to_path_buf()).unwrap();
}

drop(file_paths_tx);
drop(file_paths_tx);

let found_paths = file_paths_rx.iter().collect::<Vec<_>>();
(found_paths.len(), Some(found_paths))
}
ApplyInput::Virtual(ref virtual_info) => (virtual_info.files.len(), None),
};
Comment on lines +193 to +262
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor to reduce complexity and improve readability.

The handling of ApplyInput::Disk and ApplyInput::Virtual within the match statement is quite complex and lengthy. Consider refactoring this into separate functions or methods to handle each case. This would improve readability and maintainability of the code.

- let (found_count, disk_paths) = match my_input {
-     ApplyInput::Disk(ref my_input) => {
-         // existing code...
-     }
-     ApplyInput::Virtual(ref virtual_info) => (virtual_info.files.len(), None),
- };
+ let (found_count, disk_paths) = handle_input(my_input);

And then define handle_input function elsewhere in the module.


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
let (found_count, disk_paths) = match my_input {
ApplyInput::Disk(ref my_input) => {
let (file_paths_tx, file_paths_rx) = channel();
for file in file_walker {
let file = emit_error!(owned_emitter, &arg.visibility, file);
if file.file_type().unwrap().is_dir() {
continue;
}
if !&compiled.language.match_extension(
file.path()
.extension()
.unwrap_or_default()
.to_str()
.unwrap_or_default(),
) {
processed.fetch_add(1, Ordering::SeqCst);
let path_string = file.path().to_string_lossy().to_string();
if my_input.paths.contains(&file.path().to_path_buf()) {
let log = MatchResult::AnalysisLog(AnalysisLog {
level: 410,
message: format!(
"Skipped {} since it is not a {} file",
path_string,
&compiled.language.to_string()
),
position: Position::first(),
file: path_string.to_string(),
engine_id: "marzano".to_string(),
range: None,
syntax_tree: None,
source: None,
});
let done_file = MatchResult::DoneFile(DoneFile {
relative_file_path: path_string,
has_results: Some(false),
file_hash: None,
from_cache: false,
});
emitter.handle_results(
vec![log, done_file],
details,
arg.dry_run,
min_level,
arg.format,
&mut interactive,
None,
Some(processed),
None,
&compiled.language,
);
let file_walker = emit_error!(
owned_emitter,
&arg.visibility,
expand_paths(&my_input.paths, Some(&[(&compiled.language).into()]))
);
for file in file_walker {
let file = emit_error!(owned_emitter, &arg.visibility, file);
if file.file_type().unwrap().is_dir() {
continue;
}
if !&compiled.language.match_extension(
file.path()
.extension()
.unwrap_or_default()
.to_str()
.unwrap_or_default(),
) {
processed.fetch_add(1, Ordering::SeqCst);
let path_string = file.path().to_string_lossy().to_string();
if my_input.paths.contains(&file.path().to_path_buf()) {
let log = MatchResult::AnalysisLog(AnalysisLog {
level: 410,
message: format!(
"Skipped {} since it is not a {} file",
path_string,
&compiled.language.to_string()
),
position: Position::first(),
file: path_string.to_string(),
engine_id: "marzano".to_string(),
range: None,
syntax_tree: None,
source: None,
});
let done_file = MatchResult::DoneFile(DoneFile {
relative_file_path: path_string,
has_results: Some(false),
file_hash: None,
from_cache: false,
});
emitter.handle_results(
vec![log, done_file],
details,
arg.dry_run,
min_level,
arg.format,
&mut interactive,
None,
Some(processed),
None,
&compiled.language,
);
}
continue;
}
file_paths_tx.send(file.path().to_path_buf()).unwrap();
}
continue;
}
file_paths_tx.send(file.path().to_path_buf()).unwrap();
}
drop(file_paths_tx);
drop(file_paths_tx);
let found_paths = file_paths_rx.iter().collect::<Vec<_>>();
(found_paths.len(), Some(found_paths))
}
ApplyInput::Virtual(ref virtual_info) => (virtual_info.files.len(), None),
};
let (found_count, disk_paths) = handle_input(my_input);


let found_paths = file_paths_rx.iter().collect::<Vec<_>>();
let found_count = found_paths.len();
if let Some(pg) = pg {
pg.set_length(found_count.try_into().unwrap());
}
Expand All @@ -257,7 +269,7 @@ where
#[cfg(feature = "grit_timing")]
debug!(
"Walked {} files in {}ms",
found_paths.len(),
found_count,
current_timer.elapsed().as_millis()
);

Expand Down Expand Up @@ -323,7 +335,19 @@ where
#[cfg(feature = "grit_tracing")]
task_span.set_parent(grouped_ctx);
task_span.in_scope(|| {
compiled.execute_paths_streaming(found_paths, context, tx, cache_ref);
match disk_paths {
Some(found_paths) => {
compiled.execute_paths_streaming(found_paths, context, tx, cache_ref);
}
None => {
if let ApplyInput::Virtual(my_input) = my_input {
compiled.execute_files_streaming(my_input.files, context, tx, cache_ref);
} else {
unreachable!();
}
}
}

loop {
if processed.load(Ordering::SeqCst) >= found_count.try_into().unwrap()
|| !should_continue.load(Ordering::SeqCst)
Expand Down
2 changes: 1 addition & 1 deletion crates/cli/src/commands/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub(crate) async fn run_apply(
details,
None,
None,
flags.into(),
flags,
None,
)
.await
Expand Down
Loading
Loading