Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

Clarify that config files require unique keys #7562

Merged
merged 6 commits into from Jul 29, 2015
Merged

Conversation

mnquintana
Copy link
Contributor

From #7552 (comment)

A lot of people try to use the config files as they would a stylesheet, trying to override keybindings for instance by repeating the same selector multiple times in the file, which causes issues because CSON keys must be unique. This makes it a bit more explicit that keys need to be unique.

@thomasjo
Copy link
Contributor

Looks good to me! ⛵

@@ -2,7 +2,8 @@
#
# Atom keymaps work similarly to style sheets. Just as style sheets use
# selectors to apply styles to elements, Atom keymaps use selectors to associate
# keystrokes with events in specific contexts.
# keystrokes with events in specific contexts. (Unlike style sheets however,
# each selector can only be declared once.)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can drop the parens here.

Also, it might be helpful to give an example of the error, and then how the error should be corrected (to make things perfectly clear for users who just copy/paste examples). For example:

# This is not correct
#
# 'atom-text-editor':
#   'enter': 'editor:newline'
# 'atom-text-editor':
#   'end': 'editor:move-to-end-of-screen-line'
#
# This is correct:
#
# 'atom-text-editor':
#   'enter': 'editor:newline'
#   'end': 'editor:move-to-end-of-screen-line'

Another approach would be to fix atom/flight-manual.atom.io#43 📝, and then link to those docs with a big WARNING. I think I like that approach a bit more.

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 think I prefer the latter option too – putting the examples inline here would make the comments section really long.

@izuzak
Copy link
Contributor

izuzak commented Jul 2, 2015

@mnquintana for this -- helping users not make these mistakes would be great. I left two comments -- wondering what your 💭 are.

@mnquintana
Copy link
Contributor Author

@izuzak Just opened up atom/flight-manual.atom.io#100 - I'll link to that new section from within these comments.

# https://github.com/bevry/cson#what-is-cson
# If you are unfamiliar with CSON, you can read more about it in the
# Atom Flight Manual:
# https://atom.io/docs/latest/using-atom-basic-customization#cson
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Btw @izuzak, I changed this link to the CSON Primer I added in atom/flight-manual.atom.io#100 – do you think that's okay? There is a footnote in that section to the original link here, and it expands upon the source CSON docs quite a bit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Btw @izuzak, I changed this link to the CSON Primer I added in atom/flight-manual.atom.io#100 – do you think that's okay?

Yep, looks great to me 👍

@izuzak
Copy link
Contributor

izuzak commented Jul 29, 2015

😎 Thanks again for this, @mnquintana

mnquintana added a commit that referenced this pull request Jul 29, 2015
Clarify that config files require unique keys
@mnquintana mnquintana merged commit ae6f367 into master Jul 29, 2015
@mnquintana mnquintana deleted the mq-dot-atom-cson branch July 29, 2015 19:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants