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 support for the JSONC format #2827

Merged
merged 2 commits into from
Mar 7, 2023
Merged

feat: Add support for the JSONC format #2827

merged 2 commits into from
Mar 7, 2023

Conversation

twpayne
Copy link
Owner

@twpayne twpayne commented Mar 6, 2023

Fixes #2825.

@felipecrs can you test this?

@felipecrs
Copy link
Contributor

This is awesome, and I'll test it.

I'd just say that I suggested fromJsonc because this is the language identifier used by vscode itself as well:

image

@felipecrs
Copy link
Contributor

felipecrs commented Mar 6, 2023

GitHub Markdown itself as well ```jsonc:

{
  // comment and trailing comma
  "test": true,
}

@felipecrs
Copy link
Contributor

Deno also accepts .jsonc as a configuration file format:

https://deno.land/[email protected]/getting_started/configuration_file

@halostatue
Copy link
Collaborator

I think that adding jsonc to the supported types/extensions (with a note that it is Visual Studio Code’s variant of JSONC) but leaving the function as fromJWCC and a note (to improve search-ability) that it will successfully parse VS Code’s JSONC is probably the right choice here, as JWCC is extremely specific about the limited extension that it does to JSON parsing.

@twpayne
Copy link
Owner Author

twpayne commented Mar 6, 2023

Thanks for the super-fast testing and feedback!

So we need two things:

  1. The name of the format, for use in documentation, and an optional link to the specification, or at least a link to where users can find out more about the format. The template function should be named after this format, e.g. fromJwcc or fromJsonc (the ugly lowecased acronym is for consistently with minorminds/sprig's ugly lowercased acronyms (Replace github.com/masterminds/sprig template functions #2668)).
  2. A list of file extensions that we read in this format. Zero or more of .jsonc, .jwcc, .hujson, and maybe others.

Obviously, as there is no official standard here, there is no right answer, only a least-bad answer. The argument "do what VSCode does" is a very strong one as this is what most developers use these days.

So, is the following reasonable?

  1. We call the format JSONC.
  2. We never link to a specification because there is none.
  3. We use the extension .jsonc only.

Thoughts?

@felipecrs
Copy link
Contributor

felipecrs commented Mar 6, 2023

So, is the following reasonable?

  1. We call the format JSONC.
  2. We never link to a specification because there is none.
  3. We use the extension .jsonc only.

Thoughts?

This is very reasonable to me, although, I believe it would be nice to mention somewhere in the docs which Go library is used to parse JSONC (which would be the closest thing to a reference for a specification IMO).

@felipecrs
Copy link
Contributor

Actually I tested it only now, but it is working great:

printf "%s\n" "{" "  // This is a comment" '  "a": "whatever",' "}" | /mnt/c/Users/felip/Downloads/chezmoi-linux-amd64/chezmoi execute-template --with-stdin '{{ fromJson .chezmoi.stdin | toPrettyJson }}'
nullprintf "%s\n" "{" "  // This is a comment" '  "a": "whatever",' "}" | /mnt/c/Users/felip/Downloads/chezmoi-linux-amd64/chezmoi execute-template --with-stdin '{{ fromJwcc .chezmoi.stdin | toPrettyJson }}'
{
  "a": "whatever"
}

"toml": FormatTOML,
"yaml": FormatYAML,
"yml": FormatYAML,
"hujson": FormatJSONC,
Copy link
Contributor

@felipecrs felipecrs Mar 6, 2023

Choose a reason for hiding this comment

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

Do you think it's still worth supporting this hujson extension?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Fair point, removed extension.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I believe it would be nice to mention somewhere in the docs which Go library is used to parse JSONC

Good point. I added a link in the fromJsonc doc.

@twpayne twpayne changed the title feat: Add support for the JWCC format feat: Add support for the JSONC format Mar 6, 2023
Copy link
Contributor

@felipecrs felipecrs left a comment

Choose a reason for hiding this comment

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

This is awesome, as everything about chezmoi.

@twpayne twpayne merged commit d407d14 into master Mar 7, 2023
@twpayne twpayne deleted the fix-2825 branch March 7, 2023 18:31
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support JSON with Comments in fromJson function
3 participants