Skip to content

fix: apply --skip-parse-errors to lint, check and ci commands #5780

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

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

Conversation

unvalley
Copy link
Member

@unvalley unvalley commented Apr 26, 2025

Summary

Closes #3859
This PR applies the --skip-parse-errors option to below commands.

  • biome lint
  • biome check
  • biome ci

biome format is already applied the option.

Since there is no need to display parse errors in analyze and format respectively, parse errors in format executed later are not output as messages (commented in code).

Test Plan

Added snapshot test

@github-actions github-actions bot added the A-CLI Area: CLI label Apr 26, 2025
Comment on lines 67 to 69
if ctx.execution.should_ignore_errors() && skipped_parse_error {
// skip format to parse error files, a message for it already pushed by analyzer
} else {
Copy link
Member Author

Choose a reason for hiding this comment

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

do nothing if already skipped parse error on the analyze phase.

@unvalley unvalley marked this pull request as ready for review April 27, 2025 05:09
@ematipico
Copy link
Member

I'm wondering if we should rename this flag. I introduced this option at the very beginning, when there was only a formatter. Now the meaning of "error" is different, and now there's a linter too.

What do you think?

@unvalley
Copy link
Member Author

unvalley commented Apr 27, 2025

@ematipico

Here’s the current description of --skip-errors:

–skip-errors — Skip files with syntax errors instead of emitting an error diagnostic.

Because the flag only deals with parse errors, how about renaming it to --skip-parse-errors?

I’d also suggest making it a sub-command option rather than a global one. Commands like lint, format , check , and ci need it, but the others don’t.

PS: rethink, we don't need to make it a specific options for them, I leave it as a global option.

@ematipico
Copy link
Member

--skip-parse-errors seems good!

@unvalley
Copy link
Member Author

unvalley commented Apr 27, 2025

@ematipico Can we include the renaming to v2.0.0 release? (Assuming I make that change in this PR)
If so, I think we can rename --skip-errors to --skip-parse-errors without the deprecation phase.

@ematipico
Copy link
Member

Can we include the renaming to v2.0.0 release?

Yes, that was the idea :)

@unvalley unvalley marked this pull request as draft April 28, 2025 19:02
@github-actions github-actions bot added A-Project Area: project A-Linter Area: linter A-LSP Area: language server protocol L-JavaScript Language: JavaScript and super languages L-JSON Language: JSON and super languages labels Apr 29, 2025
@github-actions github-actions bot removed A-Project Area: project A-Linter Area: linter A-LSP Area: language server protocol labels Apr 29, 2025
Copy link

codspeed-hq bot commented Apr 29, 2025

CodSpeed Performance Report

Merging #5780 will not alter performance

Comparing unvalley:use-skip-errors (78ed2dd) with main (5f321a9)

Summary

✅ 95 untouched benchmarks

@unvalley unvalley changed the title fix: apply --skip-errors to lint, check and ci commands fix: apply --skip-parse-errors to lint, check and ci commands May 1, 2025
@unvalley unvalley marked this pull request as ready for review May 1, 2025 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLI Area: CLI L-JavaScript Language: JavaScript and super languages L-JSON Language: JSON and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 --skip-errors CLI option is ignored
2 participants