Desktop: Make sure all commands appear in keymap editor

CliClient/tests/services_KeymapService.js

@@ -3,6 +3,7 @@ require('app-module-path').addPath(__dirname);
 const { tempFilePath } = require('test-utils.js');
 const KeymapService = require('lib/services/KeymapService').default;
 const keymapService = KeymapService.instance();
+keymapService.initialize([]);
 
 describe('services_KeymapService', () => {
 	describe('validateAccelerator', () => {
@@ -31,7 +32,7 @@ describe('services_KeymapService', () => {
 			};
 
 			Object.entries(testCases).forEach(([platform, accelerators]) => {
-				keymapService.initialize(platform);
+				keymapService.initialize([], platform);
 				accelerators.forEach(accelerator => {
 					expect(() => keymapService.validateAccelerator(accelerator)).not.toThrow();
 				});
@@ -69,7 +70,7 @@ describe('services_KeymapService', () => {
 			};
 
 			Object.entries(testCases).forEach(([platform, accelerators]) => {
-				keymapService.initialize(platform);
+				keymapService.initialize([], platform);
 				accelerators.forEach(accelerator => {
 					expect(() => keymapService.validateAccelerator(accelerator)).toThrow();
 				});
@@ -81,12 +82,12 @@ describe('services_KeymapService', () => {
 		beforeEach(() => keymapService.initialize());
 
 		it('should allow registering new commands', async () => {
-			keymapService.initialize('linux');
+			keymapService.initialize([], 'linux');
 			keymapService.registerCommandAccelerator('myCustomCommand', 'Ctrl+Shift+Alt+B');
 			expect(keymapService.getAccelerator('myCustomCommand')).toEqual('Ctrl+Shift+Alt+B');
 
 			// Check that macOS key conversion is working
-			keymapService.initialize('darwin');
+			keymapService.initialize([], 'darwin');
 			keymapService.registerCommandAccelerator('myCustomCommand', 'CmdOrCtrl+Shift+Alt+B');
 			expect(keymapService.getAccelerator('myCustomCommand')).toEqual('Cmd+Shift+Option+B');
 			keymapService.setAccelerator('myCustomCommand', 'Cmd+Shift+Option+X');
@@ -95,7 +96,7 @@ describe('services_KeymapService', () => {
 			const keymapFilePath = tempFilePath('json');
 			await keymapService.saveCustomKeymap(keymapFilePath);
 
-			keymapService.initialize('darwin');
+			keymapService.initialize([], 'darwin');
 			await keymapService.loadCustomKeymap(keymapFilePath);
 
 			expect(keymapService.getAccelerator('myCustomCommand')).toEqual('Cmd+Shift+Option+X');
@@ -106,17 +107,17 @@ describe('services_KeymapService', () => {
 		beforeEach(() => keymapService.initialize());
 
 		it('should return the platform-specific default Accelerator', () => {
-			keymapService.initialize('darwin');
+			keymapService.initialize([], 'darwin');
 			expect(keymapService.getAccelerator('newNote')).toEqual('Cmd+N');
 			expect(keymapService.getAccelerator('synchronize')).toEqual('Cmd+S');
 			expect(keymapService.getAccelerator('textSelectAll')).toEqual('Cmd+A');
 			expect(keymapService.getAccelerator('textBold')).toEqual('Cmd+B');
 
-			keymapService.initialize('linux');
+			keymapService.initialize([], 'linux');
 			expect(keymapService.getAccelerator('newNote')).toEqual('Ctrl+N');
 			expect(keymapService.getAccelerator('synchronize')).toEqual('Ctrl+S');
 
-			keymapService.initialize('win32');
+			keymapService.initialize([], 'win32');
 			expect(keymapService.getAccelerator('textSelectAll')).toEqual('Ctrl+A');
 			expect(keymapService.getAccelerator('textBold')).toEqual('Ctrl+B');
 		});
@@ -130,7 +131,7 @@ describe('services_KeymapService', () => {
 		beforeEach(() => keymapService.initialize());
 
 		it('should update the Accelerator', () => {
-			keymapService.initialize('darwin');
+			keymapService.initialize(['print'], 'darwin');
 			const testCases_Darwin = [
 				{ command: 'newNote', accelerator: 'Ctrl+Option+Shift+N' },
 				{ command: 'synchronize', accelerator: 'F11' },
@@ -147,7 +148,7 @@ describe('services_KeymapService', () => {
 				expect(keymapService.getAccelerator(command)).toEqual(accelerator);
 			});
 
-			keymapService.initialize('linux');
+			keymapService.initialize(['print'], 'linux');
 			const testCases_Linux = [
 				{ command: 'newNote', accelerator: 'Ctrl+Alt+Shift+N' },
 				{ command: 'synchronize', accelerator: 'F15' },
@@ -167,7 +168,7 @@ describe('services_KeymapService', () => {
 	});
 
 	describe('getDefaultAccelerator', () => {
-		beforeEach(() => keymapService.initialize());
+		beforeEach(() => keymapService.initialize(['print', 'linux']));
 
 		it('should return the default accelerator', () => {
 			const testCases = [
@@ -196,7 +197,7 @@ describe('services_KeymapService', () => {
 		beforeEach(() => keymapService.initialize());
 
 		it('should update the keymap', () => {
-			keymapService.initialize('darwin');
+			keymapService.initialize([], 'darwin');
 			const customKeymapItems_Darwin = [
 				{ command: 'newNote', accelerator: 'Option+Shift+Cmd+N' },
 				{ command: 'synchronize', accelerator: 'Ctrl+F11' },
@@ -217,7 +218,7 @@ describe('services_KeymapService', () => {
 				expect(keymapService.getAccelerator(command)).toEqual(accelerator);
 			});
 
-			keymapService.initialize('win32');
+			keymapService.initialize([], 'win32');
 			const customKeymapItems_Win32 = [
 				{ command: 'newNote', accelerator: 'Ctrl+Alt+Shift+N' },
 				{ command: 'synchronize', accelerator: 'Ctrl+F11' },

ElectronClient/app.ts

@@ -495,14 +495,6 @@ class Application extends BaseApplication {
 		const filename = Setting.custom_css_files.JOPLIN_APP;
 		await CssUtils.injectCustomStyles(`${dir}/${filename}`);
 
-		const keymapService = KeymapService.instance();
-
-		try {
-			await keymapService.loadCustomKeymap(`${dir}/keymap-desktop.json`);
-		} catch (err) {
-			reg.logger().error(err.message);
-		}
-
 		AlarmService.setDriver(new AlarmServiceDriverNode({ appName: packageInfo.build.appId }));
 		AlarmService.setLogger(reg.logger());
 
@@ -533,6 +525,15 @@ class Application extends BaseApplication {
 			CommandService.instance().registerDeclaration(declaration);
 		}
 
+		const keymapService = KeymapService.instance();
+		keymapService.initialize(CommandService.instance().commandNames(true));
+
+		try {
+			await keymapService.loadCustomKeymap(`${dir}/keymap-desktop.json`);
+		} catch (error) {
+			reg.logger().error(error);
+		}
+
 		// Since the settings need to be loaded before the store is created, it will never
 		// receive the SETTING_UPDATE_ALL even, which mean state.settings will not be
 		// initialised. So we manually call dispatchUpdateAll() to force an update.

ElectronClient/gui/KeymapConfig/utils/getLabel.ts

@@ -5,7 +5,7 @@ import { _ } from 'lib/locale';
 
 const commandService = CommandService.instance();
 
-const getLabel = (commandName: string) => {
+const getLabel = (commandName: string):string => {
 	if (commandService.exists(commandName)) return commandService.label(commandName, true);
 
 	// Some commands are not registered in CommandService at the moment

ElectronClient/gui/KeymapConfig/utils/useKeymap.ts

@@ -1,11 +1,22 @@
 import { useState, useEffect } from 'react';
 import KeymapService, { KeymapItem } from 'lib/services/KeymapService';
+import getLabel from './getLabel';
 
 const keymapService = KeymapService.instance();
 
 // This custom hook provides a synchronized snapshot of the keymap residing at KeymapService
 // All the logic regarding altering and interacting with the keymap is isolated from the components
 
+function allKeymapItems() {
+	const output = keymapService.getKeymapItems().slice();
+
+	output.sort((a:KeymapItem, b:KeymapItem) => {
+		return getLabel(a.command).toLocaleLowerCase() < getLabel(b.command).toLocaleLowerCase() ? -1 : +1;
+	});
+
+	return output;
+}
+
 const useKeymap = (): [
 	KeymapItem[],
 	Error,
@@ -13,7 +24,7 @@ const useKeymap = (): [
 	(commandName: string, accelerator: string) => void,
 	(commandName: string) => void
 ] => {
-	const [keymapItems, setKeymapItems] = useState<KeymapItem[]>(() => keymapService.getKeymapItems());
+	const [keymapItems, setKeymapItems] = useState<KeymapItem[]>(() => allKeymapItems());
 	const [keymapError, setKeymapError] = useState<Error>(null);
 	const [mustSave, setMustSave] = useState(false);
 
@@ -42,7 +53,7 @@ const useKeymap = (): [
 
 	const overrideKeymapItems = (customKeymapItems: KeymapItem[]) => {
 		const oldKeymapItems = [...customKeymapItems];
-		keymapService.initialize(); // Start with a fresh keymap
+		keymapService.resetKeymap(); // Start with a fresh keymap
 
 		try {
 			// First, try to update the in-memory keymap of KeymapService

ReactNativeClient/lib/services/CommandService.ts

@@ -144,8 +144,17 @@ export default class CommandService extends BaseService {
 		return output;
 	}
 
-	public commandNames() {
-		return Object.keys(this.commands_);
+	public commandNames(publicOnly:boolean = false) {
+		if (publicOnly) {
+			const output = [];
+			for (const name in this.commands_) {
+				if (!this.isPublic(name)) continue;
+				output.push(name);
+			}
+			return output;
+		} else {
+			return Object.keys(this.commands_);
+		}
 	}
 
 	public commandByName(name:string, options:CommandByNameOptions = null):Command {
@@ -230,6 +239,10 @@ export default class CommandService extends BaseService {
 		return stateToWhenClauseContext(this.store_.getState());
 	}
 
+	public isPublic(commandName:string) {
+		return !!this.label(commandName);
+	}
+
 	// When looping on commands and checking their enabled state, the whenClauseContext
 	// should be specified (created using currentWhenClauseContext) to avoid having
 	// to re-create it on each call.

ReactNativeClient/lib/services/KeymapService.ts

@@ -16,7 +16,6 @@ const defaultKeymapItems = {
 		{ accelerator: 'Cmd+N', command: 'newNote' },
 		{ accelerator: 'Cmd+T', command: 'newTodo' },
 		{ accelerator: 'Cmd+S', command: 'synchronize' },
-		{ accelerator: '', command: 'print' },
 		{ accelerator: 'Cmd+H', command: 'hideApp' },
 		{ accelerator: 'Cmd+Q', command: 'quit' },
 		{ accelerator: 'Cmd+,', command: 'config' },
@@ -51,7 +50,6 @@ const defaultKeymapItems = {
 		{ accelerator: 'Ctrl+N', command: 'newNote' },
 		{ accelerator: 'Ctrl+T', command: 'newTodo' },
 		{ accelerator: 'Ctrl+S', command: 'synchronize' },
-		{ accelerator: null, command: 'print' },
 		{ accelerator: 'Ctrl+Q', command: 'quit' },
 		{ accelerator: 'Ctrl+Alt+I', command: 'insertTemplate' },
 		{ accelerator: 'Ctrl+C', command: 'textCopy' },
@@ -103,29 +101,42 @@ export default class KeymapService extends BaseService {
 		super();
 
 		this.lastSaveTime_ = Date.now();
-
-		// By default, initialize for the current platform
-		// Manual initialization allows testing for other platforms
-		this.initialize();
 	}
 
 	public get lastSaveTime():number {
 		return this.lastSaveTime_;
 	}
 
-	public initialize(platform: string = shim.platformName()) {
+	// `additionalDefaultCommandNames` will be added to the default keymap
+	// **except** if they are already in it. Basically this is a mechanism
+	// to add all the commands from the command service to the default
+	// keymap.
+	public initialize(additionalDefaultCommandNames:string[] = [], platform: string = shim.platformName()) {
 		this.platform = platform;
 
 		switch (platform) {
 		case 'darwin':
-			this.defaultKeymapItems = defaultKeymapItems.darwin;
+			this.defaultKeymapItems = defaultKeymapItems.darwin.slice();
 			this.modifiersRegExp = modifiersRegExp.darwin;
 			break;
 		default:
-			this.defaultKeymapItems = defaultKeymapItems.default;
+			this.defaultKeymapItems = defaultKeymapItems.default.slice();
 			this.modifiersRegExp = modifiersRegExp.default;
 		}
 
+		for (const name of additionalDefaultCommandNames) {
+			if (this.defaultKeymapItems.find((item:KeymapItem) => item.command === name)) continue;
+			this.defaultKeymapItems.push({
+				command: name,
+				accelerator: null,
+			});
+		}
+
+		this.resetKeymap();
+	}
+
+	// Reset keymap back to its default values
+	public resetKeymap() {
 		this.keymap = {};
 		for (let i = 0; i < this.defaultKeymapItems.length; i++) {
 			// Keep the original defaultKeymapItems array untouched
@@ -140,7 +151,9 @@ export default class KeymapService extends BaseService {
 		if (await shim.fsDriver().exists(customKeymapPath)) {
 			this.logger().info(`KeymapService: Loading keymap from file: ${customKeymapPath}`);
 
-			const customKeymapFile = await shim.fsDriver().readFile(customKeymapPath, 'utf-8');
+			const customKeymapFile = (await shim.fsDriver().readFile(customKeymapPath, 'utf-8')).trim();
+			if (!customKeymapFile) return;
+
 			// Custom keymaps are supposed to contain an array of keymap items
 			this.overrideKeymap(JSON.parse(customKeymapFile));
 		}
@@ -183,8 +196,8 @@ export default class KeymapService extends BaseService {
 
 		if (!commandName) throw new Error('Cannot register an accelerator without a command name');
 
-		const validatedAccelerator = this.convertToPlatform(accelerator);
-		this.validateAccelerator(validatedAccelerator);
+		const validatedAccelerator = accelerator ? this.convertToPlatform(accelerator) : null;
+		if (validatedAccelerator) this.validateAccelerator(validatedAccelerator);
 
 		this.keymap[commandName] = {
 			command: commandName,
@@ -264,7 +277,7 @@ export default class KeymapService extends BaseService {
 			// Throws whenever there are duplicate Accelerators used in the keymap
 			this.validateKeymap();
 		} catch (err) {
-			this.initialize(); // Discard all the changes if there are any issues
+			this.resetKeymap(); // Discard all the changes if there are any issues
 			throw err;
 		}
 	}