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

JSON.yaml: Add JSON definition #10

Merged
merged 1 commit into from
Oct 27, 2018
Merged

JSON.yaml: Add JSON definition #10

merged 1 commit into from
Oct 27, 2018

Conversation

suggoitanoshi
Copy link
Contributor

No description provided.

# Block delimiter:
- curly_braces
- square_braces
keywords: []
Copy link
Member

Choose a reason for hiding this comment

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

You can remove this line as there are no keywords.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was unsure about what to do with that field, I'll remove it now

Copy link
Member

@Man-Jain Man-Jain left a comment

Choose a reason for hiding this comment

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

You can also add an json in aliases.

- json
line_continuation: ''
delimiters:
# From coala JSON language definition
Copy link
Member

Choose a reason for hiding this comment

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

No need for this comment

line_continuation: ''
delimiters:
# From coala JSON language definition
# JSON don't have any comments
Copy link
Member

Choose a reason for hiding this comment

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

This one too is not needed.

# From coala JSON language definition
# JSON don't have any comments
# String delimiter:
- double_quote
Copy link
Member

Choose a reason for hiding this comment

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

JSON string have an escape character. Choose a more specific delimiter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, that's right. I was a bit confused by what delimiter to use, but I'll check them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is double_quote_slash_escape okay? I'm not sure about the difference between it and double_quote

Copy link
Member

Choose a reason for hiding this comment

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

Yup, JSON uses backslash escaping.

- json
aliases:
- json
line_continuation: ''
Copy link
Member

Choose a reason for hiding this comment

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

What does this mean?

Does JSON have a line continuation token?
If not, remove this property.

Copy link
Contributor Author

@suggoitanoshi suggoitanoshi Oct 27, 2018

Choose a reason for hiding this comment

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

Alright, so we just drop the properties if it's not used? At first I thought some property needs to be present, so I just kind of kept it there... I'll remember that next time, though!

git add . Outdated
@@ -0,0 +1,14 @@
identifier: JSON
Copy link
Member

Choose a reason for hiding this comment

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

This file is created by mistake. Remove it 😃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uh oh. I'm sorry, I'll remove it now!

Copy link
Member

Choose a reason for hiding this comment

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

Np 😄

Copy link
Member

@Man-Jain Man-Jain 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 to me. One last thing. Please squash your commits into one. You can follow this guide https://api.coala.io/en/latest/Developers/Git_Basics.html#squashing-your-commits Please feel free to ask if you face any problem.

@suggoitanoshi
Copy link
Contributor Author

Thanks @jayvdb and @Man-Jain for reviewing my first PR! I sure make a lot of mistake for one, but I'll try to not do as much in the future.

Copy link
Member

@Man-Jain Man-Jain left a comment

Choose a reason for hiding this comment

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

No problem you did great 👍

@jayvdb
Copy link
Member

jayvdb commented Oct 27, 2018

ack d0771c6

@jayvdb
Copy link
Member

jayvdb commented Oct 27, 2018

@gitmate-bot ff

@jayvdb
Copy link
Member

jayvdb commented Oct 27, 2018

Hey! I'm GitMate.io! This pull request is being fastforwarded automatically. Please DO NOT push while fastforward is in progress or your changes would be lost permanently ⚠️

@jayvdb jayvdb merged commit d0771c6 into coala:master Oct 27, 2018
@jayvdb
Copy link
Member

jayvdb commented Oct 27, 2018

Automated fastforward with GitMate.io was successful! 🎉

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

Successfully merging this pull request may close these issues.

3 participants