Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Remove quotes from the CSON keys as they aren't required #68

Closed
wants to merge 1 commit into from

Conversation

Victorystick
Copy link
Contributor

  • Enables nice highlighting
  • Cleaner

Sadly, it breaks git blame... 😢

* Enables nice highlighting
* Cleaner
@winstliu
Copy link
Contributor

winstliu commented Nov 4, 2015

👎 against this since all other grammars have quotes. Are you interested though in applying this change to all of the other languages?

/cc @atom/feedback

@MaximSokolov
Copy link

In addition, CSON allows to remove comment properties:

comment: '...'
# ...

🎨 :

captures:
  1:
    name: '...'
  2:
    name: '...'
captures:
  1: name: '...'
  2: name: '...'

@Victorystick
Copy link
Contributor Author

I don't mind going through all atom/language-* packages if you want. Likely, all grammars have quotes since that's what the conversion from .tmLanguage generated, not because that's what people developing the grammars want. I imagine that developers will welcome the change.

@MaximSokolov I guess we could remove the comment properties regardless.

@lee-dohm
Copy link
Contributor

lee-dohm commented Nov 4, 2015

I feel like if the problem is with the way the keys are highlighted, we should fix the highlighter, not change the text. I wouldn't change from using addition to multiplication in C++ because addition was highlighted in a way I didn't like ... I would fix the highlighter.

I agree about the comments.

@Victorystick
Copy link
Contributor Author

I agree about the comments.

😄

Changing the highlighter would be nice, but that doesn't address the cleanliness. Opinion?

@Victorystick
Copy link
Contributor Author

I see atom/language-javascript#254 is also ongoing.

@MaximSokolov
Copy link

Do you have any objection?

@Victorystick
Copy link
Contributor Author

Closing to keep the issue tracker clean. This doesn't seem likely to happen.

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.

4 participants