From 4c71c99325b294e4c1dbb1756de500c9002ae6e4 Mon Sep 17 00:00:00 2001 From: Wilson Tayar Date: Tue, 19 Sep 2017 20:36:06 -0300 Subject: [PATCH] Normalize keymap internals (#2227) Fix #2195: normalizing keybindings using localeCompare to include non english keyboards as well --- app/config/keymaps.js | 7 +- app/utils/keymaps/find-command-by-keys.js | 5 ++ app/utils/keymaps/get-command.js | 6 ++ app/utils/keymaps/normalize.js | 13 ++++ lib/utils/keymaps.js | 4 +- .../unit/keymaps-find-command-by-keys.test.js | 64 +++++++++++++++++++ test/unit/keymaps-normalize.test.js | 22 +++++++ 7 files changed, 116 insertions(+), 5 deletions(-) create mode 100644 app/utils/keymaps/find-command-by-keys.js create mode 100644 app/utils/keymaps/get-command.js create mode 100644 app/utils/keymaps/normalize.js create mode 100644 test/unit/keymaps-find-command-by-keys.test.js create mode 100644 test/unit/keymaps-normalize.test.js diff --git a/app/config/keymaps.js b/app/config/keymaps.js index 22602c08..420bbb9b 100644 --- a/app/config/keymaps.js +++ b/app/config/keymaps.js @@ -1,4 +1,5 @@ const {readFileSync} = require('fs'); +const normalize = require('../utils/keymaps/normalize'); const {defaultPlatformKeyPath} = require('./paths'); const commands = {}; @@ -7,7 +8,7 @@ const keys = {}; const _setKeysForCommands = function (keymap) { for (const command in keymap) { if (command) { - commands[command] = keymap[command].toLowerCase(); + commands[command] = normalize(keymap[command]); } } }; @@ -35,8 +36,8 @@ const _extend = function (customsKeys) { if (customsKeys) { for (const command in customsKeys) { if (command) { - commands[command] = customsKeys[command]; - keys[customsKeys[command]] = command; + commands[command] = normalize(customsKeys[command]); + keys[normalize(customsKeys[command])] = command; } } } diff --git a/app/utils/keymaps/find-command-by-keys.js b/app/utils/keymaps/find-command-by-keys.js new file mode 100644 index 00000000..5cba0d05 --- /dev/null +++ b/app/utils/keymaps/find-command-by-keys.js @@ -0,0 +1,5 @@ +const normalize = require('./normalize'); + +module.exports = (keys, commands) => { + return commands[normalize(keys)]; +}; diff --git a/app/utils/keymaps/get-command.js b/app/utils/keymaps/get-command.js new file mode 100644 index 00000000..a27dd3ed --- /dev/null +++ b/app/utils/keymaps/get-command.js @@ -0,0 +1,6 @@ +const {getKeymaps} = require('../../config'); +const findCommandByKeys = require('./find-command-by-keys'); + +module.exports = keys => { + return findCommandByKeys(keys, getKeymaps().keys); +}; diff --git a/app/utils/keymaps/normalize.js b/app/utils/keymaps/normalize.js new file mode 100644 index 00000000..934fa48c --- /dev/null +++ b/app/utils/keymaps/normalize.js @@ -0,0 +1,13 @@ +// This function receives a keymap in any key order and returns +// the same keymap alphatetically sorted by the clients locale. +// eg.: cmd+alt+o -> alt+cmd+o +// We do this in order to normalize what the user defined to what we +// internally parse. By doing this, you can set your keymaps in any given order +// eg.: alt+cmd+o, cmd+alt+o, o+alt+cmd, etc. #2195 +module.exports = keybinding => { + function sortAlphabetically(a, b) { + return a.localeCompare(b); + } + + return keybinding.toLowerCase().split('+').sort(sortAlphabetically).join('+'); +}; diff --git a/lib/utils/keymaps.js b/lib/utils/keymaps.js index dc590b81..0f9cd697 100644 --- a/lib/utils/keymaps.js +++ b/lib/utils/keymaps.js @@ -1,6 +1,6 @@ import {remote} from 'electron'; -const {getKeymaps} = remote.require('./config'); +const getCommand = remote.require('./utils/keymaps/get-command'); export default function returnKey(e) { let keys = []; @@ -30,5 +30,5 @@ export default function returnKey(e) { } keys = keys.join('+'); - return getKeymaps().keys[keys.toLowerCase()]; + return getCommand(keys); } diff --git a/test/unit/keymaps-find-command-by-keys.test.js b/test/unit/keymaps-find-command-by-keys.test.js new file mode 100644 index 00000000..9c0cf1c4 --- /dev/null +++ b/test/unit/keymaps-find-command-by-keys.test.js @@ -0,0 +1,64 @@ +import test from 'ava'; +import findCommandByKeys from '../../app/utils/keymaps/find-command-by-keys'; + +const expectedCommand = 'test-command'; +const expectedLocalizedCommand = 'test-localized-command'; + +const commands = { + 'alt+p+shift': expectedCommand, + 'ç+cmd+ctrl': expectedLocalizedCommand +}; + +test(`returns a command`, t => { + t.is( + findCommandByKeys('alt+shift+p', commands), + expectedCommand + ); + + t.is( + findCommandByKeys('shift+p+alt', commands), + expectedCommand + ); + + t.is( + findCommandByKeys('p+alt+shift', commands), + expectedCommand + ); + + t.is( + findCommandByKeys('alt+shift+P', commands), + expectedCommand + ); + + t.is( + findCommandByKeys('Shift+P+Alt', commands), + expectedCommand + ); +}); + +test(`returns a localized command`, t => { + t.is( + findCommandByKeys('cmd+ctrl+ç', commands), + expectedLocalizedCommand + ); + + t.is( + findCommandByKeys('ç+cmd+ctrl', commands), + expectedLocalizedCommand + ); + + t.is( + findCommandByKeys('ctrl+ç+cmd', commands), + expectedLocalizedCommand + ); + + t.is( + findCommandByKeys('ctrl+Ç+cmd', commands), + expectedLocalizedCommand + ); + + t.is( + findCommandByKeys('Cmd+Ctrl+Ç', commands), + expectedLocalizedCommand + ); +}); diff --git a/test/unit/keymaps-normalize.test.js b/test/unit/keymaps-normalize.test.js new file mode 100644 index 00000000..41d94e4f --- /dev/null +++ b/test/unit/keymaps-normalize.test.js @@ -0,0 +1,22 @@ +import test from 'ava'; +import normalize from '../../app/utils/keymaps/normalize'; + +test(`is normalizing keymaps correctly`, t => { + const notNormalized = 'cmd+alt+o'; + const normalized = 'alt+cmd+o'; + + t.is( + normalize(notNormalized), + normalized + ); +}); + +test(`is normalizing localized keymaps correctly`, t => { + const notNormalized = 'cmd+alt+ç'; + const normalized = 'alt+ç+cmd'; + + t.is( + normalize(notNormalized), + normalized + ); +});