Suggestion on data structure for keyboard shortcut project

Hi @anjulalk,

I had another look at your proposal and one issue I have with it is how the shortcuts themselves are defined:

newNoteShortcut:​ {
​	value: '​C​ommandOrControl+N​',​ 
	// ....
	label:​​() ​=>​_​(​'​New note'​),
}

Essentially the shortcut is associated with a label, and somewhere in Joplin you associate that shortcut with an action. There’s some issue with this approach:

  1. The label is a duplicate - it would also be defined in the menu item, in the toolbar button tooltip, and basically anywhere this action is used

  2. It’s also not computer-friendly to associate a shortcut with a user-facing string. Let’s say I want to find what shortcut is currently associated with the “New note” action - you’d need to translate that string to the current language, and lookup the shortcut from that.

For me these two issues are because the shortcut is associated with the wrong thing (a label, essentially). However, Joplin defines many commands in the app, for example the “newNote”, “newFolder”, “commandStartExternalEditing”, etc. so I think that’s what you should be associating shortcuts with.

Then if you need to know the label of a command to display in your list, you could perhaps define a new structure that associate a command with various other properties, such as indeed a label.

In general Joplin commands are a bit messy - they should have been better defined from the start, for example like it’s done in Qt - https://doc.qt.io/qt-5/qaction.html#details or in Sublime Text. But that’s a different issue, not completely related to your project. However, as a first step I think we should do as if these commands are nicely defined, as that will provide a path for further enhancement later on.

Hope that makes sense and please let me know if you have any questions.

2 Likes

Since there is not a bad amount of time at hand, it might make sense to rewrite the internal command system. This is of course only my opinion and its up to @laurent to decide.
If the command system is not properly defined, then the user will be limited in his ability to make new shortcuts, and also as long as the internal architecture is not sound anything on built on top of it will be essentially hackish. To quote @laurent,

One model to emulate might be the model that Atom or VS Code follows, defining hierarchial commands like core.editor.file.new. This is especially good since these can be generated from very maintainable json files. Also these projects being written in Electron will provide a good model to base the code off.

@anjulalk, what do you think?

Hi @laurent,

I was originally proposing to use the existing Setting.js model. The plan would be to store each shortcut as a setting. I was thinking of accessing the setting value by using something like Setting.value('newNoteShortcut'), and the label is the setting label which would be shown in the Options panel.

However, after considering your suggestion and a discussion I had with my mentors on Discord, I’d like to use a data structure that is similar to what the CLI client uses. Basically, it’s a JSON file similar to CLI’s keymap.json, but with an extra attribute (like VSCode’s when) to provide a context. This extra attribute may have a value of global, noteEditor, etc. It’ll be helpful to verify that no conflicting shortcuts are having the same key combination within the same context.

We can implement a class abstracting the parsing/updating of the file. It would be the one responsible for providing keyboard shortcut accelerator strings. Ideally, the class should “know” all commands of Joplin, their labels, and their accelerator strings. All of these values shall be associated with Joplin’s commands like newNote, etc.

To handle the visual-end of the editor, we need to implement a config screen. It can make use of the class mentioned above to display and update keyboard shortcuts as needed.

What I have in mind is, we’d store a few shortcut “layouts” hard-coded as JSON files in Joplin’s source. The user can choose one of these layouts as the default layout. Changes made by the user would be stored in a JSON file located in their profile directory. These changes would override the shortcut keys from the default layout.

I feel like this would allow further enhancements in the future as well.

Let me know if you’d prefer this approach. If yes, I’ll work on a more detailed specification so we can clear up any issues that may arise.

Thanks!

What I have in mind is, we’d store a few shortcut “layouts” hard-coded as JSON files in Joplin’s source. The user can choose one of these layouts as the default layout

I wonder if that would be over-engineering it a bit? Because then we need to decide what to do when the user switches the default layout, and already have shortcuts setup. Should we clear their shortcuts? (probably not) Should we apply them on top of the new default layout? But then we have to deal with conflicts in case some shortcuts in the new default layout conflict with the previous user shortcuts. I would think it's best at least for the first version if you limit yourself to one layout.

As with custom CSS, I'd expect users can then share their keymap.json on the forum anyway, and if it turns out one layout is particularly popular, perhaps we can add it as default later on.

However, after considering your suggestion and a discussion I had with my mentors on Discord, I’d like to use a data structure that is similar to what the CLI client uses. Basically, it’s a JSON file similar to CLI’s keymap.json , but with an extra attribute (like VSCode’s when ) to provide a context.

That's a good idea to add this concept, although I think it would be only a hint at the moment? Because the way commands are handled at the moment, the context is hard-coded. For example there's a doCommand method in the header, note list, note editor, etc. - this is the context. In other words, even if you create a shortcut { type: "newNote", when: "config" } it doesn't mean creating a new note from the config screen will work because the doCommand call won't be there.

But adding the context to the shortcut object will be useful later on, once we start refactoring the command system. Or do you have something more in mind, and if so how will you use this when parameter?

1 Like

This is planned but I prefer if it's done separately to keep things manageable. I think Anjula's proposal is flexible enough that it will allow doing the command refactoring later on.

I agree. It would make sense to focus on one layout first. :blush:

It would be a hint, and a way of making sure that there are no conflicting shortcuts within a context. I think something like this would be useful for the shortcut editor, so we can alert the user in case of a conflict.

Apologies if I wasn't being clear. My idea of context is where the user is focused at the moment. i.e: Note body, sidebar, note list, etc.

It's okay to have the same key combination as long as they are different in context. But if there are conflicting shortcuts within the same context, we can alert the user.

This conflict handling functionality is the reason I've thought of having such attribute.

1 Like

Indeed that would be useful for this too :+1:

1 Like

Glad to hear this. Hoping this gets implemented in the best way possible. :love_you_gesture:

2 Likes