-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Show per-cell diffs when analyzing notebooks over stdin
#7789
Conversation
CodSpeed Performance ReportMerging #7789 will improve performances by 2.58%Comparing Summary
Benchmarks breakdown
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
match transformed.as_ref() { | ||
SourceKind::Python(_) => { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
crates/ruff_cli/src/diagnostics.rs
Outdated
unified_diff.header( | ||
&format!("cell {}", idx), | ||
&format!("cell {}", idx), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think a default value of -
(or stdin
/ STDIN
) would be good to show as the filename for stdin? I think we do that in other places. So, the output would something like:
-:cell 0:
...
-:cell 2:
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, we don't do this in the Python case, so I'll leave it for now.
1d1d3fe
to
f35e4f1
Compare
PR Check ResultsEcosystem✅ ecosystem check detected no changes. |
Summary
The implementation here differs from the non-
stdin
version -- this is now more consistent.Test Plan