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 support for HOCON #5989

Merged
merged 4 commits into from
Sep 2, 2022
Merged

Add support for HOCON #5989

merged 4 commits into from
Sep 2, 2022

Conversation

LaurenceWarne
Copy link
Contributor

@LaurenceWarne LaurenceWarne commented Jul 22, 2022

Description

Hi, this PR adds support for hocon files with the .hocon file extension. Also prevalently used with hocon files is the .conf extension, though adding this would bring in a lot of false positives, so I'd like to maybe do a follow up PR for that if that's ok 😅 . Also let me know if the sample files are too large, thanks.

Checklist:

@LaurenceWarne LaurenceWarne requested a review from a team as a code owner July 22, 2022 19:12
@lildude
Copy link
Member

lildude commented Jul 25, 2022

Also let me know if the sample files are too large, thanks

Oh yes, they most definitely are (if files are not loaded by the UI in the diff you know it's almost certainly too big 😉), and they're mostly filled with comments which are completely ignored so are adding unnecessary bloat.

@LaurenceWarne
Copy link
Contributor Author

LaurenceWarne commented Jul 26, 2022

I've switched them out for some (significantly 🙂 ) shorter ones, the first has fair few comments, I can remove them if you like. I think a sample like https://github.com/emqx/hocon/blob/master/sample-configs/test01.conf would be a better overall showcase of the syntax, though I think I saw somewhere that you prefer real world examples?

@lildude lildude changed the title Add (some) Support for HOCON Add support for HOCON Sep 2, 2022
@lildude
Copy link
Member

lildude commented Sep 2, 2022

Also prevalently used with hocon files is the .conf extension, though adding this would bring in a lot of false positives, so I'd like to maybe do a follow up PR for that if that's ok 😅 .

Sure, but there be dragons. This is going to be fraught with troubles as this is an incredibly popular extension and getting a performant heuristic for what is effectively a variant of JSON is going to be a nightmare. It's better to leave it for now and peeps can use an override.

I think a sample like https://github.com/emqx/hocon/blob/master/sample-configs/test01.conf would be a better overall showcase of the syntax, though I think I saw somewhere that you prefer real world examples?

Yup. We want real world usage, not illustrations of usage. The samples here should be go on for now, especially as this is a unique extension.

@lildude lildude merged commit 06a125b into github-linguist:master Sep 2, 2022
@LaurenceWarne
Copy link
Contributor Author

Cheers!

This is going to be fraught with troubles as this is an incredibly popular extension and getting a performant heuristic for what is effectively a variant of JSON is going to be a nightmare. It's better to leave it for now and peeps can use an override.

In that case I'll take your advice, I see conf is an unfortunate choice for a file extension!

@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 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.

None yet

2 participants