Skip to content

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

Merged
fabiospampinato merged 3 commits intoprettier:mainfrom
43081j:line-after-fwe
May 19, 2025
Merged

test: port over line-after-filepath-with-errors test#35
fabiospampinato merged 3 commits intoprettier:mainfrom
43081j:line-after-fwe

Conversation

@43081j
Copy link
Collaborator

@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
Collaborator 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
Collaborator 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

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 if aligning with the old CLI in this aspect isn't a huge pain in the ass we should just do that 🤔 That seems the only blocker for this PR, right?

@fabiospampinato fabiospampinato force-pushed the main branch 4 times, most recently from 0c953fb to ada6549 Compare February 10, 2025 01:33
@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
Collaborator 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.

@43081j 43081j force-pushed the line-after-fwe branch 3 times, most recently from e2dd09d to a80310c Compare May 14, 2025 14:28
@43081j
Copy link
Collaborator Author

43081j commented May 14, 2025

i dont remember anymore what we wanted --list-different to do. what it did in prettier core vs here

the tests pass now and i caught the branch up

i suggest we just get this merged before we forget again whats going on. presumably --list-different errors in prettier but not here 🤷

@fabiospampinato
Copy link
Collaborator

I think we need to rebase this PR on top of the fix for forward slashes in snapshots to make the tests for Windows pass.

43081j added 3 commits May 19, 2025 17:58
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.
@fabiospampinato fabiospampinato merged commit b9e0cb9 into prettier:main May 19, 2025
4 checks passed
@fabiospampinato
Copy link
Collaborator

Thanks!

@43081j 43081j deleted the line-after-fwe branch May 19, 2025 17:22
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