Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feat: revamp diff printing schema
BREAKING CHANGE: now diff is print one by one
  • Loading branch information
HerringtonDarkholme committed Sep 5, 2023
1 parent 4860aa4 commit 2b30111
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 47 deletions.
10 changes: 6 additions & 4 deletions crates/cli/src/print/cloud_print.rs
Expand Up @@ -65,13 +65,15 @@ impl<W: Write> Printer for CloudPrinter<W> {
unreachable!()
}

fn print_rule_diffs<'a>(
fn print_rule_diffs(
&self,
diffs: Diffs!('a),
diffs: Vec<(Diff<'_>, &RuleConfig<SgLang>)>,
path: &Path,
rule: &RuleConfig<SgLang>,
) -> Result<()> {
print_rule(self, diffs.map(|d| d.node_match), path, rule)
for (diff, rule) in diffs {
print_rule(self, std::iter::once(diff.node_match), path, rule)?;
}
Ok(())
}
}

Expand Down
15 changes: 7 additions & 8 deletions crates/cli/src/print/colored_print.rs
Expand Up @@ -174,17 +174,16 @@ impl<W: WriteColor> Printer for ColoredPrinter<W> {
let context = self.diff_context();
print_diffs(diffs, path, &self.styles, writer, context)
}
fn print_rule_diffs<'a>(
fn print_rule_diffs(
&self,
diffs: Diffs!('a),
diffs: Vec<(Diff<'_>, &RuleConfig<SgLang>)>,
path: &Path,
rule: &RuleConfig<SgLang>,
) -> Result<()> {
let writer = &mut *self.writer.lock().expect("should success");
let context = self.diff_context();
let mut start = 0;
print_prelude(path, &self.styles, writer)?;
for diff in diffs {
for (diff, rule) in diffs {
let range = diff.node_match.range();
// skip overlapping diff
if range.start < start {
Expand All @@ -200,10 +199,10 @@ impl<W: WriteColor> Printer for ColoredPrinter<W> {
&source[start..],
);
print_diff(source, &new_str, &self.styles, writer, context)?;
}
if let Some(note) = &rule.note {
writeln!(writer, "{}", self.styles.rule.note.paint("Note:"))?;
writeln!(writer, "{note}")?;
if let Some(note) = &rule.note {
writeln!(writer, "{}", self.styles.rule.note.paint("Note:"))?;
writeln!(writer, "{note}")?;
}
}
Ok(())
}
Expand Down
4 changes: 2 additions & 2 deletions crates/cli/src/print/colored_print/test.rs
Expand Up @@ -235,9 +235,9 @@ fix: '{rewrite}'"
let matcher = rule.get_matcher(&globals).expect("should parse");
let fixer = rule.fixer.as_ref().expect("should have fixer");
let matches = grep.root().find_all(&matcher);
let diffs = matches.map(|n| Diff::generate(n, &pattern, fixer));
let diffs = matches.map(|n| (Diff::generate(n, &pattern, fixer), &rule));
printer
.print_rule_diffs(diffs, Path::new("test.tsx"), &rule)
.print_rule_diffs(diffs.collect(), Path::new("test.tsx"))
.expect("test only");
let text = get_text(&printer);
assert!(text.contains("test.tsx"), "{note}");
Expand Down
68 changes: 49 additions & 19 deletions crates/cli/src/print/interactive_print.rs
Expand Up @@ -100,51 +100,81 @@ impl<P: Printer> Printer for InteractivePrinter<P> {
return self.rewrite_action(diffs.collect(), &path);
}
utils::run_in_alternate_screen(|| {
let all = print_diffs_and_prompt_action(self, &path, diffs, None)?;
let all = print_diffs_and_prompt_action(self, &path, diffs.collect())?;
if all {
self.accept_all.store(true, Ordering::SeqCst);
}
Ok(())
})
}
fn print_rule_diffs<'a>(
fn print_rule_diffs(
&self,
diffs: Diffs!('a),
diffs: Vec<(Diff<'_>, &RuleConfig<SgLang>)>,
path: &Path,
rule: &RuleConfig<SgLang>,
) -> Result<()> {
let path = path.to_path_buf();
if self.accept_all.load(Ordering::SeqCst) {
return self.rewrite_action(diffs.collect(), &path);
return self.rewrite_action(diffs.into_iter().map(|p| p.0).collect(), &path);
}
utils::run_in_alternate_screen(|| {
let all = print_diffs_and_prompt_action(self, &path, diffs, Some(rule))?;
let mut confirmed = vec![];
let mut all = false;
for (diff, rule) in diffs {
if all {
self.accept_all.store(true, Ordering::SeqCst);
confirmed.push(diff);
continue;
}
Ok(())
})
all = print_rule_diff_and_prompt_action(self, &path, (diff, rule), &mut confirmed)?;
}
self.rewrite_action(confirmed, &path)?;
if all {
self.accept_all.store(true, Ordering::SeqCst);
}
Ok(())
}
}
/// returns if accept_all is chosen
fn print_rule_diff_and_prompt_action<'a>(
interactive: &InteractivePrinter<impl Printer>,
path: &PathBuf,
(diff, rule): (Diff<'a>, &RuleConfig<SgLang>),
confirmed: &mut Vec<Diff<'a>>,
) -> Result<bool> {
let printer = &interactive.inner;
utils::run_in_alternate_screen(|| {
printer.print_rule_diffs(vec![(diff.clone(), rule)], path)?;
match interactive.prompt_edit() {
'y' => {
confirmed.push(diff);
Ok(false)
}
'a' => {
confirmed.push(diff);
Ok(true)
}
'e' => {
let pos = diff.node_match.start_pos().0;
open_in_editor(path, pos)?;
Ok(false)
}
'q' => Err(anyhow::anyhow!("Exit interactive editing")),
'n' => Ok(false),
_ => Ok(false),
}
})
}

/// returns if accept_all is chosen
fn print_diffs_and_prompt_action<'a>(
fn print_diffs_and_prompt_action(
interactive: &InteractivePrinter<impl Printer>,
path: &PathBuf,
diffs: Diffs!('a),
rule: Option<&RuleConfig<SgLang>>,
diffs: Vec<Diff<'_>>,
) -> Result<bool> {
let printer = &interactive.inner;
let diffs: Vec<_> = diffs.collect();
let first_match = match diffs.first() {
Some(n) => n.node_match.start_pos().0,
None => return Ok(false),
};
if let Some(rule) = rule {
printer.print_rule_diffs(diffs.clone().into_iter(), path, rule)?;
} else {
printer.print_diffs(diffs.clone().into_iter(), path)?;
}
printer.print_diffs(diffs.clone().into_iter(), path)?;
match interactive.prompt_edit() {
'y' => {
interactive.rewrite_action(diffs, path)?;
Expand Down
7 changes: 3 additions & 4 deletions crates/cli/src/print/json_print.rs
Expand Up @@ -301,14 +301,13 @@ impl<W: Write> Printer for JSONPrinter<W> {
});
self.print_docs(jsons)
}
fn print_rule_diffs<'a>(
fn print_rule_diffs(
&self,
diffs: Diffs!('a),
diffs: Vec<(Diff<'_>, &RuleConfig<SgLang>)>,
path: &Path,
rule: &RuleConfig<SgLang>,
) -> Result<()> {
let path = path.to_string_lossy();
let jsons = diffs.map(|diff| {
let jsons = diffs.into_iter().map(|(diff, rule)| {
let mut v = RuleMatchJSON::new(diff.node_match, &path, rule);
v.matched.replacement = Some(diff.replacement);
v
Expand Down
5 changes: 2 additions & 3 deletions crates/cli/src/print/mod.rs
Expand Up @@ -40,11 +40,10 @@ pub trait Printer {
) -> Result<()>;
fn print_matches<'a>(&self, matches: Matches!('a), path: &Path) -> Result<()>;
fn print_diffs<'a>(&self, diffs: Diffs!('a), path: &Path) -> Result<()>;
fn print_rule_diffs<'a>(
fn print_rule_diffs(
&self,
diffs: Diffs!('a),
diffs: Vec<(Diff<'_>, &RuleConfig<SgLang>)>,
path: &Path,
rule: &RuleConfig<SgLang>,
) -> Result<()>;
/// Run before all printing. One CLI will run this exactly once.
#[inline]
Expand Down
36 changes: 29 additions & 7 deletions crates/cli/src/scan.rs
Expand Up @@ -2,7 +2,6 @@ use std::path::{Path, PathBuf};

use anyhow::{Context, Result};
use ast_grep_config::{CombinedScan, RuleCollection, RuleConfig, Severity};
use ast_grep_core::traversal::Visitor;
use ast_grep_core::{NodeMatch, StrDoc};
use bit_set::BitSet;
use clap::Args;
Expand Down Expand Up @@ -138,10 +137,15 @@ impl<P: Printer + Sync> Worker for ScanWithConfig<P> {
let rules = self.configs.for_path(path);
let combined = CombinedScan::new(rules);
if self.arg.output.needs_interacive() {
for (nm, idx) in combined.diffs(&grep, hit_set) {
let rule = combined.get_rule(idx);
match_rule_on_file(path, vec![nm], rule, &file_content, &self.printer)?;
}
let diffs = combined
.diffs(&grep, hit_set)
.into_iter()
.map(|(nm, idx)| {
let rule = combined.get_rule(idx);
(nm, rule)
})
.collect();
match_rule_diff_on_file(path, diffs, &self.printer)?;
continue;
}
let matched = combined.scan(&grep, hit_set);
Expand Down Expand Up @@ -215,6 +219,22 @@ impl<P: Printer + Sync> StdInWorker for ScanWithRule<P> {
has_match.then(|| (PathBuf::from("STDIN"), grep))
}
}
fn match_rule_diff_on_file(
path: &Path,
matches: Vec<(NodeMatch<StrDoc<SgLang>>, &RuleConfig<SgLang>)>,
reporter: &impl Printer,
) -> Result<()> {
let diffs = matches
.into_iter()
.filter_map(|(m, rule)| {
let fix = rule.fixer.as_ref()?;
let diff = Diff::generate(m, &rule.matcher, fix);
Some((diff, rule))
})
.collect();
reporter.print_rule_diffs(diffs, path)?;
Ok(())
}

fn match_rule_on_file(
path: &Path,
Expand All @@ -226,8 +246,10 @@ fn match_rule_on_file(
let matches = matches.into_iter();
let file = SimpleFile::new(path.to_string_lossy(), file_content);
if let Some(fixer) = &rule.fixer {
let diffs = matches.map(|m| Diff::generate(m, &rule.matcher, fixer));
reporter.print_rule_diffs(diffs, path, rule)?;
let diffs = matches
.map(|m| (Diff::generate(m, &rule.matcher, fixer), rule))
.collect();
reporter.print_rule_diffs(diffs, path)?;
} else {
reporter.print_rule(matches, file, rule)?;
}
Expand Down

0 comments on commit 2b30111

Please sign in to comment.