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

Add json5 support to mixin config jsons #2375

Merged

Conversation

Bawnorton
Copy link
Contributor

@Bawnorton Bawnorton commented Sep 18, 2024

Allow mixin config json inspections and references to recognise mixin configs of the json5 file type

Motivation

Stonecutter, a gradle pre-processor plugin has been gaining traction for it's use in multi-loader, multi-version projects, and a plugin that was developed alongside it, j52j, which converts json5 files to json during compilation allowing versioned comments within json files resulting in json5 being used as the file type for a mixin config:

https://github.com/Bawnorton/BetterTrims/blob/8558c398e0cd5a85fdd5dce6b2ed9e410d239743/src/main/resources/bettertrims.mixins.json5#L6-L12

mcdev doesn't recognise json5 as a valid file type for mixin config jsons, so this pr adds that support.

Copy link
Member

@RedNesto RedNesto left a comment

Choose a reason for hiding this comment

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

I don't think all of those changes are needed at all. Simply changing filenameRegex by adding 5? at the end of the extension should do the job.

@Bawnorton
Copy link
Contributor Author

Bawnorton commented Sep 19, 2024

I tested that initially and it causes a json language inspection in the mixin json for violating the json standard as the config file type specifies that the language is Json not Json5

Screenshot 2024-09-19 at 3 24 17 PM

Copy link
Member

@RedNesto RedNesto left a comment

Choose a reason for hiding this comment

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

Looks good now, thanks for the PR 👍

@RedNesto RedNesto merged commit 56b3764 into minecraft-dev:dev Sep 23, 2024
4 checks passed
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.

2 participants