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

[BUG] Dry run reporting unpretty files when all files match styles #37

Closed
davwheat opened this issue Jan 19, 2021 · 8 comments · Fixed by #48
Closed

[BUG] Dry run reporting unpretty files when all files match styles #37

davwheat opened this issue Jan 19, 2021 · 8 comments · Fixed by #48
Assignees
Labels
bug Something isn't working

Comments

@davwheat
Copy link

What exactly happened?
When performing a dry run, Prettier reports that all files match the code style, but this action reports unpretty files.

See: https://github.com/flarumite/simple-spoilers/runs/1731327132#step:4:19

How did it look?
image

@davwheat davwheat added the bug Something isn't working label Jan 19, 2021
@creyD
Copy link
Owner

creyD commented Feb 3, 2021

Pretty weird, sorry I wasn't able to investigate, will take a couple of days more. Do you have an idea what causes this? Maybe git changes the newline chars or something? There must be some kind of change.

@davwheat
Copy link
Author

davwheat commented Feb 3, 2021

Honestly I have no clue.

Prettier outputs that it all matches, then the action reports a failure.

I ended up switching to my own action in the end.

@fsz-codeshop
Copy link
Contributor

Hi guys! Maybe it is caused by this block on entry point?

  if $INPUT_DRY; then
    echo "Prettier found unpretty files!"
    exit 1
  else

creyD added a commit that referenced this issue Apr 10, 2021
…npretty

Fixes Issue #37 [BUG] Dry run reporting unpretty files when all files match styles
@creyD creyD closed this as completed Apr 10, 2021
@creyD
Copy link
Owner

creyD commented Apr 10, 2021

@fsz-codeshop Can you please test it with the new push in dev? I tried your fix a couple of times now and it doesn't seem to work (https://github.com/creyD/prettier_test/runs/2313102123)

@fsz-codeshop
Copy link
Contributor

Hi @creyD, you are indeed right. It is not working with the dev branch. I am getting:

Error:  No parser and no file path given, couldn't infer a parser.

I have forked and worked on top of the main branch for that dry-run fix.
Nevertheless, I have also checked out the last commit on dev before merging my changes and I am still getting the same error (87e1553). When using fsz-codeshop/prettier_action@issue-37-dry-run-reporting-unpretty I get it working as expected:

Installing prettier...
/usr/local/bin/prettier -> /usr/local/lib/node_modules/prettier/bin-prettier.js
+ prettier@2.2.1
added 1 package from 1 contributor in 0.627s
Prettifying files...
Files:
Checking formatting...
[warn] .github/PULL_REQUEST_TEMPLATE.md
[warn] docs/CONTRIBUTING.md
[warn] index.js
[warn] README.md
[warn] test.js
[warn] Code style issues found in the above file(s). Forgot to run Prettier?
Problem running prettier with --check --ignore-path ./.prettierignore --config ./prettier.config.js **/*.{js,ts,md}
Nothing to commit. Exiting.

Maybe it is something else of the other changes?

@fsz-codeshop
Copy link
Contributor

Furthermore, checking the provided link (https://github.com/creyD/prettier_test/runs/2313102123) it seems that it worked as expected

No unpretty files! Finishing dry-run.

Did I miss something?

@creyD
Copy link
Owner

creyD commented Apr 13, 2021

Furthermore, checking the provided link (https://github.com/creyD/prettier_test/runs/2313102123) it seems that it worked as expected

No unpretty files! Finishing dry-run.

Did I miss something?

It states that it worked, but there was a unpretty file and it even showed it in the "Files:" statement.

@fsz-codeshop
Copy link
Contributor

fsz-codeshop commented Apr 13, 2021

Ok. I think I got it.
The logic only works when using --check configuration because of the prettier command status code

Anyway, I reopened the PR (#49) with the change of the logic to handle the cases using --write + dry-run and --check + dry-run.

I have forked the prettier_test and ran some tests and it seems it is working as expected now. If you can check it =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants