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

feat: Add allowTrailingCommas option for JSONC #42

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

Conversation

nzakas
Copy link
Member

@nzakas nzakas commented Oct 16, 2024

Prerequisites checklist

What is the purpose of this pull request?

Add support for trailing commas to the JSONC language.

What changes did you make? (Give an overview)

This pull request introduces support for allowing trailing commas in JSONC files and includes several updates to the codebase and documentation to reflect this new feature. The most important changes include adding a new option to enable trailing commas, updating the JSONLanguage class to validate and handle this option, and adding tests to ensure the feature works as expected.

New Feature: Allow Trailing Commas in JSONC

  • README.md: Added a section explaining how to enable trailing commas in JSONC files using the allowTrailingCommas language option.

Code Updates

  • src/languages/json-language.js:
    • Added a new typedef for JSONLanguageOptions to include the allowTrailingCommas property.
    • Updated the validateLanguageOptions method to check for the allowTrailingCommas option and ensure it is only used in JSONC mode.
    • Modified the parse method to accept and handle the allowTrailingCommas option.

Dependency Update

  • package.json: Updated the @humanwhocodes/momoa dependency to version ^3.3.0.

Tests

  • tests/languages/json-language.test.js:
    • Added tests for the validateLanguageOptions method to ensure it correctly handles the allowTrailingCommas option.
    • Added tests for the parse method to verify that trailing commas are correctly parsed or rejected based on the allowTrailingCommas option.

Related Issues

fixes #41

Is there anything you'd like reviewers to focus on?

Right now, validateLanguageOptions() throws an error when it finds allowTrailingCommas in JSON mode (where it cannot be enabled) and JSON5 mode (where it cannot be disabled). Would it be better to check the value of allowTrailingCommas before deciding to throw an error? So in JSON mode it would only throw if allowTrailingCommas is true and in JSON5 mode it would only throw an error if allowTrailingCommas is false?

@mdjermanovic
Copy link
Member

Right now, validateLanguageOptions() throws an error when it finds allowTrailingCommas in JSON mode (where it cannot be enabled) and JSON5 mode (where it cannot be disabled). Would it be better to check the value of allowTrailingCommas before deciding to throw an error? So in JSON mode it would only throw if allowTrailingCommas is true and in JSON5 mode it would only throw an error if allowTrailingCommas is false?

I think it's fine to throw an error when this option is used in a mode where it isn't applicable, regardless of the value.

language: "json/jsonc",
languageOptions: {
allowTrailingCommas: true
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}
},

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

Successfully merging this pull request may close these issues.

Feature: Dangling commas in JSONC
2 participants