Skip to content

Issue #22767: Implement Unmute-all and Unsolo-all buttons to mixer#33798

Open
pombcoraft wants to merge 1 commit into
musescore:mainfrom
pombcoraft:unmute-unsolo-buttons
Open

Issue #22767: Implement Unmute-all and Unsolo-all buttons to mixer#33798
pombcoraft wants to merge 1 commit into
musescore:mainfrom
pombcoraft:unmute-unsolo-buttons

Conversation

@pombcoraft

Copy link
Copy Markdown
Contributor

Resolves: #22767

This commit adds a button to unmute all muted channels in the mixer and another to unsolo all soloed channels in the mixer. Removing the need to do these actions to each channel one by one.
It also adds a shortcut to unmute all (ctrl+shift+m) and a shortcut to unsolo all (ctrl+shift+l) following the same logic as the buttons.

  • I signed the CLA as misousa07:
  • The title of the PR describes the problem it addresses.
  • Each commit's message describes its purpose and effects, and references the issue it resolves. If changes are extensive, there is a sequence of easily reviewable commits.
  • The code in the PR follows the coding rules.
  • I understand all aspects of the code I'm contributing and I'm able to explain it if requested.
  • The code compiles and runs on my machine, preferably after each commit individually. I have manually tested and verified that my changes fulfil their intended purpose.
  • No prior attempts to resolve this problem exist, or if they do, I listed them in my PR description and described how I avoided repeating past mistakes.
  • There are no unnecessary changes.
  • I created a unit test or vtest to verify the changes I made (if applicable).

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds two mixer controls—unmute-all (Ctrl+Shift+U) and unsolo-all (Ctrl+Shift+L): keyboard shortcuts (standard, AZERTY, mac), PlaybackController methods and dispatcher registrations, UI actions, a new MixerUnMuteAndUnSoloSection QML component and MixerPanel layout integration, a MixerPanelContextMenuModel visibility property and signal, mixer-model synchronization for solo/mute updates, and a small tidy_file.sh tooling update.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning One out-of-scope change detected: modifications to tools/codestyle/tidy_file.sh regarding uncrustify configuration paths is unrelated to the mixer buttons feature. Remove the changes to tools/codestyle/tidy_file.sh or move them to a separate PR focused on build tooling updates.
Docstring Coverage ⚠️ Warning Docstring coverage is 5.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: implementing unmute-all and unsolo-all buttons for the mixer, which is the primary objective of this PR.
Description check ✅ Passed The description is complete and follows the template structure with signed CLA, all checkboxes checked (except the unit test checkbox), and clear explanation of changes including shortcuts.
Linked Issues check ✅ Passed The PR successfully implements both requested features from issue #22767: unmute-all and unsolo-all buttons in the mixer with corresponding keyboard shortcuts (Ctrl+Shift+U and Ctrl+Shift+L).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Linked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped musescore/muse_framework.git.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🤖 Prompt for all review comments with 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.

Inline comments:
In `@src/app/configs/data/shortcuts_azerty.xml`:
- Around line 863-867: The shortcut conflict: the <key>unmute-all</key> binding
uses Ctrl+Shift+M which duplicates the existing <key>toggle-mmrest</key>
binding; change the <seq> value for unmute-all (the <SC> entry with key
"unmute-all") to an unused combination (e.g., Ctrl+Shift+U) so it no longer
collides with the "toggle-mmrest" shortcut and update any related
documentation/tests referencing the old binding.

In `@src/app/configs/data/shortcuts_mac.xml`:
- Around line 822-826: The shortcut for the <key>unmute-all</key> action
conflicts with <key>toggle-mmrest</key> because both use
<seq>Ctrl+Shift+M</seq>; update the <seq> value inside the <SC> block for
<key>unmute-all</key> to the same alternative shortcut used in the AZERTY and
standard layouts (replace the current "Ctrl+Shift+M" string), ensuring the new
sequence matches the other layouts so the conflict is resolved and behavior is
consistent.

In `@src/app/configs/data/shortcuts.xml`:
- Around line 822-826: The new keyboard shortcut for the action with key
"unmute-all" currently uses the same chord as the existing "toggle-mmrest" (both
have <seq>Ctrl+Shift+M</seq>), causing a collision; pick a different,
non-conflicting <seq> value for either "unmute-all" or "toggle-mmrest" (e.g.,
change the <seq> under the SC entry for key "unmute-all" to an alternative
chord) and ensure the updated chord is unique across shortcuts in shortcuts.xml
so both actions remain reachable.

In `@src/playback/internal/playbackcontroller.cpp`:
- Around line 963-997: Add null-safety guards at the start of both
PlaybackController::unmuteAll() and PlaybackController::unsoloAll(): verify
notationPlayback() and m_notation (as used) are non-null (or use
IF_ASSERT_FAILED macros like other methods) and return early if the checks fail;
then proceed to call existingTrackIdSet(), m_notation->soloMuteState(),
trackSoloMuteState(), setTrackSoloMuteState(...) and updateSoloMuteStates() as
before. Ensure you reference the same null-check pattern used in
updateCurrentTempo() and seekElement() so behavior and logging/assertion are
consistent.
- Around line 983-997: The unsoloAll() implementation is inconsistent with
unmuteAll(): it must skip the metronome track like unmuteAll() does and also fix
local style issues; update PlaybackController::unsoloAll() to check and skip the
metronome (use the same check used in unmuteAll()), rename the local variable
new_state to newState to follow camelCase, remove the redundant "== true" so the
condition is simply if (newState.solo), and ensure spacing after keywords and
commas (e.g., if (newState.solo) and setTrackSoloMuteState(instrumentTrackId,
newState)); keep the call to updateSoloMuteStates() at the end.
- Around line 963-981: In PlaybackController::unmuteAll rename the local
variable new_state to newState (use camelCase), change the conditional from
if(new_state.mute == true) to if (newState.mute) (add a space after the if and
use the simpler boolean check), and ensure proper spacing in the call
setTrackSoloMuteState(instrumentTrackId,new_state) by adding a space after the
comma and updating the identifier to setTrackSoloMuteState(instrumentTrackId,
newState); keep the rest of the logic (notationPlayback(), metronomeTrackId(),
INotationSoloMuteState::SoloMuteState, updateSoloMuteStates()) unchanged.

In
`@src/playback/qml/MuseScore/Playback/internal/MixerUnMuteAndUnSoloSection.qml`:
- Around line 60-62: In MixerUnMuteAndUnSoloSection.qml remove uses of the
undefined content object (replace navigation.accessible.name with a direct
qsTrc("playback", "Unmute All") string), add on the root: property int
navigationRowStart: 0 and signal navigateControlIndexChanged(var index) so
root.navigationRowStart and the signal exist, and remove any other references to
content; then in MixerPanel.qml where the UnMuteAndUnSolo section is
instantiated, pass navigationRowStart (e.g. 650), add needReadChannelName:
prv.isPanelActivated, and wire the signal with onNavigateControlIndexChanged:
function(index) { prv.setNavigateControlIndex(index) } to match the pattern used
by soundSection and fxSection.
- Line 94: The component references an undefined identifier `content` (e.g.
navigation.panel: content.channelItem.panel and content.accessibleName); replace
those with direct, local properties: set the component's accessibleName to a
literal translation such as qsTrc("playback", "Mute and Solo controls") on the
root (MixerUnMuteAndUnSoloSection) and update navigation.panel to use the local
channelItem.panel (or another local panel property) instead of
content.channelItem.panel so no `content` identifier is required.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 14fe7254-aab5-4604-a415-71eb6a95f56d

📥 Commits

Reviewing files that changed from the base of the PR and between d53a8ed and 015dfbc.

📒 Files selected for processing (15)
  • src/app/configs/data/shortcuts.xml
  • src/app/configs/data/shortcuts_azerty.xml
  • src/app/configs/data/shortcuts_mac.xml
  • src/playback/internal/playbackconfiguration.cpp
  • src/playback/internal/playbackcontroller.cpp
  • src/playback/internal/playbackcontroller.h
  • src/playback/internal/playbackuiactions.cpp
  • src/playback/internal/playbackuiactions.h
  • src/playback/playbacktypes.h
  • src/playback/qml/MuseScore/Playback/CMakeLists.txt
  • src/playback/qml/MuseScore/Playback/MixerPanel.qml
  • src/playback/qml/MuseScore/Playback/internal/MixerUnMuteAndUnSoloSection.qml
  • src/playback/qml/MuseScore/Playback/mixerpanelcontextmenumodel.cpp
  • src/playback/qml/MuseScore/Playback/mixerpanelcontextmenumodel.h
  • src/playback/qml/MuseScore/Playback/mixerpanelmodel.cpp

