-
Notifications
You must be signed in to change notification settings - Fork 28
added commands diagnostic #90
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 |
|---|---|---|
| @@ -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>; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 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 |
||
| } | ||
|
|
||
| void CommandsRegister::unreg(const IModuleCommandsPtr& module) | ||
| void CommandsRegister::unreg(const IModuleCommandsRegisterPtr& module) | ||
| { | ||
| IF_ASSERT_FAILED(module) { | ||
| return; | ||
|
|
@@ -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; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: musescore/muse_framework
Length of output: 15643
🏁 Script executed:
Repository: musescore/muse_framework
Length of output: 2292
🏁 Script executed:
Repository: musescore/muse_framework
Length of output: 4685
🏁 Script executed:
Repository: musescore/muse_framework
Length of output: 5421
Gate the rcommands diagnostic flow behind the same QML build flag.
The
MUSE_MODULE_RCOMMAND_QMLflag gates only the QML subdirectory build (framework/rcommand/qml/), but the diagnostics action handler and URI registration inframework/rcommand/rcommandmodule.cpp,framework/diagnostics/internal/diagnosticsactionscontroller.cpp, andframework/diagnostics/internal/diagnosticsactions.cppare unconditional. WhenMUSE_MODULE_RCOMMAND_QMLis 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.cppand the action definition/handler indiagnosticsactionscontroller.cppanddiagnosticsactions.cppwith#if MUSE_MODULE_RCOMMAND_QMLguards, or ensure the flag is always ON in build configurations.🤖 Prompt for AI Agents