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

Formatter: Add an option to show formatting changes when running --check #7231

Closed
MichaReiser opened this issue Sep 8, 2023 · 1 comment · Fixed by #7937
Closed

Formatter: Add an option to show formatting changes when running --check #7231

MichaReiser opened this issue Sep 8, 2023 · 1 comment · Fixed by #7937
Assignees
Labels
cli Related to the command-line interface formatter Related to the formatter

Comments

@MichaReiser
Copy link
Member

MichaReiser commented Sep 8, 2023

There are two options:

  1. Add a new --diff option to ruff format --check that prints the formatting changes similar to Black
  2. --check uses Ruff's diagnostic system and shows diffs for format: human but not for format: compact (which would enable diffs by default)

I prefer option 2.

Open questions

  • Should this be integrated into our diagnostic system?
  • Should it print the entire diff or collapse very large code frames
  • Which diffing library to use. We ran into exponential back-tracking cases in our cargo dev script for large files / large changes

CC @zanieb regarding CLI design

@MichaReiser MichaReiser added cli Related to the command-line interface formatter Related to the formatter labels Sep 8, 2023
@MichaReiser MichaReiser added this to the Formatter: Beta milestone Sep 8, 2023
@MichaReiser MichaReiser changed the title Formatter: Add --diff option to CLI Formatter: Add --diff option to CLI or respect format configuration Sep 8, 2023
@MichaReiser MichaReiser changed the title Formatter: Add --diff option to CLI or respect format configuration Formatter: Add an option to show formatting changes when running --check Sep 8, 2023
@charliermarsh
Copy link
Member

I believe we've settled on ruff format --diff, which will have the same semantics as ruff check --diff (i.e., it exits with status code 1 if there are any diffs).

@charliermarsh charliermarsh added the help wanted Contributions especially welcome label Sep 22, 2023
@konstin konstin removed the help wanted Contributions especially welcome label Oct 13, 2023
konstin added a commit that referenced this issue Oct 18, 2023
**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.

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

---------

Co-authored-by: Micha Reiser <micha@reiser.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to the command-line interface formatter Related to the formatter
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants