Add --diff option for outputting diffs of formatting changes.#12598
Add --diff option for outputting diffs of formatting changes.#12598Osmose wants to merge 4 commits intoprettier:mainfrom
Conversation
|
What's your opinion on #11833 |
If there was, like, a |
|
Just checking in, anything I can do to help move this along? |
|
@fisker Hiiii, checking in again, anything besides fixing the merge conflicts I can do to help move this along? Thanks! |
| runPrettier("cli/with-shebang", ["--diff", "--parser", "babel"], { | ||
| input: "0", | ||
| }).test({ | ||
| stdout: outdent({ trimTrailingNewline: false })` |
There was a problem hiding this comment.
Let's not assert this, but make a snapshot. (Remove stdout)
| "cli/write", | ||
| ["--diff", "formatted.js", "unformatted.js"], | ||
| { | ||
| stdoutIsTTY: true, |
| [context.argv.check, context.argv.listDifferent, context.argv.diff].filter( | ||
| (o) => o | ||
| ).length > 1 |
There was a problem hiding this comment.
| [context.argv.check, context.argv.listDifferent, context.argv.diff].filter( | |
| (o) => o | |
| ).length > 1 | |
| [context.argv.check, context.argv.listDifferent, context.argv.diff].filter( | |
| Boolean | |
| ).length > 1 |
| test("Should be the same", async () => { | ||
| expect(await result0.stdout).toEqual(await result1.stdout); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Can you add a test for stdin format/check?
| <!-- | ||
|
|
||
| 1. Choose a folder based on which language your PR is for. | ||
|
|
||
| - For JavaScript, choose `javascript/` etc. | ||
| - For TypeScript specific syntax, choose `typescript/`. | ||
| - If your PR applies to multiple languages, such as TypeScript/Flow, choose one folder and mention which languages it applies to. | ||
|
|
||
| 2. In your chosen folder, create a file with your PR number: `XXXX.md`. For example: `typescript/6728.md`. | ||
|
|
||
| 3. Copy the content below and paste it in your new file. | ||
|
|
||
| 4. Fill in a title, the PR number and your user name. | ||
|
|
||
| 5. Optionally write a description. Many times it’s enough with just sample code. | ||
|
|
||
| 6. Change ```jsx to your language. For example, ```yaml. | ||
|
|
||
| 7. Change the `// Input` and `// Prettier` comments to the comment syntax of your language. For example, `# Input`. | ||
|
|
||
| 8. Choose some nice input example code. Paste it along with the output before and after your PR. | ||
|
|
||
| --> | ||
|
|
||
| #### Add --diff option for outputting diffs of formatting changes. (#12598 by @Osmose) |
There was a problem hiding this comment.
| <!-- | |
| 1. Choose a folder based on which language your PR is for. | |
| - For JavaScript, choose `javascript/` etc. | |
| - For TypeScript specific syntax, choose `typescript/`. | |
| - If your PR applies to multiple languages, such as TypeScript/Flow, choose one folder and mention which languages it applies to. | |
| 2. In your chosen folder, create a file with your PR number: `XXXX.md`. For example: `typescript/6728.md`. | |
| 3. Copy the content below and paste it in your new file. | |
| 4. Fill in a title, the PR number and your user name. | |
| 5. Optionally write a description. Many times it’s enough with just sample code. | |
| 6. Change ```jsx to your language. For example, ```yaml. | |
| 7. Change the `// Input` and `// Prettier` comments to the comment syntax of your language. For example, `# Input`. | |
| 8. Choose some nice input example code. Paste it along with the output before and after your PR. | |
| --> | |
| #### Add --diff option for outputting diffs of formatting changes. (#12598 by @Osmose) | |
| #### Add --diff option for outputting diffs of formatting changes (#12598 by @Osmose) |
|
|
||
| You can also use [`--check`](cli.md#--check) flag, which works the same way as `--list-different`, but also prints a human-friendly summary message to stdout. | ||
|
|
||
| ## `--diff` |
There was a problem hiding this comment.
I prefer --diff only work when doing --check
prettier --check --diff .WDYT?
There was a problem hiding this comment.
My only thought against it would be that if anyone wanted to pipe the output to a diff file that could be applied later by a diff tool, requiring --check would mean that they have to strip the summary (unless you mean adding --diff would implicitly remove the summary).
But I don't really feel strongly about it and am fine either way.
There was a problem hiding this comment.
My only thought against it would be that if anyone wanted to pipe the output to a diff file that could be applied later by a diff tool
Make sense.
There was a problem hiding this comment.
How about:
For files
prettier . --diffFormat and show diff
prettier . --write --diffNot allow.
prettier . --check --diffFormat and show diff
For stdin
echo "code" | prettier --diffFormat and show diff
echo "code" | prettier --write --diffNot allow.
echo "code" | prettier --check --diffFormat and show diff.
So --diff always do the same as --check, but show different message, and it can use together with --check, but not --write.
|
Updated based on feedback:
@fisker Thanks for the review, lemme know if anything else needs fixing! |
|
Besides the conflicts, is there anything holding this back? We'd love to use this |
|
@fisker can we do anything to have this issue prioritized, or at least reviewed? |
|
Any progress on this? |
| ).length > 1 | ||
| ) { | ||
| throw new Error( | ||
| "Cannot use --check, --list-different, or --diff together." |
There was a problem hiding this comment.
Seems like this would be sufficient:
if ((context.argv.check || context.argv.diff) && context.argv.listDifferent) {
throw new error("Cannot use --list-different with --check or --diff");
}
|
@oalders is this PR still being worked on? |
|
@LoganDark I have no idea. I'm interested in seeing this get done, but I haven't been working on it. Maybe you meant to ping someone else? |
oops, meant to ping the author. sorry (I'm sure they've already been notified by now of course) |
This is waiting on having someone with merge powers to review it, I'll always be ready to rebase it once it's clear that it will be reviewed again. |
It looks like there's already an unaddressed review comment? #12598 (comment) |
|
gregmarr isn't part of the Prettier org and wouldn't be able to merge the PR. |
|
@fisker what can we do to move this forward? |
|
The best solution I found so far is the one I commented in #6885 (comment). |
I have that in mind for sure, but I also want this feature for my local environment. |
|
I tried out this branch a little bit. Here are some things I noticed:
It probably makes sense to test |
|
Sorry, I missed the message last year, thanks for the feedback. However, I don't really have interest in following up with that much feedback after two and a half years of little meaningful engagement from maintainers, so I'm gonna close this and someone else can work off of that if they want to. |
|
I made a tiny tool that runs prettier with If a maintainer is interested, I can do a PR to implement a |


Description
Adds a new
--diffflag that is similar to--checkand--list-different, but instead of listing filenames, it outputs unified diffs with the changes Prettier would have made to format the file.This was first requested in #6885 and there isn't really a firm answer one way or another from a maintainer yet, but I figure having a PR to look at could help move the discussion forward.
Thank you!
Checklist
--checkand--list-differentare tested so any pointers to examples to mimick would be welcome.docs/directory).changelog_unreleased/*/XXXX.mdfile followingchangelog_unreleased/TEMPLATE.md.✨Try the playground for this PR✨