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

test: port over line-after-filepath-with-errors test #35

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented Feb 3, 2025

Copies the line-after-filepath-with-errors tests from prettier.

Notes on differences:

  • Running prettier ./* vs prettier --check ./* has different behaviour for invalid files. in the former, no errors are raised and the files are essentially ignored. We should decide if this is expected
  • Running with --list-different, invalid files have their paths listed in stderr, while in prettier they will raise errors.

});
// TODO (43081j): this test throws errors in prettier. here it doesn't, and
// lists each invalid file path in the output (stderr). we should decide
// if to align with prettier
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see this TODO and the one above

this has been raised in another PR too. basically this block:

prettier-cli/src/index.ts

Lines 181 to 185 in 5d5a6ca

if (options.check || options.write) {
stderr.prefixed.error(`${fileRelativePath}: ${error}`);
} else if (options.list) {
stderr.error(fileRelativePath);
}

means that with no args, errors are not raised and are ignored

this means running with list: true or check: true behaves differently than with no args (i.e. both false)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should first add the fourth mode of operation as mentioned in #30, then I need to look at what the difference is exactly, because if prettier just throws it seems better to output a comprehensible error, while still erroring, no?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "forth mode", "dump" is implemented now, maybe we can reference that here if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the two tests fail in prettier because the files matching the glob are invalid I think (syntax errors)

so in this test (--list-different) and the argless one, we do error in prettier but not in prettier CLI

the fourth mode doesn't seem relevant now that I think about it. in general if a glob matches invalid files, it seems we skip them? but I don't remember what past-james was doing here, so I'm just going off the TODOs

Copies the `line-after-filepath-with-errors` tests from prettier.

Notes on differences:

- Running `prettier ./*` vs `prettier --check ./*` has different
  behaviour for invalid files. in the former, no errors are raised and
  the files are essentially ignored. We should decide if this is expected
- Running with `--list-different`, invalid files have their paths listed in
  stderr, while in prettier they will raise errors.
@43081j 43081j force-pushed the line-after-fwe branch 2 times, most recently from 18074f4 to fd79cfa Compare March 2, 2025 19:43
@43081j
Copy link
Contributor Author

43081j commented Mar 2, 2025

Blocked by #51

@fabiospampinato fabiospampinato force-pushed the main branch 4 times, most recently from 34e0257 to f8dc396 Compare March 6, 2025 00:45
@fabiospampinato
Copy link
Collaborator

fabiospampinato commented Mar 6, 2025

#51 got addressed, maybe this can be finished now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants