Skip to content

fixed nav shorcuts (added compat map) #143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
igorkorsukov merged 1 commit into from
Jul 10, 2026
Merged

fixed nav shorcuts (added compat map) #143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion framework/actions/internal/actionsdispatcher.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ void ActionsDispatcher::dispatch(const ActionCode& actionCode, const ActionData&

void ActionsDispatcher::dispatch(const ActionQuery& actionQuery)
{
if (actionQuery.uri().scheme() == rcommand::COMMAND_SCHEME) {
if (muse::strings::startsWith(actionQuery.uri().scheme(), rcommand::COMMAND_SCHEME)) {

@coderabbitai coderabbitai Bot Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Prefer == over startsWith for exact scheme comparison.

actionQuery.uri().scheme() returns the parsed scheme component (e.g., "command"). Using startsWith instead of equality means URIs with schemes like "commander" or "commands" would also match and be routed to the command dispatcher. An exact equality check is more precise and prevents unintended scheme matches.

🔧 Proposed fix 
-    if (muse::strings::startsWith(actionQuery.uri().scheme(), rcommand::COMMAND_SCHEME)) {
+    if (actionQuery.uri().scheme() == rcommand::COMMAND_SCHEME) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (muse::strings::startsWith(actionQuery.uri().scheme(), rcommand::COMMAND_SCHEME)) {
if (actionQuery.uri().scheme() == rcommand::COMMAND_SCHEME) {
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@framework/actions/internal/actionsdispatcher.cpp` at line 81, Replace the
startsWith check in the action dispatch logic with an exact equality comparison
between actionQuery.uri().scheme() and rcommand::COMMAND_SCHEME, ensuring only
the intended command scheme is routed to the command dispatcher.

commandDispatcher()->dispatch(actionQuery);
return;
}
Expand Down
2 changes: 1 addition & 1 deletion framework/diagnostics/qml/Muse/Diagnostics/DiagnosticActionsPanel.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ Rectangle {

FlatButton {
anchors.verticalCenter: parent.verticalCenter
text: "Print"
text: "Print this"
onClicked: actionsModel.print()
Comment on lines +72 to 73

@coderabbitai coderabbitai Bot Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Align the label with the print scope.

actionsModel.print() prints all current actions, so “Print this” misleadingly suggests that only the selected item is printed. Use “Print” or “Print all” instead.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@framework/diagnostics/qml/Muse/Diagnostics/DiagnosticActionsPanel.qml` around
lines 72 - 73, Update the button label associated with actionsModel.print() from
“Print this” to “Print” or “Print all” so it accurately reflects that all
current actions are printed.

}
}
Expand Down
2 changes: 1 addition & 1 deletion framework/rcommand/commandtypes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
#include "ui/uitypes.h"

namespace muse::rcommand {
constexpr std::string_view COMMAND_SCHEME = "command://";
constexpr std::string_view COMMAND_SCHEME = "command";

@coderabbitai coderabbitai Bot Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

COMMAND_SCHEME change from "command://" to "command" broadens startsWith matching in ActionsDispatcher::dispatch(const ActionCode&).

In actionsdispatcher.cpp line 64, startsWith(actionCode, rcommand::COMMAND_SCHEME) is applied to the raw action code. With the old value "command://", only action codes beginning with "command://" matched. With the new value "command", any action code starting with the substring "command" (e.g., "commando-action", "command-edit") will be incorrectly routed to commandDispatcher() instead of being looked up among registered action clients.

This is a cross-file contract break: the constant value change in commandtypes.h silently broadens the dispatch routing in actionsdispatcher.cpp line 64, potentially hijacking action codes that were previously handled by registered clients.

Consider either:

  1. Using == on line 81 (scheme comparison) and restoring a delimited prefix (e.g., "command://" or "command:") for the startsWith check on line 64.
  2. Or introducing a separate constant (e.g., COMMAND_URI_PREFIX = "command://") for the raw-string startsWith check on line 64, keeping COMMAND_SCHEME = "command" for scheme comparisons.
🔧 Proposed fix: separate scheme and URI prefix constants 
 constexpr std::string_view COMMAND_SCHEME = "command";
+constexpr std::string_view COMMAND_URI_PREFIX = "command://";
 using Command = Uri;

Then in actionsdispatcher.cpp line 64:

-    if (muse::strings::startsWith(actionCode, rcommand::COMMAND_SCHEME)) {
+    if (muse::strings::startsWith(actionCode, rcommand::COMMAND_URI_PREFIX)) {

And line 81:

-    if (muse::strings::startsWith(actionQuery.uri().scheme(), rcommand::COMMAND_SCHEME)) {
+    if (actionQuery.uri().scheme() == rcommand::COMMAND_SCHEME) {
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@framework/rcommand/commandtypes.h` at line 36, Prevent command dispatch from
matching unrelated action codes beginning with “command”. In
ActionsDispatcher::dispatch(const ActionCode&), use a delimited URI prefix for
the raw startsWith check, such as a new COMMAND_URI_PREFIX constant set to
“command://”, while retaining COMMAND_SCHEME as “command” for parsed scheme
comparisons; update the relevant references in commandtypes.h and
actionsdispatcher.cpp consistently.

using Command = Uri;
using CommandQuery = UriQuery;

Expand Down
2 changes: 2 additions & 0 deletions framework/shortcuts/internal/shortcutscontroller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
*/
#include "shortcutscontroller.h"

#include "actions/actiontypes.h"
#include "log.h"

#define SHORTCUTS_DEBUG 1
Expand All @@ -34,6 +35,7 @@
using namespace muse::shortcuts;
using namespace muse::actions;
using namespace muse::rcommand;
using namespace muse::ui;

void ShortcutsController::init()
{
Expand Down
26 changes: 26 additions & 0 deletions framework/shortcuts/internal/shortcutsregister.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,17 @@
#include "global/io/file.h"
#include "global/serialization/xmlstreamreader.h"
#include "global/serialization/xmlstreamwriter.h"
#include "global/containers.h"

#include "multiwindows/resourcelockguard.h"
#include "ui/navigationcommands.h"

#include "log.h"

using namespace muse;
using namespace muse::shortcuts;
using namespace muse::async;
using namespace muse::ui;

static constexpr std::string_view SHORTCUTS_TAG("Shortcuts");
static constexpr std::string_view SHORTCUT_TAG("SC");
Expand All @@ -46,6 +49,25 @@ static constexpr std::string_view AUTOREPEAT_TAG("autorepeat");

static const std::string SHORTCUTS_RESOURCE_NAME("SHORTCUTS");

static const std::map<std::string, rcommand::Command> compatActionToCommand = {
{ "nav-next-section", NEXT_SECTION_COMMAND },
{ "nav-prev-section", PREV_SECTION_COMMAND },
{ "nav-next-panel", NEXT_PANEL_COMMAND },
{ "nav-prev-panel", PREV_PANEL_COMMAND },
{ "nav-next-tab", NEXT_PANEL_COMMAND },
{ "nav-prev-tab", PREV_PANEL_COMMAND },
{ "nav-trigger-control", TRIGGER_CONTROL_COMMAND },
{ "nav-right", RIGHT_COMMAND },
{ "nav-left", LEFT_COMMAND },
{ "nav-up", UP_COMMAND },
{ "nav-down", DOWN_COMMAND },
{ "nav-escape", ESCAPE_COMMAND },
{ "nav-first-control", FIRST_CONTROL_COMMAND },
{ "nav-last-control", LAST_CONTROL_COMMAND },
{ "nav-nextrow-control", NEXTROW_CONTROL_COMMAND },
{ "nav-prevrow-control", PREVROW_CONTROL_COMMAND },
};

static const std::map<QKeySequence::StandardKey, QKeyCombination> SHORTCUTS_EXPAND_IGNORE_MAP = {
{ QKeySequence::StandardKey::HelpContents, Qt::Key_Help },
{ QKeySequence::StandardKey::Open, Qt::Key_Open },
Expand Down Expand Up @@ -324,6 +346,10 @@ Shortcut ShortcutsRegister::readShortcut(XmlStreamReader& reader) const

if (tag == ACTION_CODE_TAG) {
shortcut.action = reader.readAsciiText();
const rcommand::Command& command = muse::value(compatActionToCommand, shortcut.action);
if (command.isValid()) {
shortcut.action = command.toString();
}
} else if (tag == STANDARD_KEY_TAG) {
shortcut.standardKey = QKeySequence::StandardKey(reader.readInt());
} else if (tag == SEQUENCE_TAG) {
Expand Down
18 changes: 10 additions & 8 deletions framework/ui/api/navigationapi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
*/
#include "navigationapi.h"

#include "navigationcommands.h"

#include "log.h"

using namespace muse::api;
Expand All @@ -37,37 +39,37 @@ NavigationApi::~NavigationApi()

void NavigationApi::nextPanel()
{
dispatcher()->dispatch("nav-next-panel");
dispatcher()->dispatch(NEXT_PANEL_COMMAND);
}

void NavigationApi::prevPanel()
{
dispatcher()->dispatch("nav-prev-panel");
dispatcher()->dispatch(PREV_PANEL_COMMAND);
}

void NavigationApi::right()
{
dispatcher()->dispatch("nav-right");
dispatcher()->dispatch(RIGHT_COMMAND);
}

void NavigationApi::left()
{
dispatcher()->dispatch("nav-left");
dispatcher()->dispatch(LEFT_COMMAND);
}

void NavigationApi::up()
{
dispatcher()->dispatch("nav-up");
dispatcher()->dispatch(UP_COMMAND);
}

void NavigationApi::down()
{
dispatcher()->dispatch("nav-down");
dispatcher()->dispatch(DOWN_COMMAND);
}

void NavigationApi::escape()
{
dispatcher()->dispatch("nav-escape");
dispatcher()->dispatch(ESCAPE_COMMAND);
}

bool NavigationApi::goToControl(const QString& section, const QString& panel, const QJSValue& controlNameOrIndex)
Expand Down Expand Up @@ -95,7 +97,7 @@ bool NavigationApi::goToControl(const QString& section, const QString& panel, co

void NavigationApi::trigger()
{
dispatcher()->dispatch("nav-trigger-control");
dispatcher()->dispatch(TRIGGER_CONTROL_COMMAND);
}

bool NavigationApi::triggerControl(const QString& section, const QString& panel, const QJSValue& controlNameOrIndex)
Expand Down
4 changes: 2 additions & 2 deletions framework/ui/api/navigationapi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@

#include "api/apiobject.h"
#include "modularity/ioc.h"
#include "actions/iactionsdispatcher.h"
#include "rcommand/icommanddispatcher.h"
#include "ui/inavigationcontroller.h"

namespace muse::api {
class NavigationApi : public api::ApiObject
{
Q_OBJECT

ContextInject<actions::IActionsDispatcher> dispatcher = { this };
ContextInject<rcommand::ICommandDispatcher> dispatcher = { this };
ContextInject<ui::INavigationController> navigation = { this };

public:
Expand Down