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

Use file suffix to load specific platform keybindings. #1456

Merged
merged 6 commits into from
Jan 22, 2014

Conversation

probablycorey
Copy link

For Example: On OS X, keymap files named keymap.cson and darwin.cson would load, but win32.cson would not load.

@probablycorey
Copy link
Author

I changed this from using a file suffix to uses the entire filename. This way it matches how menu's handle platform specific bindings.

@probablycorey
Copy link
Author

Fixes #1453

otherPlatforms = platforms.filter (name) -> name != process.platform

for filePath in fs.listSync(directoryPath, ['.cson', '.json'])
platform = path.basename(filePath).match(/^(\w+)\.\w+$/)?[1]
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need this regex now that you aren't using the suffix?

Copy link
Author

Choose a reason for hiding this comment

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

I thought about using path.basename(filePath, path.extname(filePath)) not in otherPlatforms but that seemed just as ugly. Is there a cleaner way?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I get it now, I prefer the basename/extname dance because then I don't have to think about what actually is in \w in case someone reports an issue about their keymap not loading someday.

This would be clear to me:

continue if path.basename(filePath, path.extname(filePath)) in otherPlatforms

probablycorey pushed a commit that referenced this pull request Jan 22, 2014
Use file suffix to load specific platform keybindings.
@probablycorey probablycorey merged commit a95a510 into master Jan 22, 2014
@probablycorey probablycorey deleted the cj-os-keybindings branch January 22, 2014 01:59
This was referenced Sep 16, 2021
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