Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions framework/diagnostics/internal/diagnosticsactions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ const UiActionList DiagnosticsActions::m_actions = {
TranslatableString("action", "Show &actions list"),
TranslatableString("action", "Show actions list")
),
UiAction("diagnostic-show-rcommands",
muse::ui::UiCtxAny,
muse::shortcuts::CTX_DISABLED,
TranslatableString("action", "Show &rcommands list"),
TranslatableString("action", "Show rcommands list")
),
UiAction("action://diagnostic/actions/query",
muse::ui::UiCtxAny,
muse::shortcuts::CTX_DISABLED,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ static const muse::UriQuery ENGRAVING_ELEMENTS_URI("musescore://diagnostics/engr
static const muse::UriQuery ENGRAVING_UNDOSTACK_URI("musescore://diagnostics/engraving/undostack?modal=false&floating=true");
static const muse::UriQuery ENGRAVING_STYLE_URI("musescore://diagnostics/engraving/style?modal=false&floating=true");
static const muse::UriQuery ACTIONS_LIST_URI("muse://diagnostics/actions/list?modal=false&floating=true");
static const muse::UriQuery RCOMMAND_LIST_URI("muse://diagnostics/rcommand/list?modal=false&floating=true");

void DiagnosticsActionsController::init()
{
Expand All @@ -55,6 +56,7 @@ void DiagnosticsActionsController::init()
dispatcher()->reg(this, "diagnostic-show-engraving-style", [this]() { openUri(ENGRAVING_STYLE_URI, false); });
dispatcher()->reg(this, "diagnostic-save-diagnostic-files", this, &DiagnosticsActionsController::saveDiagnosticFiles);
dispatcher()->reg(this, "diagnostic-show-actions", [this]() { openUri(ACTIONS_LIST_URI); });
dispatcher()->reg(this, "diagnostic-show-rcommands", [this]() { openUri(RCOMMAND_LIST_URI); });

dispatcher()->reg(this, ActionQuery("action://diagnostic/actions/query"), this, &DiagnosticsActionsController::onActionQuery);
dispatcher()->reg(this, ActionQuery("action://diagnostic/actions/query_params1"), this, &DiagnosticsActionsController::onActionQuery);
Expand Down
18 changes: 12 additions & 6 deletions framework/rcommand/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
# SPDX-License-Identifier: GPL-3.0-only
# MuseScore-Studio-CLA-applies
# MuseScore/Audacity CLA applies
#
# MuseScore Studio
# Music Composition & Notation
#
# Copyright (C) 2026 MuseScore Limited and others
# Copyright (C) MuseScore/Audacity and others
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License version 3 as
Expand All @@ -24,16 +21,25 @@ target_sources(muse_rcommand PRIVATE
rcommandmodule.cpp
rcommandmodule.h
icommanddispatcher.h
imodulecommands.h
icommandsregister.h
imodulecommandsregister.h
icommandsstate.h
imodulecommandsstate.h
commandable.h
commandtypes.h

internal/commanddispatcher.cpp
internal/commanddispatcher.h
internal/commandsregister.cpp
internal/commandsregister.h
internal/commandsstate.cpp
internal/commandsstate.h
)

if (MUSE_MODULE_RCOMMAND_QML)
add_subdirectory(qml/Muse/RCommand)
endif()
Comment on lines +39 to +41

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "1) Locate definitions/defaults/usages of MUSE_MODULE_RCOMMAND_QML"
rg -n "MUSE_MODULE_RCOMMAND_QML" -g 'CMakeLists.txt' -g '*.cmake' -g 'CMakePresets.json' -g '*.json'

echo
echo "2) Locate unconditional rcommands diagnostics wiring"
rg -n -C2 "diagnostic-show-rcommands|RCOMMAND_LIST_URI|muse://diagnostics/rcommand/list"

echo
echo "3) Check whether diagnostics code is conditionally compiled with the same flag"
rg -n "target_compile_definitions\\(|add_definitions\\(|MUSE_MODULE_RCOMMAND_QML" -g 'CMakeLists.txt' -g '*.cmake'

Repository: musescore/muse_framework

Length of output: 15643


🏁 Script executed:

# Check rcommandmodule.cpp for conditional compilation around the URI registration
cat -n framework/rcommand/rcommandmodule.cpp | head -60

Repository: musescore/muse_framework

Length of output: 2292


🏁 Script executed:

# Check diagnosticsactionscontroller.cpp for conditional compilation around RCOMMAND_LIST_URI
cat -n framework/diagnostics/internal/diagnosticsactionscontroller.cpp | head -70

Repository: musescore/muse_framework

Length of output: 4685


🏁 Script executed:

# Check diagnosticsactions.cpp for conditional compilation around the UiAction
cat -n framework/diagnostics/internal/diagnosticsactions.cpp | head -110

Repository: musescore/muse_framework

Length of output: 5421


Gate the rcommands diagnostic flow behind the same QML build flag.

The MUSE_MODULE_RCOMMAND_QML flag gates only the QML subdirectory build (framework/rcommand/qml/), but the diagnostics action handler and URI registration in framework/rcommand/rcommandmodule.cpp, framework/diagnostics/internal/diagnosticsactionscontroller.cpp, and framework/diagnostics/internal/diagnosticsactions.cpp are unconditional. When MUSE_MODULE_RCOMMAND_QML is OFF, the "diagnostic-show-rcommands" action will attempt to open a URI pointing to a QML dialog that was never compiled, creating a dead action path.

Wrap the URI registration in rcommandmodule.cpp and the action definition/handler in diagnosticsactionscontroller.cpp and diagnosticsactions.cpp with #if MUSE_MODULE_RCOMMAND_QML guards, or ensure the flag is always ON in build configurations.

🤖 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/CMakeLists.txt` around lines 39 - 41, The
MUSE_MODULE_RCOMMAND_QML build flag currently only controls the QML subdirectory
compilation, but the diagnostics action handler and URI registration that depend
on the QML resources are unconditional. This creates a dead action path when the
flag is OFF since the diagnostic-show-rcommands action tries to open a QML URI
that was never compiled. Add preprocessor guards using `#if`
MUSE_MODULE_RCOMMAND_QML and `#endif` around the URI registration in
rcommandmodule.cpp, the action definition in diagnosticsactions.cpp, and the
action handler in diagnosticsactionscontroller.cpp to ensure these diagnostic
components are only built when the QML module is enabled.


#if (MUSE_MODULE_RCOMMAND_TESTS)
# add_subdirectory(tests)
#endif()
21 changes: 21 additions & 0 deletions framework/rcommand/commandtypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,27 @@ struct CommandInfo
Decoration decoration;
};

struct CommandState {
bool enabled = false;
bool checked = false;

CommandState() = default;
CommandState(bool enabled, bool checked)
: enabled(enabled), checked(checked) {}
CommandState(bool enabled)
: enabled(enabled), checked(false) {}

bool operator==(const CommandState& other) const
{
return enabled == other.enabled && checked == other.checked;
}

bool operator!=(const CommandState& other) const
{
return !this->operator==(other);
}
};

// Call

using CallId = uint64_t;
Expand Down
5 changes: 5 additions & 0 deletions framework/rcommand/icommanddispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,10 @@ class ICommandDispatcher : MODULE_CONTEXT_INTERFACE
{
return dispatch(make_request(query));
}

async::Promise<Response> dispatch(const Command& query)
{
return dispatch(CommandQuery(query));
}
};
}
11 changes: 8 additions & 3 deletions framework/rcommand/icommandsregister.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
#pragma once

#include "modularity/imoduleinterface.h"
#include "imodulecommands.h"

#include "imodulecommandsregister.h"
#include "commandtypes.h"

namespace muse::rcommand {
class ICommandsRegister : MODULE_GLOBAL_INTERFACE
Expand All @@ -29,9 +31,12 @@ class ICommandsRegister : MODULE_GLOBAL_INTERFACE
public:
virtual ~ICommandsRegister() = default;

virtual void reg(const IModuleCommandsPtr& module) = 0;
virtual void unreg(const IModuleCommandsPtr& module) = 0;
virtual void reg(const IModuleCommandsRegisterPtr& module) = 0;
virtual void unreg(const IModuleCommandsRegisterPtr& module) = 0;
virtual IModuleCommandsRegisterPtr moduleRegister(const std::string& moduleName) const = 0;

virtual std::vector<CommandInfo> commandList() const = 0;

virtual const std::string& commandModuleName(const Command& command) const = 0;
};
}
43 changes: 43 additions & 0 deletions framework/rcommand/icommandsstate.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* SPDX-License-Identifier: GPL-3.0-only
* MuseScore/Audacity CLA applies
*
* Copyright (C) MuseScore/Audacity and others
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/

#pragma once

#include "modularity/imoduleinterface.h"

#include "global/async/channel.h"

#include "imodulecommandsstate.h"
#include "commandtypes.h"

namespace muse::rcommand {
class ICommandsState : MODULE_CONTEXT_INTERFACE
{
INTERFACE_ID(ICommandsState);

public:
virtual ~ICommandsState() = default;

virtual void reg(const IModuleCommandsStatePtr& module) = 0;
virtual void unreg(const IModuleCommandsStatePtr& module) = 0;

virtual CommandState commandState(const Command& command) const = 0;
virtual async::Channel<Command, CommandState> commandStateChanged() const = 0;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/

#pragma once
#pragma once

#include <string>
#include <vector>
Expand All @@ -26,15 +26,17 @@
#include "commandtypes.h"

namespace muse::rcommand {
class IModuleCommands
class IModuleCommandsRegister
{
public:

virtual ~IModuleCommands() = default;
virtual ~IModuleCommandsRegister() = default;

virtual std::string moduleName() const = 0;
virtual const std::vector<CommandInfo>& commandInfos() const = 0;

virtual const std::vector<Command>& commandList() const = 0;
virtual const std::vector<CommandInfo>& commandInfoList() const = 0;
};

using IModuleCommandsPtr = std::shared_ptr<IModuleCommands>;
using IModuleCommandsRegisterPtr = std::shared_ptr<IModuleCommandsRegister>;
}
45 changes: 45 additions & 0 deletions framework/rcommand/imodulecommandsstate.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* SPDX-License-Identifier: GPL-3.0-only
* MuseScore/Audacity CLA applies
*
* Copyright (C) MuseScore/Audacity and others
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/

#pragma once

#include <string>
#include <memory>

#include "global/async/channel.h"

#include "commandtypes.h"

namespace muse::rcommand {
class IModuleCommandsState
{
public:
virtual ~IModuleCommandsState() = default;

virtual std::string moduleName() const = 0;

virtual void init() = 0;
virtual void deinit() = 0;

virtual CommandState commandState(const Command& command) const = 0;
virtual async::Channel<Command, CommandState> commandStateChanged() const = 0;
};

using IModuleCommandsStatePtr = std::shared_ptr<IModuleCommandsState>;
}
35 changes: 32 additions & 3 deletions framework/rcommand/internal/commandsregister.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
using namespace muse;
using namespace muse::rcommand;

void CommandsRegister::reg(const IModuleCommandsPtr& module)
void CommandsRegister::reg(const IModuleCommandsRegisterPtr& module)
{
IF_ASSERT_FAILED(module) {
return;
Expand All @@ -41,9 +41,13 @@ void CommandsRegister::reg(const IModuleCommandsPtr& module)
}

m_modules[moduleName] = module;

for (const auto& info : module->commandInfoList()) {
m_commandModuleNames[info.command] = moduleName;
}
Comment on lines +45 to +47

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve command ownership when duplicate command IDs are present.

On Line 45–Line 47, ownership is overwritten unconditionally; on Line 63–Line 65, ownership is erased unconditionally. If two modules expose the same Command, unregistering one can remove the other module’s mapping and break commandModuleName() resolution.

Suggested fix
 void CommandsRegister::reg(const IModuleCommandsRegisterPtr& module)
 {
@@
-    for (const auto& info : module->commandInfoList()) {
-        m_commandModuleNames[info.command] = moduleName;
-    }
+    for (const auto& info : module->commandInfoList()) {
+        auto [ownerIt, inserted] = m_commandModuleNames.emplace(info.command, moduleName);
+        if (!inserted && ownerIt->second != moduleName) {
+            LOGW() << "command already registered by another module: " << info.command.toString()
+                   << ", current owner: " << ownerIt->second
+                   << ", attempted owner: " << moduleName;
+        }
+    }
 }
@@
 void CommandsRegister::unreg(const IModuleCommandsRegisterPtr& module)
 {
@@
-    for (const auto& info : module->commandInfoList()) {
-        m_commandModuleNames.erase(info.command);
-    }
+    for (const auto& info : module->commandInfoList()) {
+        auto ownerIt = m_commandModuleNames.find(info.command);
+        if (ownerIt != m_commandModuleNames.end() && ownerIt->second == moduleName) {
+            m_commandModuleNames.erase(ownerIt);
+        }
+    }
 }

Also applies to: 63-65

🤖 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/internal/commandsregister.cpp` around lines 45 - 47, The
command registration loop iterates through module->commandInfoList() and
unconditionally overwrites m_commandModuleNames entries, while the corresponding
unregistration code unconditionally erases them. This causes issues when
multiple modules register the same command ID—unregistering one module
incorrectly removes the mapping for the other. Fix this by adding a conditional
check in the registration loop (lines 45-47) to only insert the
command-to-module mapping if the command ID does not already exist in
m_commandModuleNames, and similarly in the unregistration section (lines 63-65)
only erase the mapping if it actually belongs to the module being unregistered.

}

void CommandsRegister::unreg(const IModuleCommandsPtr& module)
void CommandsRegister::unreg(const IModuleCommandsRegisterPtr& module)
{
IF_ASSERT_FAILED(module) {
return;
Expand All @@ -55,14 +59,39 @@ void CommandsRegister::unreg(const IModuleCommandsPtr& module)
}

m_modules.erase(moduleName);

for (const auto& info : module->commandInfoList()) {
m_commandModuleNames.erase(info.command);
}
}

IModuleCommandsRegisterPtr CommandsRegister::moduleRegister(const std::string& moduleName) const
{
auto it = m_modules.find(moduleName);
if (it != m_modules.end()) {
return it->second;
}

return nullptr;
}

std::vector<CommandInfo> CommandsRegister::commandList() const
{
std::vector<CommandInfo> commands;
for (const auto& module : m_modules) {
const auto& infos = module.second->commandInfos();
const auto& infos = module.second->commandInfoList();
commands.insert(commands.end(), infos.begin(), infos.end());
}
return commands;
}

const std::string& CommandsRegister::commandModuleName(const Command& command) const
{
auto it = m_commandModuleNames.find(command);
if (it != m_commandModuleNames.end()) {
return it->second;
}

static const std::string empty;
return empty;
}
12 changes: 9 additions & 3 deletions framework/rcommand/internal/commandsregister.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,17 @@ class CommandsRegister : public ICommandsRegister
public:
CommandsRegister() = default;

void reg(const IModuleCommandsPtr& module) override;
void unreg(const IModuleCommandsPtr& module) override;
void reg(const IModuleCommandsRegisterPtr& module) override;
void unreg(const IModuleCommandsRegisterPtr& module) override;
IModuleCommandsRegisterPtr moduleRegister(const std::string& moduleName) const override;

std::vector<CommandInfo> commandList() const override;

const std::string& commandModuleName(const Command& command) const override;

private:
std::map<std::string, IModuleCommandsPtr> m_modules;
std::map<std::string, IModuleCommandsRegisterPtr> m_modules;

std::map<Command, std::string> m_commandModuleNames;
};
}
Loading