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.

Sign up for GitHub

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

Jump to bottom

Show per-cell diffs when analyzing notebooks over stdin #7789

Merged
merged 1 commit into from
Oct 4, 2023
Merged

Show per-cell diffs when analyzing notebooks over stdin #7789

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
84 changes: 71 additions & 13 deletions crates/ruff_cli/src/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,20 +428,78 @@ pub(crate) fn lint_stdin(
flags::FixMode::Diff => {
// But only write a diff if it's non-empty.
if !fixed.is_empty() {
let text_diff = TextDiff::from_lines(
source_kind.source_code(),
transformed.source_code(),
);
let mut unified_diff = text_diff.unified_diff();
if let Some(path) = path {
unified_diff
.header(&fs::relativize_path(path), &fs::relativize_path(path));
}
match transformed.as_ref() {
SourceKind::Python(_) => {
Comment on lines +431 to +432
Copy link
Member

Choose a reason for hiding this comment

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

nit: This is repeated for stdout as well but I think the only difference is that the path is optional. Could we combine them under SourceKind in the same way as done in FixMode::Apply? That would avoid code duplication.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll take a look at this in a follow-up as its leading me to want to make some other changes.

let text_diff = TextDiff::from_lines(
source_kind.source_code(),
transformed.source_code(),
);
let mut unified_diff = text_diff.unified_diff();
if let Some(path) = path {
unified_diff.header(
&fs::relativize_path(path),
&fs::relativize_path(path),
);
}

let mut stdout = io::stdout().lock();
unified_diff.to_writer(&mut stdout)?;
stdout.write_all(b"\n")?;
stdout.flush()?;
}
SourceKind::IpyNotebook(dst_notebook) => {
let src_notebook = source_kind.as_ipy_notebook().unwrap();
let mut stdout = io::stdout().lock();
// Cell indices are 1-based.
for ((idx, src_cell), dest_cell) in (1u32..)
.zip(src_notebook.cells().iter())
.zip(dst_notebook.cells().iter())
{
let (Cell::Code(src_code_cell), Cell::Code(dest_code_cell)) =
(src_cell, dest_cell)
else {
continue;
};

let src_source_code = src_code_cell.source.to_string();
let dest_source_code = dest_code_cell.source.to_string();
let text_diff =
TextDiff::from_lines(&src_source_code, &dest_source_code);
let mut unified_diff = text_diff.unified_diff();

// Jupyter notebook cells don't necessarily have a newline
// at the end. For example,
//
// ```python
// print("hello")
// ```
//
// For a cell containing the above code, there'll only be one line,
// and it won't have a newline at the end. If it did, there'd be
// two lines, and the second line would be empty:
//
// ```python
// print("hello")
//
// ```
unified_diff.missing_newline_hint(false);

if let Some(path) = path {
unified_diff.header(
&format!("{}:cell {}", &fs::relativize_path(path), idx),
&format!("{}:cell {}", &fs::relativize_path(path), idx),
);
} else {
unified_diff
.header(&format!("cell {idx}"), &format!("cell {idx}"));
};

let mut stdout = io::stdout().lock();
unified_diff.to_writer(&mut stdout)?;
stdout.write_all(b"\n")?;
stdout.flush()?;
unified_diff.to_writer(&mut stdout)?;
}
stdout.write_all(b"\n")?;
stdout.flush()?;
}
}
}
}
flags::FixMode::Generate => {}
Expand Down
Loading