Conversation
PR Check ResultsEcosystem✅ ecosystem check detected no changes. |
| Formatted { | ||
| unformatted: SourceKind, | ||
| formatted: SourceKind, | ||
| }, |
There was a problem hiding this comment.
This is making me wonder if format_source should take &SourceKind, and we should change Unchanged(SourceKind) to just Unchanged, and remove unformatted from the Formatted variant. It strikes me as inflexible that the method takes ownership of SourceKind but only acts on references and then returns the owned SourceKind in all cases. But, this is minor -- if it doesn't work for whatever reason, or you disagree, it's fine.
There was a problem hiding this comment.
I guess the challenge with this is that we need to move SourceKind out of the linter, maybe into ruff_source_file? I recommend investigating this as a separate PR.
There was a problem hiding this comment.
Thought so too but didn't want to create so much churn, changed it now
I think it's fine to leave this as-is for now -- I will fold into the documentation when I revisit it next week. |
| Formatted { | ||
| unformatted: SourceKind, | ||
| formatted: SourceKind, | ||
| }, |
There was a problem hiding this comment.
I guess the challenge with this is that we need to move SourceKind out of the linter, maybe into ruff_source_file? I recommend investigating this as a separate PR.
| let mut writer = stdout().lock(); | ||
| let text_diff = TextDiff::from_lines( | ||
| unformatted.source_code(), | ||
| formatted_source.source_code(), | ||
| ); | ||
| let mut unified_diff = text_diff.unified_diff(); | ||
| unified_diff.header(&fs::relativize_path(path), &fs::relativize_path(path)); | ||
| unified_diff.to_writer(&mut writer).unwrap(); | ||
|
|
||
| formatted += 1; | ||
| } |
There was a problem hiding this comment.
Nit: It feels a bit strange that the diff is written as part of the FormatSummary's Display implementation (the diff isn't part of the summary).
I would prefer moving the diff printing out of the summary implementation and handle it explicitly when the FormatMode is Diff (also allows to locking stdout exactly once)
There was a problem hiding this comment.
I went with renaming FormatSummary to FormatResults and giving it two different write methods. We unfortunately lose the fmt impl but we get proper separation
Did work without it, but i'm fine with moving it anyway |
**Summary** `ruff format --diff` is similar to `ruff format --check`, but we don't only error with the list of file that would be formatted, but also show a diff between the unformatted input and the formatted output. ```console $ ruff format --diff scratch.py scratch.pyi scratch.ipynb warning: `ruff format` is not yet stable, and subject to change in future versions. --- scratch.ipynb +++ scratch.ipynb @@ -1,3 +1,4 @@ import numpy -maths = (numpy.arange(100)**2).sum() -stats= numpy.asarray([1,2,3,4]).median() + +maths = (numpy.arange(100) ** 2).sum() +stats = numpy.asarray([1, 2, 3, 4]).median() --- scratch.py +++ scratch.py @@ -1,3 +1,3 @@ x = 1 -y=2 +y = 2 z = 3 2 files would be reformatted, 1 file left unchanged ``` With `--diff`, the summary message gets printed to stderr to allow e.g. `ruff format --diff . > format.patch`. At the moment, jupyter notebooks are formatted as code diffs, while everything else is a real diff that could be applied. This means that the diffs containing jupyter notebooks are not real diffs and can't be applied. We could change this to json diffs, but they are hard to read. We could also split the diff option into a human diff option, where we deviate from the machine readable diff constraints, and a proper machine readable, appliable diff output that you can pipe into other tools. **Test plan** Fixtures for the change and no change cases, including a jupyter notebook and for file input and stdin.
Co-authored-by: Micha Reiser <micha@reiser.io>
Co-authored-by: Micha Reiser <micha@reiser.io>
Summary
ruff format --diffis similar toruff format --check, but we don't only error with the list of file that would be formatted, but also show a diff between the unformatted input and the formatted output.With
--diff, the summary message gets printed to stderr to allow e.g.ruff format --diff . > format.patch.At the moment, jupyter notebooks are formatted as code diffs, while everything else is a real diff that could be applied. This means that the diffs containing jupyter notebooks are not real diffs and can't be applied. We could change this to json diffs, but they are hard to read. We could also split the diff option into a human diff option, where we deviate from the machine readable diff constraints, and a proper machine readable, appliable diff output that you can pipe into other tools.
To make the tests work, the results (and errors, if any) are sorted before printing them. Previously, the print order was random, i.e. two identical runs could have different output.
Open question: Should this go into the markdown docs? Or will this be subsumed by the integration of the formatter into
ruff check?Test plan Fixtures for the change and no change cases, including a jupyter notebook and for file input and stdin.
Fixes #7231