Custom shortcuts on commands

[bjorn]

While customizable shortcuts in general would be nice (#215), there is a special case for commands. Users may set up an arbitrary number of them, but at the moment there is only one hardcoded shortcut (F5) for running the first enabled one.

It would be nice if custom shortcuts could be set in the Edit Commands dialog. For this, a separate (not in the table view) QKeySequenceEdit could be added. These shortcuts then need to be registered on the QMainWindow, probably by instantiating QShortcut instances that will map to the custom commands.

http://discourse.mapeditor.org/t/more-shortcuts-for-commands/2082

[ketanhwr]

@bjorn, I've started working on this. Anything I should know beforehand?

[bjorn]

@ketanhwr You can interface with the commands using the CommandDataModel and you can find most of the UI functionality in the CommandButton. I think maybe the CommandDataModel should be made available as a singleton, so that the MainWindow can manage a list of QShortcut instances based on it.

[ketanhwr]

Alright, I'll take a look into the flow and try to complete this one :)

[ketanhwr]

So should I add another column in the CommandTreeView for the shortcut of that command and then register that command into the main window?

[bjorn]

So should I add another column in the CommandTreeView for the shortcut of that command and then register that command into the main window?

It can be added separately, in which case it would apply to the currently selected row. Ideally, I think the table should be just a list with the names of the commands with the checkboxes for toggling whether they are active (and possible a column for displaying the shortcut), and all editable details should go in a form to the right of it. This would also make room for adding stuff like editable environment variables or working directory (#941).

For this task, it's enough to add the QKeySequenceEdit along with a label like "Shortcut:" below the table view.

[ketanhwr]

What's wrong with this?

[bjorn]

@ketanhwr It's not scalable in terms of adding more configuration to each command. Though if you prefer, feel free to implement the feature like that for now. When done this way, you'd need to instantiate a QKeySequenceEdit as the widget used for editing cells in the last column. If you're lucky this happens automatically based on the data type, but I don't really expect that to be the case.

[ketanhwr]

Then what exactly do you suggest? I'm not able to understand what you are asking for (the label one).

[bjorn]

Then what exactly do you suggest? I'm not able to understand what you are asking for (the label one).

I was just suggesting to add these widgets right below the table, in a horizontal layout:

QLabel "Shortcut:", QKeySequenceEdit

What I mean about scalability in terms of adding more configuration, what I'm looking for eventually is something that looks more like the External Tools in Qt Creator:

It can stay a lot simpler in Tiled, but in general I think it should use the same arrangement to make space for extra options (the "Save map before executing" option could then also easily be made per-command for example).

[ketanhwr]

Alright, so here's what I have in my mind right now:

• Just like you suggested, there will be a QLabel along with a QKeySequenceEdit just below the CommandTreeView which will display the shortcut for the currently selected command.
• Change ExtendedSelection to SingleSelection for the CommandTreeView so that we can configure shortcut of each command separately.
• When the user selects a particular command, the shortcut for that particular command is displayed in the QKeySequenceEdit
• Initially, only the first command will have F5 as the shortcut and other commands won't have any. The user can edit them.

Is this flow alright? I'm currently facing trouble for registering the shortcut each time it is changed. I'll try to make a pull request quickly!

[bjorn]

Is this flow alright? I'm currently facing trouble for registering the shortcut each time it is changed. I'll try to make a pull request quickly!

I think it's mostly fine. I don't think you need to change the selection from ExtendedSelection, since the QTreeView has the notion of "current index" that you can use for the shortcut widget. Also, if you keep the Command menu that you're working on updated with the shortcuts, you probably won't have to deal with manually registering the shortcuts since menu item shortcuts are registered automatically.

[ketanhwr]

Oh, I'll add this over that branch itself then 👍

[ketanhwr]

@bjorn if the user enters a shortcut which is already registered somewhere else, it would generate the Ambiguous Shortcut error. What should I do to prevent this? Display another dialog that this shortcut is already registered somewhere else?