Add diff in output of --check (for CI use cases)

[muescha]

More people submit PR for documents in Markdown files or other things via Github web editor. If they make a mistake then the CI fails but there is no chance to inform the user whats wrong.

I see some infos should be with the --debug-check but currently there is no output with this option.

Environments:

• Prettier Version: 1.18.2
• Running Prettier via: cli
• Runtime: node?
• Operating System: linux

Steps to reproduce:

prettier "**/*.{md,css,scss,yaml,yml,ts}" "--check"

Expected behavior:

Show each error and the changes like with --debug-check here in #4415

Actual behavior:

Show nothing about the errors:

> @ prettier /home/circleci/project
> prettier "**/*.{md,css,scss,yaml,yml,ts}" "--check"

Checking formatting...
docs/contributing/environment.md
Code style issues found in the above file(s). Forgot to run Prettier?

Also there are no more infos available with this commands:

prettier "**/*.{md,css,scss,yaml,yml,ts}" "--debug-check"
prettier "**/*.{md,css,scss,yaml,yml,ts}" "--debug-check" "--check"

[dylannz-sailthru]

"Forgot to run Prettier?" is really not a very helpful message...

[thorn0]

The message contains the file names. What else do you need to run Prettier on those files? ¯\_(ツ)_/¯

[lydell]

I personally like the output of --check very much.

CI: "Forgot to run Prettier?"

Me: "Oh yeah, I guess I did!" Runs Prettier.

CI: "All good!"

[muescha]

I like to point again to:

more people submit PR for documents in Markdown files or other things via Github web editor.

That means they have no local IDE installed. That's why they can not run prettier on the file.

[muescha]

Expected behavior would be a detailed error message or a diff

[error] src/index.js: ast(input) !== ast(prettier(input))
[error] Index:
[error] ===================================================================
[error] ---
[error] +++
[error] @@ -17,6 +17,6 @@
[error]                    "method": false,
[error]                    "key": {
[error] -                    "type": "StringLiteral",
[error] -                    "value": "asd"
[error] +                    "type": "Identifier",
[error] +                    "name": "asd"
[error]                    },
[error]                    "computed": false,
[error]
[error] Index:
[error] ===================================================================
[error] ---
[error] +++
[error] @@ -1,1 +1,1 @@
[error] -const test = { "asd": 1 };
[error] +const test = { asd: 1 };
[error]

[alexander-akait]

Why not run prettier \"{**/*,*}.{js,json,md,yml,css}\" --list-different" using npm/yarn script?

[muescha]

thx. I would try on monday the output of --list-different

[lydell]

--list-different also just lists file names – I don’t think it’ll make any difference for @muescha.

@muescha I think this proposal might be solving your problem the wrong way. Forcing people to click into a CI build log, find diffs of expected output and then manually applying them does no sound very friendly. I think we could try several other things:

• Encourage people to clone the project rather than using the web editor.
• Use --prose-wrap preserve or --prose-wrap never to reduce chance of needing to run Prettier.
• Running Prettier in the web editor via https://github.com/prettier/prettier-chrome-extension.
• A bot that automatically runs Prettier on PRs.
• You could pull people’s branches, run Prettier yourself and push to their PR (contributions from maintainers is generally enabled).
• Not use Prettier for markdown files if you get a lot of contributions to them via the web editor.

[muescha]

• Encourage people to clone the project rather than using the web editor.

