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

Added JSONC and aws credentials to the syntax mappings #2795

Merged
merged 2 commits into from
Feb 23, 2024

Conversation

mxaddict
Copy link
Contributor

@mxaddict mxaddict commented Dec 7, 2023

I'm not 100% sure this is where these should be added.

Let me know if I need to move them somewhere else.

@Enselic
Copy link
Collaborator

Enselic commented Dec 28, 2023

Thank you for the PR. Let's wait with this PR until after #2755 has landed to avoid creating conflicts.

@mxaddict
Copy link
Contributor Author

Thank you for the PR. Let's wait with this PR until after #2755 has landed to avoid creating conflicts.

Thanks for the update, let me know when it's viable to get this added, I'll fix the branch/pr conflicts then 😄

@sharkdp
Copy link
Owner

sharkdp commented Jan 21, 2024

#2755 is now merged

@mxaddict
Copy link
Contributor Author

#2755 is now merged

Does #2755 add support for jsonc files?

Or do I need to fix my merge?

@keith-hall
Copy link
Collaborator

I'm pretty sure no actual changes were made to what mappings are applied by default in that PR, and I see no mention of JSONC in the changes

@mxaddict
Copy link
Contributor Author

Alright, I'll rebase this PR on new master branch

@mxaddict
Copy link
Contributor Author

I've applied my changes to the new structure, not 100% sure that it's done correctly though, let me know if anything needs changing.

@sharkdp sharkdp merged commit 708c74f into sharkdp:master Feb 23, 2024
22 checks passed
@akkadaya
Copy link

Shouldn't AWS config & credentials be parsed as TOML instead of INI?

@mxaddict
Copy link
Contributor Author

Shouldn't AWS config & credentials be parsed as TOML instead of INI?

I'm not sure, the official amazon docs that I looked at did not mention if it was toml formatted, but from the examples, it looks like simple INI format to me.

Granted, basic INI if I remember correctly is valid TOML as well.

@mxaddict
Copy link
Contributor Author

Shouldn't AWS config & credentials be parsed as TOML instead of INI?

I'm not sure, the official amazon docs that I looked at did not mention if it was toml formatted, but from the examples, it looks like simple INI format to me.

Granted, basic INI if I remember correctly is valid TOML as well.

image

@keith-hall
Copy link
Collaborator

conclusion: INI is correct 👍️

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

Successfully merging this pull request may close these issues.

5 participants