Comment thread src/app/configs/data/shortcuts_azerty.xml
Comment thread src/app/configs/data/shortcuts_mac.xml
Comment thread src/app/configs/data/shortcuts.xml
Comment on lines +963 to +981
void PlaybackController::unmuteAll()
{
InstrumentTrackIdSet existingTrackIdSet = notationPlayback()->existingTrackIdSet();

for (const InstrumentTrackId& instrumentTrackId : existingTrackIdSet) {
if (instrumentTrackId == notationPlayback()->metronomeTrackId()) {
continue;
}

INotationSoloMuteState::SoloMuteState new_state = m_notation->soloMuteState()->trackSoloMuteState(instrumentTrackId);
if(new_state.mute == true) {
new_state.mute = false;

setTrackSoloMuteState(instrumentTrackId,new_state);
}
}

updateSoloMuteStates();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Code style issues in unmuteAll().

Several style inconsistencies:

  • Line 972: Variable new_state uses snake_case instead of camelCase (codebase convention)
  • Line 973: Redundant comparison if(new_state.mute == true) should be if (new_state.mute), plus missing space after if
  • Line 976: Missing space after comma in function call
♻️ Proposed fix for style issues
-        INotationSoloMuteState::SoloMuteState new_state = m_notation->soloMuteState()->trackSoloMuteState(instrumentTrackId);
-        if(new_state.mute == true) {
+        INotationSoloMuteState::SoloMuteState newState = m_notation->soloMuteState()->trackSoloMuteState(instrumentTrackId);
+        if (newState.mute) {
-            new_state.mute = false;
+            newState.mute = false;

-            setTrackSoloMuteState(instrumentTrackId,new_state);
+            setTrackSoloMuteState(instrumentTrackId, newState);
         }
🤖 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 `@src/playback/internal/playbackcontroller.cpp` around lines 963 - 981, In
PlaybackController::unmuteAll rename the local variable new_state to newState
(use camelCase), change the conditional from if(new_state.mute == true) to if
(newState.mute) (add a space after the if and use the simpler boolean check),
and ensure proper spacing in the call
setTrackSoloMuteState(instrumentTrackId,new_state) by adding a space after the
comma and updating the identifier to setTrackSoloMuteState(instrumentTrackId,
newState); keep the rest of the logic (notationPlayback(), metronomeTrackId(),
INotationSoloMuteState::SoloMuteState, updateSoloMuteStates()) unchanged.

Comment on lines +963 to +997
void PlaybackController::unmuteAll()
{
InstrumentTrackIdSet existingTrackIdSet = notationPlayback()->existingTrackIdSet();

for (const InstrumentTrackId& instrumentTrackId : existingTrackIdSet) {
if (instrumentTrackId == notationPlayback()->metronomeTrackId()) {
continue;
}

INotationSoloMuteState::SoloMuteState new_state = m_notation->soloMuteState()->trackSoloMuteState(instrumentTrackId);
if(new_state.mute == true) {
new_state.mute = false;

setTrackSoloMuteState(instrumentTrackId,new_state);
}
}

updateSoloMuteStates();
}

void PlaybackController::unsoloAll()
{
InstrumentTrackIdSet existingTrackIdSet = notationPlayback()->existingTrackIdSet();

for (const InstrumentTrackId& instrumentTrackId : existingTrackIdSet) {

INotationSoloMuteState::SoloMuteState new_state = trackSoloMuteState(instrumentTrackId);
if(new_state.solo == true) {
new_state.solo = false;
setTrackSoloMuteState(instrumentTrackId,new_state);
}
}

updateSoloMuteStates();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Missing null safety checks in unmuteAll() and unsoloAll().

Both methods call notationPlayback()->existingTrackIdSet() (Lines 965, 985) and m_notation->soloMuteState() (Line 972) without checking if notationPlayback() or m_notation are non-null. Other methods in this file (e.g., updateCurrentTempo() at Line 178, seekElement() at Line 425) guard these calls with IF_ASSERT_FAILED or early returns.

🛡️ Proposed fix to add guards
 void PlaybackController::unmuteAll()
 {
+    IF_ASSERT_FAILED(notationPlayback() && m_notation) {
+        return;
+    }
+
     InstrumentTrackIdSet existingTrackIdSet = notationPlayback()->existingTrackIdSet();
     ...
 }

 void PlaybackController::unsoloAll()
 {
+    IF_ASSERT_FAILED(notationPlayback()) {
+        return;
+    }
+
     InstrumentTrackIdSet existingTrackIdSet = notationPlayback()->existingTrackIdSet();
     ...
 }
📝 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
void PlaybackController::unmuteAll()
{
InstrumentTrackIdSet existingTrackIdSet = notationPlayback()->existingTrackIdSet();
for (const InstrumentTrackId& instrumentTrackId : existingTrackIdSet) {
if (instrumentTrackId == notationPlayback()->metronomeTrackId()) {
continue;
}
INotationSoloMuteState::SoloMuteState new_state = m_notation->soloMuteState()->trackSoloMuteState(instrumentTrackId);
if(new_state.mute == true) {
new_state.mute = false;
setTrackSoloMuteState(instrumentTrackId,new_state);
}
}
updateSoloMuteStates();
}
void PlaybackController::unsoloAll()
{
InstrumentTrackIdSet existingTrackIdSet = notationPlayback()->existingTrackIdSet();
for (const InstrumentTrackId& instrumentTrackId : existingTrackIdSet) {
INotationSoloMuteState::SoloMuteState new_state = trackSoloMuteState(instrumentTrackId);
if(new_state.solo == true) {
new_state.solo = false;
setTrackSoloMuteState(instrumentTrackId,new_state);
}
}
updateSoloMuteStates();
}
void PlaybackController::unmuteAll()
{
IF_ASSERT_FAILED(notationPlayback() && m_notation) {
return;
}
InstrumentTrackIdSet existingTrackIdSet = notationPlayback()->existingTrackIdSet();
for (const InstrumentTrackId& instrumentTrackId : existingTrackIdSet) {
if (instrumentTrackId == notationPlayback()->metronomeTrackId()) {
continue;
}
INotationSoloMuteState::SoloMuteState new_state = m_notation->soloMuteState()->trackSoloMuteState(instrumentTrackId);
if(new_state.mute == true) {
new_state.mute = false;
setTrackSoloMuteState(instrumentTrackId,new_state);
}
}
updateSoloMuteStates();
}
void PlaybackController::unsoloAll()
{
IF_ASSERT_FAILED(notationPlayback()) {
return;
}
InstrumentTrackIdSet existingTrackIdSet = notationPlayback()->existingTrackIdSet();
for (const InstrumentTrackId& instrumentTrackId : existingTrackIdSet) {
INotationSoloMuteState::SoloMuteState new_state = trackSoloMuteState(instrumentTrackId);
if(new_state.solo == true) {
new_state.solo = false;
setTrackSoloMuteState(instrumentTrackId,new_state);
}
}
updateSoloMuteStates();
}
🤖 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 `@src/playback/internal/playbackcontroller.cpp` around lines 963 - 997, Add
null-safety guards at the start of both PlaybackController::unmuteAll() and
PlaybackController::unsoloAll(): verify notationPlayback() and m_notation (as
used) are non-null (or use IF_ASSERT_FAILED macros like other methods) and
return early if the checks fail; then proceed to call existingTrackIdSet(),
m_notation->soloMuteState(), trackSoloMuteState(), setTrackSoloMuteState(...)
and updateSoloMuteStates() as before. Ensure you reference the same null-check
pattern used in updateCurrentTempo() and seekElement() so behavior and
logging/assertion are consistent.

Comment on lines +983 to +997
void PlaybackController::unsoloAll()
{
InstrumentTrackIdSet existingTrackIdSet = notationPlayback()->existingTrackIdSet();

for (const InstrumentTrackId& instrumentTrackId : existingTrackIdSet) {

INotationSoloMuteState::SoloMuteState new_state = trackSoloMuteState(instrumentTrackId);
if(new_state.solo == true) {
new_state.solo = false;
setTrackSoloMuteState(instrumentTrackId,new_state);
}
}

updateSoloMuteStates();
}

Copy link
Copy Markdown
Contributor

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

Inconsistent metronome handling between unmuteAll() and unsoloAll().

unmuteAll() (Line 968-970) explicitly skips the metronome track, but unsoloAll() does not. While the QML UI behavior mirrors this (see MixerUnMuteAndUnSoloSection.qml:48-109), the inconsistency suggests a design gap: should solo operations on the metronome be allowed at all?

Additionally, the same code-style issues apply here: snake_case naming, redundant bool comparison, missing space after if (Line 990), and missing space after comma (Line 992).

♻️ Proposed fix for consistency and style

Add metronome skip for symmetry and apply style fixes:

 void PlaybackController::unsoloAll()
 {
     InstrumentTrackIdSet existingTrackIdSet = notationPlayback()->existingTrackIdSet();

     for (const InstrumentTrackId& instrumentTrackId : existingTrackIdSet) {
-
-        INotationSoloMuteState::SoloMuteState new_state = trackSoloMuteState(instrumentTrackId);
-        if(new_state.solo == true) {
-            new_state.solo = false;
-            setTrackSoloMuteState(instrumentTrackId,new_state);
+        if (instrumentTrackId == notationPlayback()->metronomeTrackId()) {
+            continue;
+        }
+
+        INotationSoloMuteState::SoloMuteState newState = trackSoloMuteState(instrumentTrackId);
+        if (newState.solo) {
+            newState.solo = false;
+            setTrackSoloMuteState(instrumentTrackId, newState);
         }
     }
🤖 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 `@src/playback/internal/playbackcontroller.cpp` around lines 983 - 997, The
unsoloAll() implementation is inconsistent with unmuteAll(): it must skip the
metronome track like unmuteAll() does and also fix local style issues; update
PlaybackController::unsoloAll() to check and skip the metronome (use the same
check used in unmuteAll()), rename the local variable new_state to newState to
follow camelCase, remove the redundant "== true" so the condition is simply if
(newState.solo), and ensure spacing after keywords and commas (e.g., if
(newState.solo) and setTrackSoloMuteState(instrumentTrackId, newState)); keep
the call to updateSoloMuteStates() at the end.

Comment on lines +60 to +62
navigation.panel: content.channelItem.panel
navigation.row: root.navigationRowStart
navigation.accessible.name: content.accessibleName + " " + qsTrc("playback", "Unmute All")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Incomplete navigation/property contract across MixerUnMuteAndUnSoloSection.qml and MixerPanel.qml.

MixerUnMuteAndUnSoloSection.qml references content (undefined), root.navigationRowStart (not declared), and emits navigateControlIndexChanged (signal not declared). Meanwhile, MixerPanel.qml instantiates the section without providing navigationRowStart or handling onNavigateControlIndexChanged. The shared root cause is an incomplete copy-paste adaptation from an existing mixer section (likely MixerMuteAndSoloSection).

In MixerUnMuteAndUnSoloSection.qml:

  • Remove all references to content (lines 60–62, 94–96) and replace with a direct accessible-name string.
  • Declare property int navigationRowStart: 0 and signal navigateControlIndexChanged(var index) on root.

In MixerPanel.qml (lines 329–336):

  • Add navigationRowStart: <value> (e.g., 650), needReadChannelName: prv.isPanelActivated, and onNavigateControlIndexChanged: function(index) { prv.setNavigateControlIndex(index) }.

Match the pattern used by soundSection, fxSection, and the other mixer sections in MixerPanel.qml.

🤖 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 `@src/playback/qml/MuseScore/Playback/internal/MixerUnMuteAndUnSoloSection.qml`
around lines 60 - 62, In MixerUnMuteAndUnSoloSection.qml remove uses of the
undefined content object (replace navigation.accessible.name with a direct
qsTrc("playback", "Unmute All") string), add on the root: property int
navigationRowStart: 0 and signal navigateControlIndexChanged(var index) so
root.navigationRowStart and the signal exist, and remove any other references to
content; then in MixerPanel.qml where the UnMuteAndUnSolo section is
instantiated, pass navigationRowStart (e.g. 650), add needReadChannelName:
prv.isPanelActivated, and wire the signal with onNavigateControlIndexChanged:
function(index) { prv.setNavigateControlIndex(index) } to match the pattern used
by soundSection and fxSection.

Comment thread src/playback/qml/MuseScore/Playback/internal/MixerUnMuteAndUnSoloSection.qml Outdated
This commit adds a button to unmute all muted channels in the mixer and
another to unsolo all soloed channels in the mixer. Removing the need to
do these actions to each channel one by one.
It also adds a shortcut to unmute all (ctrl+shift+m) and
a shortcut to unsolo all (ctrl+shift+l) following the same logic as
the buttons.

Closes: musescore#22767

Co-authored-by: Joana Guia <joana.a.guia@tecnico.ulisboa.pt>
@pombcoraft pombcoraft force-pushed the unmute-unsolo-buttons branch from 015dfbc to 8b146a6 Compare June 12, 2026 18:40

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (3)
src/playback/internal/playbackcontroller.cpp (3)

968-970: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Inconsistent metronome handling between unmuteAll() and unsoloAll().

unmuteAll() explicitly skips the metronome track (lines 968-970 in src/playback/internal/playbackcontroller.cpp), but unsoloAll() (lines 987-993 in src/playback/internal/playbackcontroller.cpp) does not. For symmetry and to prevent unexpected behavior, both methods should handle the metronome consistently.

♻️ Proposed fix to add metronome skip in `unsoloAll()`
 void PlaybackController::unsoloAll()
 {
     IF_ASSERT_FAILED(notationPlayback() && m_notation) {
         return;
     }

     InstrumentTrackIdSet existingTrackIdSet = notationPlayback()->existingTrackIdSet();

     for (const InstrumentTrackId& instrumentTrackId : existingTrackIdSet) {
+        if (instrumentTrackId == notationPlayback()->metronomeTrackId()) {
+            continue;
+        }
+
         INotationSoloMuteState::SoloMuteState newState = trackSoloMuteState(instrumentTrackId);
         if (newState.solo) {
             newState.solo = false;
             setTrackSoloMuteState(instrumentTrackId, newState);
         }
     }

     updateSoloMuteStates();
 }
🤖 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 `@src/playback/internal/playbackcontroller.cpp` around lines 968 - 970,
unmuteAll() currently skips the metronome by checking if instrumentTrackId ==
notationPlayback()->metronomeTrackId(), but unsoloAll() does not; update
unsoloAll() to perform the same check and continue (skip) when the current
instrumentTrackId equals notationPlayback()->metronomeTrackId() so both methods
handle the metronome consistently. Locate the loop in unsoloAll() that iterates
instrumentTrackId (the same iteration context as in unmuteAll()) and add the
identical metronome skip condition using notationPlayback()->metronomeTrackId().

963-981: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Missing null safety checks and redundant boolean comparison.

Both issues flagged in previous review remain:

  1. No null checks for notationPlayback() (line 965) or m_notation (line 972) before dereferencing. Other methods in this file (e.g., updateCurrentTempo() at line 178, seekElement() at line 425) guard these calls.
  2. Line 973: Redundant == true comparison; prefer if (newState.mute).
🛡️ Proposed fix
 void PlaybackController::unmuteAll()
 {
+    IF_ASSERT_FAILED(notationPlayback() && m_notation) {
+        return;
+    }
+
     InstrumentTrackIdSet existingTrackIdSet = notationPlayback()->existingTrackIdSet();

     for (const InstrumentTrackId& instrumentTrackId : existingTrackIdSet) {
         if (instrumentTrackId == notationPlayback()->metronomeTrackId()) {
             continue;
         }

         INotationSoloMuteState::SoloMuteState newState = m_notation->soloMuteState()->trackSoloMuteState(instrumentTrackId);
-        if (newState.mute == true) {
+        if (newState.mute) {
             newState.mute = false;

             setTrackSoloMuteState(instrumentTrackId, newState);
         }
     }

     updateSoloMuteStates();
 }
🤖 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 `@src/playback/internal/playbackcontroller.cpp` around lines 963 - 981,
PlaybackController::unmuteAll lacks null-safety and uses a redundant boolean
comparison; before calling notationPlayback() or dereferencing m_notation, guard
with early returns (or null checks) similar to updateCurrentTempo()/seekElement
so you skip processing if notationPlayback() or m_notation is null, and replace
"if (newState.mute == true)" with "if (newState.mute)"; ensure you still skip
metronomeTrackId(), call setTrackSoloMuteState(instrumentTrackId, newState) and
finally call updateSoloMuteStates() only after the null checks.

983-996: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Missing null safety checks and redundant boolean comparison.

Same issues as unmuteAll():

  1. No null check for notationPlayback() (line 985) before calling existingTrackIdSet(). The call to trackSoloMuteState() at line 988 also dereferences m_notation (see line 327) without a guard.
  2. Line 989: Redundant == true comparison; prefer if (newState.solo).
🛡️ Proposed fix
 void PlaybackController::unsoloAll()
 {
+    IF_ASSERT_FAILED(notationPlayback() && m_notation) {
+        return;
+    }
+
     InstrumentTrackIdSet existingTrackIdSet = notationPlayback()->existingTrackIdSet();

     for (const InstrumentTrackId& instrumentTrackId : existingTrackIdSet) {
         INotationSoloMuteState::SoloMuteState newState = trackSoloMuteState(instrumentTrackId);
-        if (newState.solo == true) {
+        if (newState.solo) {
             newState.solo = false;
             setTrackSoloMuteState(instrumentTrackId, newState);
         }
     }

     updateSoloMuteStates();
 }
🤖 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 `@src/playback/internal/playbackcontroller.cpp` around lines 983 - 996,
PlaybackController::unsoloAll lacks null checks and uses a redundant boolean
comparison; guard calls to notationPlayback() (and any downstream methods that
dereference m_notation such as trackSoloMuteState) by returning early if
notationPlayback() is null, then iterate existingTrackIdSet; replace the
redundant "if (newState.solo == true)" with "if (newState.solo)"; keep calls to
setTrackSoloMuteState(instrumentTrackId, newState) and updateSoloMuteStates()
unchanged but only run them when notationPlayback() is valid.
🤖 Prompt for all review comments with 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.

Inline comments:
In `@src/playback/qml/MuseScore/Playback/MixerPanel.qml`:
- Around line 329-336: The MixerUnMuteAndUnSoloSection instance is missing
required navigation properties used internally (causing runtime errors); set the
navigation props on this instance: provide channelItem.panel by wiring
channelItem.panel: root.channelItem.panel, set navigationRowStart (e.g.
navigationRowStart: 650 to sit between the mute/solo and title sections),
forward accessibleName: accessibleName: root.accessibleName, and hook the
navigation signal with onNavigateControlIndexChanged:
root.navigateControlIndexChanged; also add needReadChannelName: true if the
component expects it. Ensure MixerUnMuteAndUnSoloSection.qml declares these
properties/signals if not already.

In `@src/playback/qml/MuseScore/Playback/mixerpanelcontextmenumodel.cpp`:
- Line 52: The string returned for MixerSectionType::UnMuteAndUnSolo uses
inconsistent capitalization ("Unmute and Unsolo"); update the TranslatableString
in the corresponding case in mixerpanelcontextmenumodel.cpp to use "Unmute and
unsolo" so it matches the lowercase "solo" used in the "Mute and solo" entry
(change the second word to lowercase "unsolo" in the TranslatableString call).

In `@tools/codestyle/tidy_file.sh`:
- Around line 18-19: The tidy_file.sh script currently sets
UNCRUSTIFY_CONFIG="${REPO_ROOT}/muse/tools/codestyle/uncrustify_muse.cfg" and
assumes the submodule file exists; update tidy_file.sh (and mention
hooks/pre-commit which calls it) to check for the existence of the
UNCRUSTIFY_CONFIG path and exit with a clear actionable message if missing
(e.g., instructing to init/update the muse_framework submodule or run a download
step), or implement an automatic fallback that downloads or copies the config
(reuse the logic/pattern from buildscripts/tools/codestyle/download.cmake)
before invoking uncrustify; reference the REPO_ROOT and UNCRUSTIFY_CONFIG
variables so the check/fallback is implemented using those symbols.

---

Duplicate comments:
In `@src/playback/internal/playbackcontroller.cpp`:
- Around line 968-970: unmuteAll() currently skips the metronome by checking if
instrumentTrackId == notationPlayback()->metronomeTrackId(), but unsoloAll()
does not; update unsoloAll() to perform the same check and continue (skip) when
the current instrumentTrackId equals notationPlayback()->metronomeTrackId() so
both methods handle the metronome consistently. Locate the loop in unsoloAll()
that iterates instrumentTrackId (the same iteration context as in unmuteAll())
and add the identical metronome skip condition using
notationPlayback()->metronomeTrackId().
- Around line 963-981: PlaybackController::unmuteAll lacks null-safety and uses
a redundant boolean comparison; before calling notationPlayback() or
dereferencing m_notation, guard with early returns (or null checks) similar to
updateCurrentTempo()/seekElement so you skip processing if notationPlayback() or
m_notation is null, and replace "if (newState.mute == true)" with "if
(newState.mute)"; ensure you still skip metronomeTrackId(), call
setTrackSoloMuteState(instrumentTrackId, newState) and finally call
updateSoloMuteStates() only after the null checks.
- Around line 983-996: PlaybackController::unsoloAll lacks null checks and uses
a redundant boolean comparison; guard calls to notationPlayback() (and any
downstream methods that dereference m_notation such as trackSoloMuteState) by
returning early if notationPlayback() is null, then iterate existingTrackIdSet;
replace the redundant "if (newState.solo == true)" with "if (newState.solo)";
keep calls to setTrackSoloMuteState(instrumentTrackId, newState) and
updateSoloMuteStates() unchanged but only run them when notationPlayback() is
valid.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c953c180-e1e3-4449-9ea7-4693a7e76a94

📥 Commits

Reviewing files that changed from the base of the PR and between 015dfbc and 8b146a6.

📒 Files selected for processing (16)
  • src/app/configs/data/shortcuts.xml
  • src/app/configs/data/shortcuts_azerty.xml
  • src/app/configs/data/shortcuts_mac.xml
  • src/playback/internal/playbackconfiguration.cpp
  • src/playback/internal/playbackcontroller.cpp
  • src/playback/internal/playbackcontroller.h
  • src/playback/internal/playbackuiactions.cpp
  • src/playback/internal/playbackuiactions.h
  • src/playback/playbacktypes.h
  • src/playback/qml/MuseScore/Playback/CMakeLists.txt
  • src/playback/qml/MuseScore/Playback/MixerPanel.qml
  • src/playback/qml/MuseScore/Playback/internal/MixerUnMuteAndUnSoloSection.qml
  • src/playback/qml/MuseScore/Playback/mixerpanelcontextmenumodel.cpp
  • src/playback/qml/MuseScore/Playback/mixerpanelcontextmenumodel.h
  • src/playback/qml/MuseScore/Playback/mixerpanelmodel.cpp
  • tools/codestyle/tidy_file.sh
✅ Files skipped from review due to trivial changes (1)
  • src/playback/qml/MuseScore/Playback/CMakeLists.txt
🚧 Files skipped from review as they are similar to previous changes (10)
  • src/playback/internal/playbackcontroller.h
  • src/app/configs/data/shortcuts_azerty.xml
  • src/app/configs/data/shortcuts_mac.xml
  • src/playback/playbacktypes.h
  • src/playback/internal/playbackuiactions.h
  • src/playback/internal/playbackconfiguration.cpp
  • src/playback/qml/MuseScore/Playback/mixerpanelcontextmenumodel.h
  • src/playback/qml/MuseScore/Playback/mixerpanelmodel.cpp
  • src/playback/internal/playbackuiactions.cpp
  • src/playback/qml/MuseScore/Playback/internal/MixerUnMuteAndUnSoloSection.qml

Comment on lines +329 to 336
MixerUnMuteAndUnSoloSection {
anchors.left: parent.left
anchors.verticalCenter: parent.verticalCenter
visible: contextMenuModel.unMuteAndUnSoloSectionVisible
headerVisible: contextMenuModel.labelsSectionVisible
headerWidth: prv.headerWidth
model: mixerPanelModel
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Missing required navigation properties causes runtime error and breaks keyboard navigation.

The MixerUnMuteAndUnSoloSection component references root.channelItem.panel, root.navigationRowStart, root.accessibleName, and root.navigateControlIndexChanged in its button navigation setup (see src/playback/qml/MuseScore/Playback/internal/MixerUnMuteAndUnSoloSection.qml lines 40, 42, 44, 68, 70, 72), but none of these properties are set here. Accessing undefined root.channelItem.panel will cause a QML runtime error when keyboard navigation activates. Additionally, the missing onNavigateControlIndexChanged handler breaks navigation tracking, and the missing navigationRowStart breaks keyboard navigation sequencing.

Every other mixer section in this file (sound, fx, auxSends, balance, volume, fader, muteAndSolo, title) sets navigationRowStart and wires onNavigateControlIndexChanged.

🔧 Proposed fix to add missing navigation properties
 MixerUnMuteAndUnSoloSection {
     anchors.left: parent.left
     anchors.verticalCenter: parent.verticalCenter
     visible: contextMenuModel.unMuteAndUnSoloSectionVisible
     headerVisible: contextMenuModel.labelsSectionVisible
     headerWidth: prv.headerWidth
     model: mixerPanelModel
+    navigationRowStart: 650
+    needReadChannelName: prv.isPanelActivated
+
+    onNavigateControlIndexChanged: function(index) {
+        prv.setNavigateControlIndex(index)
+    }
 }

Note: navigationRowStart: 650 is suggested to place it between MixerMuteAndSoloSection (600) and MixerTitleSection (700). You may also need to verify that MixerUnMuteAndUnSoloSection.qml declares the required properties (navigationRowStart, needReadChannelName, signal navigateControlIndexChanged) if they are not already present.

📝 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
MixerUnMuteAndUnSoloSection {
anchors.left: parent.left
anchors.verticalCenter: parent.verticalCenter
visible: contextMenuModel.unMuteAndUnSoloSectionVisible
headerVisible: contextMenuModel.labelsSectionVisible
headerWidth: prv.headerWidth
model: mixerPanelModel
}
MixerUnMuteAndUnSoloSection {
anchors.left: parent.left
anchors.verticalCenter: parent.verticalCenter
visible: contextMenuModel.unMuteAndUnSoloSectionVisible
headerVisible: contextMenuModel.labelsSectionVisible
headerWidth: prv.headerWidth
model: mixerPanelModel
navigationRowStart: 650
needReadChannelName: prv.isPanelActivated
onNavigateControlIndexChanged: function(index) {
prv.setNavigateControlIndex(index)
}
}
🤖 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 `@src/playback/qml/MuseScore/Playback/MixerPanel.qml` around lines 329 - 336,
The MixerUnMuteAndUnSoloSection instance is missing required navigation
properties used internally (causing runtime errors); set the navigation props on
this instance: provide channelItem.panel by wiring channelItem.panel:
root.channelItem.panel, set navigationRowStart (e.g. navigationRowStart: 650 to
sit between the mute/solo and title sections), forward accessibleName:
accessibleName: root.accessibleName, and hook the navigation signal with
onNavigateControlIndexChanged: root.navigateControlIndexChanged; also add
needReadChannelName: true if the component expects it. Ensure
MixerUnMuteAndUnSoloSection.qml declares these properties/signals if not
already.

case MixerSectionType::Fader: return TranslatableString("playback", "Fader");
case MixerSectionType::MuteAndSolo: return TranslatableString("playback", "Mute and solo");
case MixerSectionType::Title: return TranslatableString("playback", "Name");
case MixerSectionType::UnMuteAndUnSolo: return TranslatableString("playback", "Unmute and Unsolo");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Inconsistent capitalization in section title.

The title "Unmute and Unsolo" capitalizes "Unsolo", but line 50 uses lowercase "solo" in "Mute and solo". For consistency, this should be "Unmute and unsolo".

✏️ Proposed fix
-    case MixerSectionType::UnMuteAndUnSolo: return TranslatableString("playback", "Unmute and Unsolo");
+    case MixerSectionType::UnMuteAndUnSolo: return TranslatableString("playback", "Unmute and unsolo");
📝 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
case MixerSectionType::UnMuteAndUnSolo: return TranslatableString("playback", "Unmute and Unsolo");
case MixerSectionType::UnMuteAndUnSolo: return TranslatableString("playback", "Unmute and unsolo");
🤖 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 `@src/playback/qml/MuseScore/Playback/mixerpanelcontextmenumodel.cpp` at line
52, The string returned for MixerSectionType::UnMuteAndUnSolo uses inconsistent
capitalization ("Unmute and Unsolo"); update the TranslatableString in the
corresponding case in mixerpanelcontextmenumodel.cpp to use "Unmute and unsolo"
so it matches the lowercase "solo" used in the "Mute and solo" entry (change the
second word to lowercase "unsolo" in the TranslatableString call).

Comment on lines +18 to +19
REPO_ROOT="${HERE}/../.."
UNCRUSTIFY_CONFIG="${REPO_ROOT}/muse/tools/codestyle/uncrustify_muse.cfg" # lives in framework submodule

Copy link
Copy Markdown
Contributor

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
# Description: Verify uncrustify_muse.cfg exists at the new hardcoded path

# Check if the config file exists at the new path
if [ -f "muse/tools/codestyle/uncrustify_muse.cfg" ]; then
    echo "✓ Config file found at muse/tools/codestyle/uncrustify_muse.cfg"
else
    echo "✗ Config file NOT found at muse/tools/codestyle/uncrustify_muse.cfg"
    echo "Searching for uncrustify_muse.cfg anywhere in the repository:"
    fd -t f "uncrustify_muse.cfg"
fi

# Also check if the old config path exists
if [ -f "tools/codestyle/uncrustify_musescore.cfg" ]; then
    echo "ℹ Old config file still exists at tools/codestyle/uncrustify_musescore.cfg"
fi

Repository: musescore/MuseScore

Length of output: 193


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Locate uncrustify_muse*.cfg files =="
fd -t f -a "uncrustify_muse*.cfg" . || true
echo
echo "== Locate uncrustify_musescore.cfg =="
fd -t f -a "uncrustify_musescore.cfg" . || true
echo
echo "== Print tidy_file.sh relevant lines (18-30) =="
sed -n '1,120p' tools/codestyle/tidy_file.sh | nl -ba | sed -n '1,120p' 
echo
echo "== Search for UNCRUSTIFY_CONFIG usage =="
rg -n "UNCRUSTIFY_CONFIG|uncrustify_muse\.cfg|uncrustify_musescore\.cfg" tools/codestyle/tidy_file.sh tools/codestyle -S || true

Repository: musescore/MuseScore

Length of output: 253


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Tool availability =="
command -v fd || echo "fd: not found"
command -v rg || echo "rg: not found"
command -v cat || echo "cat: not found"

echo
echo "== Locate uncrustify_muse*.cfg files =="
find . -type f -name 'uncrustify_muse*.cfg' 2>/dev/null | sed 's|^\./||' || true

echo
echo "== Locate uncrustify_musescore.cfg =="
find . -type f -name 'uncrustify_musescore.cfg' 2>/dev/null | sed 's|^\./||' || true

echo
echo "== Print tools/codestyle/tidy_file.sh (lines 1-80) with line numbers =="
sed -n '1,120p' tools/codestyle/tidy_file.sh | cat -n | sed -n '1,120p'

echo
echo "== Search for UNCRUSTIFY_CONFIG/uncrustify cfg references in tidy_file.sh =="
if command -v rg >/dev/null 2>&1; then
  rg -n "UNCRUSTIFY_CONFIG|uncrustify_muse\.cfg|uncrustify_musescore\.cfg" tools/codestyle/tidy_file.sh || true
else
  grep -nE "UNCRUSTIFY_CONFIG|uncrustify_muse\.cfg|uncrustify_musescore\.cfg" tools/codestyle/tidy_file.sh || true
fi

Repository: musescore/MuseScore

Length of output: 2767


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check whether ./muse/ exists =="
ls -la muse 2>/dev/null || echo "muse/ not present in this checkout"

echo
echo "== Search for uncrustify_muse/uncrustify_musescore references across repo =="
rg -n "uncrustify_muse|uncrustify_musescore|uncrustify\.cfg" . || true

echo
echo "== Find download.cmake files =="
find . -type f -name 'download.cmake' 2>/dev/null | sed 's|^\./||' || true

echo
echo "== Search for uncrustify in *.cmake files =="
rg -n "uncrustify" --glob='*.cmake' . || true

echo
echo "== Show any uncrustify download/copy commands near matches =="
# Heuristic: print 60 lines around any line containing 'uncrustify' in CMake
rg -n "uncrustify" --glob='*.cmake' . -C 30 || true

Repository: musescore/MuseScore

Length of output: 6392


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Show tools/codestyle/tidy_file.sh (15-35) =="
sed -n '15,40p' tools/codestyle/tidy_file.sh | cat -n

echo
echo "== Show buildscripts/tools/codestyle/download.cmake =="
sed -n '1,120p' buildscripts/tools/codestyle/download.cmake | cat -n

echo
echo "== Show buildscripts/tools/codestyle/format_file.cmake (1-80) =="
sed -n '1,90p' buildscripts/tools/codestyle/format_file.cmake | cat -n

echo
echo "== Find pre-commit / hooks that call tidy_file.sh =="
rg -n "tidy_file\.sh|tools/codestyle/tidy_file\.sh|codestyle/tidy" . || true

echo
echo "== List .git hooks / pre-commit configs if present =="
find . -maxdepth 3 -type f \( -iname '*pre-commit*' -o -iname '*precommit*' -o -iname '*.pre-commit*' \) 2>/dev/null || true

Repository: musescore/MuseScore

Length of output: 3577


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Show hooks/pre-commit around where tidy_file.sh is called (60-120) =="
sed -n '60,140p' hooks/pre-commit | cat -n

echo
echo "== Search for uncrustify_muse.cfg anywhere in repo =="
find . -type f -name 'uncrustify_muse.cfg' -print | sed 's|^\./||' || true

echo
echo "== Search for uncrustify_muse*.cfg anywhere in repo =="
find . -type f -name 'uncrustify_muse*.cfg' -print | sed 's|^\./||' || true

Repository: musescore/MuseScore

Length of output: 2441


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Search for uncrustify_musescore.cfg =="
find . -type f -name 'uncrustify_musescore.cfg' -print | sed 's|^\./||' || true

echo
echo "== Any references to uncrustify_musescore in repo =="
rg -n "uncrustify_musescore\.cfg" . || true

echo
echo "== Any references to uncrustify_muse.cfg besides tidy_file/download/format =="
rg -n "uncrustify_muse\.cfg" . || true

Repository: musescore/MuseScore

Length of output: 720


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== .gitmodules =="
if [ -f .gitmodules ]; then
  cat .gitmodules
else
  echo "No .gitmodules file found."
fi

echo
echo "== Check if 'muse' is tracked as a submodule (git config) =="
if command -v git >/dev/null 2>&1; then
  git config --file .gitmodules --get-regexp '^submodule\..*\.path$' || true
  git submodule status || true
else
  echo "git not available."
fi

echo
echo "== List contents of muse/ (max depth 3) =="
find muse -maxdepth 3 -print 2>/dev/null || true

Repository: musescore/MuseScore

Length of output: 371


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Search docs for muse_framework/submodule initialization =="
rg -n "muse_framework|muse/.*submodule|git submodule|submodule.*muse" . || true

echo
echo "== Search for 'pre-commit' setup instructions =="
rg -n "pre-commit|hooks/pre-commit" . || true

Repository: musescore/MuseScore

Length of output: 2955


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== README.md (70-120) =="
sed -n '70,120p' README.md | cat -n

echo
echo "== hooks/install.sh (1-120) =="
sed -n '1,160p' hooks/install.sh | cat -n

echo
echo "== hooks/uninstall.sh (1-120) =="
sed -n '1,120p' hooks/uninstall.sh | cat -n

Repository: musescore/MuseScore

Length of output: 3529


Fix pre-commit uncrustify config path (may break formatting for developers)

  • tools/codestyle/tidy_file.sh hardcodes UNCRUSTIFY_CONFIG="${REPO_ROOT}/muse/tools/codestyle/uncrustify_muse.cfg" and uses it in uncrustify -c ....
  • hooks/pre-commit runs tidy_file.sh directly and does not download/copy uncrustify_muse.cfg, so the hook relies entirely on the submodule file being present at that exact path.
  • In this repo checkout, muse/tools/codestyle/uncrustify_muse.cfg (and any uncrustify_muse*.cfg) is missing; add a clear existence check with actionable guidance (e.g., init/update the muse_framework submodule) or make tidy_file.sh download the cfg (similar to buildscripts/tools/codestyle/download.cmake).
  • This looks like an infra/code-style change rather than the mixer-button feature—if it wasn’t intended, split or justify it in the PR.
🤖 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 `@tools/codestyle/tidy_file.sh` around lines 18 - 19, The tidy_file.sh script
currently sets
UNCRUSTIFY_CONFIG="${REPO_ROOT}/muse/tools/codestyle/uncrustify_muse.cfg" and
assumes the submodule file exists; update tidy_file.sh (and mention
hooks/pre-commit which calls it) to check for the existence of the
UNCRUSTIFY_CONFIG path and exit with a clear actionable message if missing
(e.g., instructing to init/update the muse_framework submodule or run a download
step), or implement an automatic fallback that downloads or copies the config
(reuse the logic/pattern from buildscripts/tools/codestyle/download.cmake)
before invoking uncrustify; reference the REPO_ROOT and UNCRUSTIFY_CONFIG
variables so the check/fallback is implemented using those symbols.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mixer: enable Un-mute all and Un-solo all

4 participants