thats much overhead :(

• Use --prose-wrap preserve or --prose-wrap never to reduce chance of needing to run Prettier.

it should run normal prettier

• Running Prettier in the web editor via prettier/prettier-chrome-extension.

this is only the solution for one user, and not for all

• A bot that automatically runs Prettier on PRs.

this changes the PR - the user should have information about the problems

• You could pull people’s branches, run Prettier yourself and push to their PR (contributions from maintainers is generally enabled).

to much overhead to checkout a branch etc

• Not use Prettier for markdown files if you get a lot of contributions to them via the web editor.

also prettier is a good help

the only thing is that there should be an more verbose error message :/

[lydell]

Thanks for the responses!

You didn’t seem to respond to this one, though:

Forcing people to click into a CI build log, find diffs of expected output and then manually applying them does no sound very friendly.

Thoughts?

[muescha]

since they edit the PR on github and check why the CI build is failing they already looking into the CI logs. and then when they only find a not helpful message - that is frustrating.

example https://github.com/gatsbyjs/gatsby/pull/18719#discussion_r336781550

I checked what you suggested and lines where only word to highlight is "false" (without quotes or 0 are highlighted with no issues.

PS. @muescha is there a way to get rid of prettier complains in build errors? I am editing this in the browser.

[dylannz-sailthru]

I was making changes to a repository that'd had prettier added to it without me realising, but I had not installed it in my local development environment. So naturally it blew up in CI and asked if I'd "forgotten to run prettier". But that message came from a program called "prettier", which was pretty confusing - yes I suppose I'd "forgotten" to run prettier (I hadn't actually, I didn't even know I had to. Unnecessarily snarky error message, if you ask me...) but even more annoying was that the prettier program was printing that message!

exit 1 would have been just as helpful, and less condescending. A list of formatting violations would have been a lot better though!

[thorn0]

@dylannz-sailthru Would you really prefer to get a diff instead like @muescha has proposed and apply it manually instead of running the formatter?

[dylannz-sailthru]

@dylannz-sailthru Would you really prefer to get a diff instead like @muescha has proposed and apply it manually instead of running the formatter?

I admit, I've now set up my IDE with prettier and now it doesn't bother me. But it seems there is a use case where someone wants to make a small change and doesn't have prettier installed (either on GitHub or in a local development environment), adds an extra space somewhere, and now has to install a 3rd party program and run it instead of just fixing a small error. And while I'll admit it's a narrow use case, I can't see much of a downside to being more verbose (or at least having a configurable verbose flag). Most CI tools will collapse each CI step so you can view the logs of each one in isolation.

[muescha]

example where i used the GitHub editor and i get for this change https://github.com/gatsbyjs/gatsby/commit/7b78781d847b32c7aed358a60489046c9e7b8d3b in https://github.com/gatsbyjs/gatsby/pull/19822 this CI log message:

https://circleci.com/gh/gatsbyjs/gatsby/283065?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

now i have to checkout ~500MB data from pull request, install prettier only just to see whats wrong in this commit and i can change the file?

[thorn0]

@dylannz-sailthru Locally, extra setup isn't needed. Prettier is installed as a dev dependency and there is usually a command in the project to run all the linters, formatters, etc. (e.g. npm run format). This means we're discussing only the web UI use case here.

[lydell]

Unnecessarily snarky error message, if you ask me...

Sorry, the message is actually not supposed to be snarky, but friendly! Do you have ideas for better wording?

example gatsbyjs/gatsby#18719 (comment)

I don’t think that’s a good example. That’s not a markdown change, but a code change! Is that really something people do in the browser? That sounds terrible to me – how do you debug and test then?

---

Here’s yet one idea: When editing a markdown file in the GitHub editor and are about to hit the Commit button – first copy-paste the entire file into https://prettier.io/playground/ and then copy back the output.

[muescha]

I don’t think that’s a good example. That’s not a markdown change, but a code change! Is that really something people do in the browser?

yes - people do it. so why not also on all files.

Here’s yet one idea

that maybe a workaround but not a solution and also not fit with the project settings for prettier

[dylannz-sailthru]

Sorry, the message is actually not supposed to be snarky, but friendly! Do you have ideas for better wording?

Sure! Something like: You need to run `prettier fix` on these files to ensure they are formatted correctly.

This way it's telling me more specifically what I need to do to get past the error. I think it doesn't matter so much for people who have used prettier before, but as someone who hadn't used it before I found it a little confusing.

Here’s yet one idea: When editing a markdown file in the GitHub editor and are about to hit the Commit button – first copy-paste the entire file into https://prettier.io/playground/ and then copy back the output.

Actually that'd be a great thing to link to in the error message as well - I'd have found it really useful at the time :)

[asfaltboy]

How come html file diffs are outputted, but not JS/CSS files? Is it a verbosity concern? If so, perhaps we should be able to toggle via --log-level (i.e set the default to warn and don't display diff on warn)?

Correction: diff is only shown when an error is raised in the tool, e.g SyntaxError: Unexpected closing tag "tr".

[grncdr]

I would also like to see a diff or more detailed information about why prettier thinks a file isn't formatted. I currently don't run prettier --check in CI/pre-commit hooks because it tells me that files I've just formatted with Prettier are still not formatted. I really don't know how to begin fixing the problem when I can't see what it is.

[lydell]

@grncdr It tells you which files aren’t formatted, so all you need to do is run Prettier on those files. Or on your whole project – probably prettier --write .. Can you expand on how seeing a diff helps?

[grncdr]

As I said in my first message. I have already run prettier on those files, but it still shows the warning. Trying to make a more useful error report is impossible when I can't see what formatting prettier is complaining about.

[lydell]

1. Check that you are using the same Prettier version locally and in CI.
2. Check that you are using the same config locally and CI.
3. There’s probably no step 3.

[grncdr]

so here's what I just did, all locally, no CI involved.

$ yarn prettier --check some-file.ts
yarn run v1.22.4
$ prettier --check some-file.ts
Checking formatting...
[warn] path/to/some/file.ts
[warn] Code style issues found in the above file(s). Forgot to run Prettier?
error Command failed with exit code 1.
$ yarn prettier --write some-file.ts
yarn run v1.22.4
$ prettier --write some-file.ts
some-file.ts 188ms
✨  Done in 0.48s.
$ git diff
<actual diff output>

It turns the diff was caused by another tool formatting & sorting imports, and it disagrees with Prettier about how many imports belong on a single line. This was trivial to see once I had a diff but difficult to understand without it. The experience in VSCode is that format-on-save is using Prettier and everything looks fine, so there's no indication about the other tool messing with Prettiers formatting.

Having an option for more verbose output would have made the problem clear immediately.

[thorn0]

@guyzmo

I believe it would be nice to have a way ... for debugging purposes

But you have that way, don't you? I mean the combination of prettier --write and git diff.

[thernstig]

I am adding my two cents here, as my duplicate was closed: https://github.com/prettier/prettier/issues/10294

When I have white-space issues in a JavaScript file I get the following error when running via CLI:

> prettier --check "**/*.{js}"
Checking formatting...
[warn] api/cow/moo.js
[warn] Code style issues found in the above file(s). Forgot to run Prettier?

If I run Prettier via ESlint (using eslint-plugin-prettier) I instead get the following output:

> eslint --max-warnings=0 .

/home/user/code/project/api/cow/moo.js
  9:1  error  Delete `⏎··⏎`  prettier/prettier

✖ 1 problem (1 error, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

I find the ESLint output much better than that of Prettier. Hence why I have chosen to run Prettier via ESLint instead. This is unfortunate, as https://prettier.io/docs/en/integrating-with-linters.html does not recommend to use Prettier as plugin to ESlint.

The actual behavior is subpar to what I would expect. I want to know the file, the line:column and also exactly what was wrong.

I'd also add that Prettier is very concerned about formatting/style/"beauty" - yet the CLI output used in e.g. CI is nothing of those. I feel this is important to raise the quality-feel of Prettier.

[unekinn]

I'll just +1 the usefulness of a diff for CI.

I'm trying to debug an issue where prettier --check lists ALL files as incorrectly formatted when running in CI, while running prettier --check locally -- yes, with the same .prettierrc -- shows "All matched files use Prettier code style!"

I'm guessing it's a line ending issue, but it's very hard to tell what's going on when there's no diff in the output. I'm basically just trying various changes to Git's line ending handling and seeing if they make the CI run successfully (no luck so far).

[thibaudcolas]

This may be useful to some – you can work around Prettier not providing this by using your git (or other SCM) diffing tools. Here is an example using git diff:

prettier --write '**/?(.)*.{md,css,scss,js,json,yaml,yml}' && git --no-pager diff && git checkout -- .
prettier --check '**/?(.)*.{md,css,scss,js,json,yaml,yml}'

The first command applies formatting changes, then shows the diff, then discards all the changes. The second command checks the formatting as you normally would.

For people who want the best possible build performance, I’m sure it would also be relatively straightforward to only run Prettier the one time with --write, and then break the build if there are local changes in git. Personally I prefer to still preserve the output of --check.

---

If Prettier does end up with supporting this, it’d be great to do it in a way that can be integrated with GitHub Actions’ annotations (e.g. for ESLint).

[JoshuaKGoldberg]

I just came across this with a project with weirdly formatted endlines across Windows vs. *Nix. CI failed on a few files that passed Prettier locally. This would be useful.

Additionally, +1 to the comments mentioned about folks who don't have Prettier set up locally.

@thorn0 what part of this issue would the maintainer group consider to still be in discussion? I'd be keen on implementing this.

[trim21]

you can use reviewdog

name: Lint Other Language

on:
  push:
    branches:
      - master
    paths:
      - "**.md"
      - "**.yaml"
      - "**.yml"
      - "**.html"
      - "**.json"
  pull_request_target:
    types: [opened, synchronize, reopened]
    branches:
      - master
    paths:
      - "**.md"
      - "**.yaml"
      - "**.yml"
      - "**.html"
      - "**.json"

jobs:
  lint-other:
    runs-on: ubuntu-latest
    steps:
      - name: Install Node LTS
        uses: actions/setup-node@v2
        with:
          node-version: 14.x.x

      - run: npm i -g prettier

      - uses: reviewdog/action-setup@v1
        with:
          reviewdog_version: latest # Optional. [latest,nightly,v.X.Y.Z]

      - name: Checkout code
        uses: actions/checkout@v2
        with:
          ref: "refs/pull/${{ github.event.number }}/merge"

      - name: prettier
        run: prettier --write --list-different ./

      - run: git diff --exit-code

      - name: create review
        if: ${{ failure() }}
        run: |
          TMPFILE=$(mktemp)
          git diff >"${TMPFILE}"
          git reset --hard HEAD
          reviewdog -f=diff -f.diff.strip=1 -reporter=github-pr-review < "${TMPFILE}"
          exit 1
        env:
          REVIEWDOG_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }}

[theahura]

Adding my voice to the chorus: my team has been stuck for a few days now because prettier --check is failing on Github Actions for files that we explicitly run prettier --write on. Locally prettier says there are no files that need changing; on Github Actions, prettier throws errors.

A debug would be really important to figure out why.

[lydell]

@theahura How would a diff help you solve that problem (local and GitHub Actions disagree)? And what do you mean by “prettier throws errors”?

[theahura]

By 'prettier throws errors' I mean that prettier --check indicates that there are problems.

A diff would provide a starting point to find patterns for changes. For example, I used the git diff example above and noticed that the changes were all due to spacing. I was then able to Google search to find that there was a prettier version change that was causing issues. Without the git diff, I basically had no launching off point to figure out what the issue was. I might have eventually landed on the version change, but only after checking a lot of other things.

Prettier is great, and I understand the desire to keep the UI simple. But I imagine you're already calculating what changes need to be made in order to check and write them; is it hard to dump those to stdout, even in a messy format?

[lydell]

I was then able to Google search to find that there was a prettier version change that was causing issues.

Checking the version of Prettier locally and CI would have been the first thing I had checked. That’s usually the cause for all types of CI issues I encounter (Prettier related or not) 😅

[theahura]

Maybe the output of the prettier check should include a note that says 'if you're still seeing errors, make sure your version is correct'?

But either way I think the goal should ideally be to make people self sufficient so they don't have to ask you. One way to do that is to hide power user options behind flags like --debug or --verbose or whatever. In my case I went to check versions only after exhausting a bunch of other possibilities (I spent a long time fiddling with the prettierrc, for eg). The diff helped me realize that it wasn't the rc.

[webark]

An example of how I found myself on this issue, but in the end, it wasn't what I was after.

I came in new to a project, and it was giving me an annoying little red squiggle when I was writing something a certain way. I was like, "Ohh, someone has some annoying rule is on.. Uhhgg. People. 🙄" (sorry.. I'm opinionated about my code style (something you authors should appreciate 😉 )). So I went to see what the rule was, cause naturally, I wanted to disable it, and all I got was "prettier/prettier". After another (opinionated) eye roll, I was like "ohh I'll just run the command and it will output the rule that I can go disable!"

So I ran the command and got the "friendly" Forgot to run Prettier? output. (Now a quick aside, assuming that someone "forgot" to do something can be offputting if you purposefully did not do what they are accusing you of "forgetting". No, I did not "forget" to run prettier, I "chose" not to run prettier.) Assuming I was running the "light" output, I went to look to see all the different options and discovered, no, this is the most detailed output.

I then thought to myself, "Ohh, maybe I can just look over the options and find which one!". After skimming over the options, being surprised that was all there was, assuming the documentation was incomplete I started googling around. And that's when, after finding my way back to the config page, saw the "Option Philosophy" link, and then I read the dreaded

But we won’t add more of them.

This is when I realized that ohh, I'm just going to have to live with this (again in my opinion) substandard syntax. 😞 🤡

Sorry, the message is actually not supposed to be snarky, but friendly! Do you have ideas for better wording?

@lydell I opened up PR #11369 with an updated message. I wanted to keep it the same succinctness, just being a little blander and informative, and a little less familiar so it can't be "misinterpreted" as easily. Any additional information (such as a doc link or an example command) seemed like a separate initiative that could happen regardless if this change was made.

Thanks for the work, I know OSS isn't a glamour gig. 😊

[worc]

the diff is also important when you're verifying that prettier isn't causing breaking changes or just plain bad legibility. it's also important to discovering all the unwritten rules. i'm not here to blindly apply "fixes", i want to know what changes prettier is proposing so i can incorporate that into my work on repos that use prettier in the future.

a good example is the strong opinion prettier has on css/sass grid styles: https://github.com/prettier/prettier/issues/2703

i'd rather be shown what prettier wants to do and decide if that's the right course of action, instead of the condescending Forgot to run Prettier?

[liamjsc]

Is it possible for the CLI output to list the prettier config rules that are broken? I'm setting up my prettier config and this snippet changes from

get: param => ({ anotherParam }) => {
    const foo = bar;

to this:

  get:
    param =>
    ({ anotherParam }) => {
      const foo = bar;

I much prefer the single-line params, and I'd like to know which rule I can set to disable splitting this into multiple lines, but the only output I get from prettier-check is "Code style issues found in the above file(s). Forgot to run Prettier?"

[lydell]

@liamjsc You can:

1. Commit all your code unformatted.
2. Run Prettier with --write.
3. Check the (git) diff.
4. Discard all changes.
5. Make changes to the config.
6. Repeat from 2.

[liamjsc]

Thanks for the reply @lydell , but what changes to the config are you suggesting? Maybe my question was not clear enough. --write is not telling me which prettier rules are broken.

For example, if prettier was changing my code from: const foo = bar to const foo = bar;, how can I discover that the rule being applied is semi: true?

[lydell]

I’m not suggesting any config changes, I just tried to help with a workflow where you can see the diff of what Prettier does.

There aren’t that many options – going through https://prettier.io/docs/en/options.html should make it fairly obvious what each might do. The playground can help, too, with playing around with different options.

[Israel-Laguan]

I had an issue similar to this one.

So I added a file .gitattributes like this:

*.ts text eol=lf

With this the problem was solved, but I think prettier should have a more descriptive message

[worc]

Another use-case is when doing a rebase and re-linting / re-formatting a conflict resolution. I'd rather see what needs changing instead of blindly applying fixes in the midst of a rebase.

[Ragnoroct]

I've solved this using bash. Doing a diff directly in prettier sounds pretty complicated.

prettier_diff.sh You can use this in CI or a pre-commit hook https://gist.github.com/Ragnoroct/943bda9ed238c2919e751393ffcb2a72

one-liners

# gnu diff
file=test_file.js; diff --color -u "$file" <(cat "$file" | npx prettier --stdin-filepath "$file")

# git diff, sed makes patch applyable to correct file
file="test_file.js" cat "$file" | npx prettier --stdin-filepath "$file" | git --no-pager diff --color --no-index -- "$file" - | sed "s/b\/-/b\/$file/g"

similar to https://github.com/prettier/prettier/issues/6885#issuecomment-810136478 but uses stdin instead of writing to files directly

[DanielHabenicht]

To add my opinion on the "find the underlying problem" debug workflow:

1. Check that you are using the same Prettier version locally and in CI.
2. Check that you are using the same config locally and CI.
3. There’s probably no step 3.

Not all issues are solved by this, especially nasty ones like, invisible chars (however they ended up in the code) or more common line endings. The already given workaround with prettier --write && git diff does not help in this regard either.

Entering my short story: When debugging our CI failures we were confronted with --check failures and tried reproducing them. But the failure was only reproducible on the CI machine. One coworker got the error once, then run the above mentioned commands, but the git diff was empty. He tried again, but could not reproduce anymore.

We run prettier --write in CI - Works, but not what we want. So we added the git diff to our CI workflow. - Empty! We had a closer look at the files. cat - Same output before and after format. Finally cat -A, shows the line ending, and voila for two of the hundreds of files git decided that they need CRLF We already used .gitattributes to consolidate the line ending with * text=auto eol=LF, but apparently git can classify files as non text and check them out with another line ending.

So in the end the issue was only partially on prettiers side (EOL Option). But formatting has much more moving parts, e.g. (git) line endings, which we fixed for good now. A great tool should make mistakes of other moving parts visible to shift the blame from it.

Morale of the story:

With a simple diff option in prettier (or as others mentioned, at least outputting the violated rule) it would have cut my debug time dramatically. I am strongly in favour of this feature. As others would have simply dumped the formatting CI rule and moved on.

In the end time I save by using a tool is counterweighted by time I need invest to use a tool. This feature would reduce the time for resolving errors with this tool. Making it more friendly to use with beginners as well as veterans.

[carlocorradini]

Soo, any update and/or workaround? 😟

[masalomon]

My suggestion would be to keep the current behavior for --check, but add another option to check and display a diff (@Osmose used --diff in this pull request), or perhaps add a --verbose option for that purpose (i.e. --check --verbose). Alternatively, the potential workaround suggested by this comment, above, could be added to the documentation (along with a description of what running prettier with no options does).

For example:

To highlight the changes to a single file that --write would make, you can run $ diff filename <(npx prettier filename) (using process substitution, on *ix style shells) Or, to use git diff's features (git-diff doesn't deal with process substitution), run $ npx prettier [options] filename | git diff [--no-index] [other options] filename -

[meza]

I would very much like to see something like this too. It's one thing to enforce a standard but it's another to educate people. Telling me that I've messed something up without telling me what it is that I've messed up or why it's a bad thing in the first place is not helpful. If someone relies on prettier --write 100% of the time, they might never know that they are making obvious mistakes. Now some might argue that they don't even care because prettier will fix it. I can accept that. However for the rest of us it's just not a useful behaviour. I'd just like to learn and become a better engineer by knowing more about the conventions I work with.

[muescha]

In most cases the prettier setup differs between local machine and server (CI/CD) and without this output you don't have a chance to find out where is the problem.

[salmiakki]

black (a popular formatter for Python) has the --diff option to show the changes it would make. This option is super handy, I used it a lot. Not so much lately. Because it has taught me to write properly.

[lydell]

dprint has diff too (dprint check), and so does gofmt (gofmt -d). So it looks like there’s precedent for such a thing!

[QuintinWillison]

Adding my +1 for this feature. I'm sitting here trying to work out why Prettier running on my local macOS hosted succeeds while the same version, running on a Linux host in CI, is failing. The lack of verbose output from Prettier when things go wrong is woeful and, I suspect symptomatic of some high-level, idealistic 'opinionated' vision held by the developers of this tool that simply doesn't match the needs of the larger majority of their user base. Perhaps try listening to your users? Users will only fight the tools for so long before they just go ahead and replace them with something that does meet their needs - I've just not got the time today to do that research, but I might tomorrow. 🤷

(edit: I'll admit I have no idea how many people are working on Prettier or whether anybody is sponsored or otherwise paid to work on it. I was recommended to include this tool in our workflows by a colleague and have broadly been very happy with it. I do appreciate all of the work that has gone in to producing something that, for the most part, seems to sit as a reliable lynchpin for many projects. So, while my comment above could perhaps be taken as a bit harsh depending on your viewpoint, the point is that I do - currently - care enough about this tool that I wanted to make the point that it could be better. ❤️ )