Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix: loose strategy when suffer trailing comma in json #4379

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

Conversation

IWANABETHATGUY
Copy link
Contributor

@IWANABETHATGUY IWANABETHATGUY commented Apr 16, 2023

Summary

{
"test", "something",
"another", "something",
}

trailing comma syntax error is a common syntax error when copying and pasting something in the json file, we could lose this kind of syntax error, and give our formatter a chance to format the code, like pretty did.

# Prettier differences
```diff
--- Prettier
+++ Rome
@@ -1 +1,9 @@
-[[1, null], [1, null], [null], [0], [false], [""]]
+[
+ [
+1,null],
+ [1,null,],
+ [null,],
+ [0,],
+ [false,],
+ ['',]
+]
```

Test Plan

Changelog

  • The PR requires a changelog line

Documentation

  • The PR requires documentation
  • I will create a new PR to update the documentation

@netlify
Copy link

netlify bot commented Apr 16, 2023

Deploy Preview for docs-rometools canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 9bc3f89
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/643bd352660dc50008c8f4f3

@github-actions github-actions bot added the A-Parser Area: parser label Apr 16, 2023
@ematipico
Copy link
Contributor

I am a bit hesitant about this "fix":

  • we're not fixing anything, we're actually injecting non-spec changes to a language;
  • we can't compare ourselves to prettier in that sense. Rome does work in LSP too, and there might be people that rely on diagnostics even when the formatter is disabled;
  • if a JSON file is used by users inside an application and they don't use the formatter, that file will raise an error;

How did you want to introduce this change?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Parser Area: parser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants