Tuning plugin: UI refinements #33512
Hidden character warning
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThis PR removes predefined custom tunings and makes custom entries runtime-managed. It adds root functions addCustomTuning(), customCount(), and deleteCustomTuning(), updates reset/compare logic to treat ids starting with "c" as custom, recalculates pure-tone offsets from the active tuningModel, forces the Flickable model to refresh and recenters the view on apply, adds inline name editing for custom items, introduces Add/Delete buttons in the bottom controls, and changes restoreSavedValues() to deep-copy the live model, append saved custom entries, merge other saved entries by id (preserving existing names), and reserialize options.data. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
share/plugins/tuning/tuning.qml (1)
434-450: 💤 Low valueConsider hiding radio button text during inline editing.
When editing, both the
RoundedRadioButton(with its text label) and theTextInputFieldare visible simultaneously. Depending on component styling, this may cause visual overlap. Consider conditionally hiding the radio button's text or the entire button whentuningItem.editingis true.🤖 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 `@share/plugins/tuning/tuning.qml` around lines 434 - 450, The radio button's label and the inline TextInputField overlap during editing; update the MU.RoundedRadioButton (id: radioButton) to hide its text (or the whole button) when tuningItem.editing is true—for example, replace text: modelData.name with a conditional expression like text: tuningItem.editing ? "" : modelData.name or change visible to visible: !tuningItem.isSeparatorLine && !tuningItem.editing so the radio label/button is not shown while editing.
🤖 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 `@share/plugins/tuning/tuning.qml`:
- Line 680: The button label currently uses a hardcoded literal "Add custom
tuning"; update the Button's text property (the element with text: "Add custom
tuning") to use the qsTr() translation wrapper so it follows the file's i18n
pattern—replace the literal with a qsTr-wrapped string and ensure any nearby
similar labels remain consistent.
- Around line 356-381: The ID generation in addCustomTuning using customCount()
+ 1 can create duplicates after deletions; change it to compute a new unique
numeric suffix by scanning existing tuningModel entries for ids starting with
"c", parse their numeric parts (or track a persistent monotonic counter), take
the maximum found and add 1, then use that value for the new "id" and "name";
update/add logic in customCount() or replace it with a helper (referencing
addCustomTuning, customCount, tuningModel and the id property) so new IDs are
always increasing and never reused.
- Around line 444-449: Remove the non-existent onDoubleClicked handler from the
MU.RoundedRadioButton (the block referencing modelData.id, tuningItem.editing
and textField.text) and implement double-click handling by either wrapping the
MU.RoundedRadioButton in a MouseArea that uses onDoubleClicked to set
tuningItem.editing and textField.text, or replace it with an onClicked +
timer-based double-click detection that checks modelData.id.startsWith("c")
before toggling editing and copying modelData.name into textField.text.
---
Nitpick comments:
In `@share/plugins/tuning/tuning.qml`:
- Around line 434-450: The radio button's label and the inline TextInputField
overlap during editing; update the MU.RoundedRadioButton (id: radioButton) to
hide its text (or the whole button) when tuningItem.editing is true—for example,
replace text: modelData.name with a conditional expression like text:
tuningItem.editing ? "" : modelData.name or change visible to visible:
!tuningItem.isSeparatorLine && !tuningItem.editing so the radio label/button is
not shown while editing.
🪄 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: e168649f-ea1c-46f4-b1a2-f142e1435bce
📒 Files selected for processing (1)
share/plugins/tuning/tuning.qml
| function addCustomTuning() { | ||
| var newCustomTuning = { | ||
| "id": "c" + (customCount() + 1), | ||
| "name": "Custom " + (customCount() + 1), | ||
| "offsets": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], | ||
| "root": currentRoot, | ||
| "pure": currentPureTone, | ||
| "globalOffset": currentGlobalOffset | ||
| } | ||
| tuningModel.push(newCustomTuning) | ||
| tuningsFlickable.model = tuningModel // necessary to trigger the ListView update | ||
| tuningsFlickable.positionViewAtEnd() | ||
|
|
||
| // Select the new temperament | ||
| temperamentClicked(tuningModel.length - 1) | ||
| } | ||
|
|
||
| function customCount() { | ||
| var count = 0 | ||
| for (var i in tuningModel) { | ||
| if (tuningModel[i].id.startsWith("c")) { | ||
| count++ | ||
| } | ||
| } | ||
| return count | ||
| } |
There was a problem hiding this comment.
Custom tuning ID generation can produce duplicates after deletions.
The current approach uses customCount() + 1 for the new ID. If a user creates c1, c2, deletes c1, then adds another tuning, customCount() returns 1 and the new ID becomes "c2" — a duplicate.
Consider tracking a monotonically increasing counter or finding the maximum existing ID:
Proposed fix using max ID approach
function addCustomTuning() {
var newCustomTuning = {
- "id": "c" + (customCount() + 1),
- "name": "Custom " + (customCount() + 1),
+ "id": "c" + (nextCustomId()),
+ "name": "Custom " + (customCount() + 1), // Display name can still use count
"offsets": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
...
}
...
}
+function nextCustomId() {
+ var maxId = 0
+ for (var i in tuningModel) {
+ if (tuningModel[i].id.startsWith("c")) {
+ var num = parseInt(tuningModel[i].id.substring(1), 10)
+ if (!isNaN(num) && num > maxId) {
+ maxId = num
+ }
+ }
+ }
+ return maxId + 1
+}📝 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.
| function addCustomTuning() { | |
| var newCustomTuning = { | |
| "id": "c" + (customCount() + 1), | |
| "name": "Custom " + (customCount() + 1), | |
| "offsets": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], | |
| "root": currentRoot, | |
| "pure": currentPureTone, | |
| "globalOffset": currentGlobalOffset | |
| } | |
| tuningModel.push(newCustomTuning) | |
| tuningsFlickable.model = tuningModel // necessary to trigger the ListView update | |
| tuningsFlickable.positionViewAtEnd() | |
| // Select the new temperament | |
| temperamentClicked(tuningModel.length - 1) | |
| } | |
| function customCount() { | |
| var count = 0 | |
| for (var i in tuningModel) { | |
| if (tuningModel[i].id.startsWith("c")) { | |
| count++ | |
| } | |
| } | |
| return count | |
| } | |
| function addCustomTuning() { | |
| var newCustomTuning = { | |
| "id": "c" + (nextCustomId()), | |
| "name": "Custom " + (customCount() + 1), // Display name can still use count | |
| "offsets": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], | |
| "root": currentRoot, | |
| "pure": currentPureTone, | |
| "globalOffset": currentGlobalOffset | |
| } | |
| tuningModel.push(newCustomTuning) | |
| tuningsFlickable.model = tuningModel // necessary to trigger the ListView update | |
| tuningsFlickable.positionViewAtEnd() | |
| // Select the new temperament | |
| temperamentClicked(tuningModel.length - 1) | |
| } | |
| function nextCustomId() { | |
| var maxId = 0 | |
| for (var i in tuningModel) { | |
| if (tuningModel[i].id.startsWith("c")) { | |
| var num = parseInt(tuningModel[i].id.substring(1), 10) | |
| if (!isNaN(num) && num > maxId) { | |
| maxId = num | |
| } | |
| } | |
| } | |
| return maxId + 1 | |
| } | |
| function customCount() { | |
| var count = 0 | |
| for (var i in tuningModel) { | |
| if (tuningModel[i].id.startsWith("c")) { | |
| count++ | |
| } | |
| } | |
| return count | |
| } |
🤖 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 `@share/plugins/tuning/tuning.qml` around lines 356 - 381, The ID generation in
addCustomTuning using customCount() + 1 can create duplicates after deletions;
change it to compute a new unique numeric suffix by scanning existing
tuningModel entries for ids starting with "c", parse their numeric parts (or
track a persistent monotonic counter), take the maximum found and add 1, then
use that value for the new "id" and "name"; update/add logic in customCount() or
replace it with a helper (referencing addCustomTuning, customCount, tuningModel
and the id property) so new IDs are always increasing and never reused.
| onDoubleClicked: { | ||
| if (modelData.id.startsWith("c")) { | ||
| tuningItem.editing = true | ||
| textField.text = modelData.name | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for RoundedRadioButton definition to verify doubleClicked signal support
fd -i "RoundedRadioButton.qml" --exec cat {}Repository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
rg "RoundedRadioButton" --type qml -A 5 -B 5 | head -100Repository: musescore/MuseScore
Length of output: 9271
🏁 Script executed:
# Look for the MU module definition and find where RoundedRadioButton might be
fd "\.qml$" -x rg -l "RoundedRadioButton" {} | head -20Repository: musescore/MuseScore
Length of output: 1536
🏁 Script executed:
# Check the tuning.qml file to understand the context and imports
cat -n share/plugins/tuning/tuning.qml | head -50Repository: musescore/MuseScore
Length of output: 2351
🏁 Script executed:
fd "RoundedRadioButton" --type fRepository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
# Search in UiComponents directory
fd -type f "\.qml$" | xargs rg -l "RoundedRadioButton" | grep -i uicomponentsRepository: musescore/MuseScore
Length of output: 234
🏁 Script executed:
# Look for the component definition
rg "component RoundedRadioButton" --type qml -A 10Repository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
# Check what the component extends from
rg "RoundedRadioButton\s*:" --type qml -A 5Repository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
find . -type d -name "UiComponents" 2>/dev/nullRepository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
# Search for qml files that define RoundedRadioButton
rg "RoundedRadioButton" --type qml -B 3 | grep -E "^[./]+.*\.qml:" | head -20Repository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
# Search in common framework paths
find . -path "*/framework/*" -name "*.qml" | xargs rg -l "RoundedRadioButton" 2>/dev/nullRepository: musescore/MuseScore
Length of output: 2174
🏁 Script executed:
# Look for any qml file that contains the definition
rg "id:\s*RoundedRadioButton|component.*RoundedRadioButton" --type qml -A 20Repository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
# Look for qml in src/framework
find src -type d -name "framework" 2>/dev/nullRepository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
# Check full path structure
find src -type d | grep -i "ui" | head -20Repository: musescore/MuseScore
Length of output: 666
🏁 Script executed:
# Read the specific lines around 444-449 in tuning.qml
cat -n share/plugins/tuning/tuning.qml | sed -n '430,480p'Repository: musescore/MuseScore
Length of output: 2989
🏁 Script executed:
# Look for RadioButton definitions in the codebase
rg "^.*RadioButton\s*{" --type qml -B 5 -A 10 | head -80Repository: musescore/MuseScore
Length of output: 163
🏁 Script executed:
# Search for qmldir files that define modules
find . -name "qmldir" | head -20Repository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
# Look for RoundedRadioButton in framework directories
find src -type d -name "*Components*" 2>/dev/nullRepository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
# Search for RadioButton or Button base class definitions
rg "RadioButton\s*:\s*" --type qml -A 3 | head -50Repository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
# Check if there's a Controls module that defines RadioButton
rg "import.*Controls" share/plugins/tuning/tuning.qmlRepository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
# Search all qml files for RadioButton or Control definitions
find . -name "*.qml" -exec grep -l "RadioButton" {} \; | grep -v node_modules | head -30Repository: musescore/MuseScore
Length of output: 2344
🏁 Script executed:
# Check the entire src/framework directory structure
find src/framework -type d 2>/dev/null | head -20Repository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
# Look for any qml that might define the component
find . -name "*.qml" -exec grep -l "onDoubleClicked" {} \; | head -10Repository: musescore/MuseScore
Length of output: 850
🏁 Script executed:
# Search for component or Item definitions that might be RoundedRadioButton
rg "component.*Button|Button\s*{" --type qml -B 2 -A 5 | grep -A 5 "Rounded" | head -40Repository: musescore/MuseScore
Length of output: 183
🏁 Script executed:
# Search for CMakeLists to understand module structure
find . -name "CMakeLists.txt" -exec grep -l "UiComponents\|Muse" {} \; | head -10Repository: musescore/MuseScore
Length of output: 4138
🏁 Script executed:
# Look for the qml module registration
find . -name "*.qml" -path "*/Muse/*" -o -path "*/muse/*" | head -20Repository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
# Check src/framework for any UI or Components related files
find src/framework -name "*.qml" 2>/dev/null | head -20Repository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
# Search for where Muse.UiComponents might be defined
rg "UiComponents" --type qml -B 2 -A 2 | head -40Repository: musescore/MuseScore
Length of output: 2097
🌐 Web query:
Qt Quick RadioButton doubleClicked signal
💡 Result:
The Qt Quick RadioButton type does not have a doubleClicked signal [1][2][3]. RadioButton inherits from AbstractButton [1][2], which provides basic signals such as clicked(), pressed(), and released(), but it does not include native support for double-click events [1][2]. If you need to detect a double-click on a RadioButton in QML, you can use a MouseArea to wrap the RadioButton (or place it on top) and handle the onDoubleClicked signal [4]. Note that using a MouseArea may intercept mouse events intended for the RadioButton itself; you may need to explicitly forward those events or adjust the propagateComposedEvents property if interaction issues arise. Alternatively, if you are working with a standard Button type in some versions of Qt Quick, it may provide a doubleClicked() signal [4], but this is not available for the RadioButton component [1][2][3].
Citations:
- 1: https://doc.qt.io/qt-6.9/qml-qtquick-controls-radiobutton.html
- 2: https://doc.qt.io/qt-6/qml-qtquick-controls-radiobutton.html
- 3: https://doc.qt.io/qt-6/qml-qtquick-controls-radiobutton-members.html
- 4: https://doc.qt.io/qt-6/qml-qtquick-controls-button.html
Remove onDoubleClicked handler from MU.RoundedRadioButton—the signal doesn't exist.
Qt Quick's RadioButton and AbstractButton do not expose a doubleClicked signal. The handler at lines 444–449 will never be triggered. To enable double-click editing, wrap the RoundedRadioButton in a MouseArea with onDoubleClicked, or use an alternative approach like onClicked with a timer to detect double-clicks.
🤖 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 `@share/plugins/tuning/tuning.qml` around lines 444 - 449, Remove the
non-existent onDoubleClicked handler from the MU.RoundedRadioButton (the block
referencing modelData.id, tuningItem.editing and textField.text) and implement
double-click handling by either wrapping the MU.RoundedRadioButton in a
MouseArea that uses onDoubleClicked to set tuningItem.editing and
textField.text, or replace it with an onClicked + timer-based double-click
detection that checks modelData.id.startsWith("c") before toggling editing and
copying modelData.name into textField.text.
| MU.FlatButton { | ||
| id: addCustomButton | ||
| Layout.preferredWidth: temperamentBox.width - deleteButton.width - 5 | ||
| text: "Add custom tuning" |
There was a problem hiding this comment.
Button text should be wrapped in qsTr() for translation.
Other strings in this file use qsTr() for internationalization. This button label should follow the same pattern.
Proposed fix
- text: "Add custom tuning"
+ text: qsTr("Add custom tuning") 📝 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.
| text: "Add custom tuning" | |
| text: qsTr("Add custom tuning") |
🤖 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 `@share/plugins/tuning/tuning.qml` at line 680, The button label currently uses
a hardcoded literal "Add custom tuning"; update the Button's text property (the
element with text: "Add custom tuning") to use the qsTr() translation wrapper so
it follows the file's i18n pattern—replace the literal with a qsTr-wrapped
string and ensure any nearby similar labels remain consistent.
| // index here is modulo pitch | ||
| var rootOffset = tuningModel[currentTemperament].offsets[(fifthsOffsets[index] - currentRoot + 12) % 12] | ||
| var pureToneOffset = defaultTuning[currentTemperament].offsets[(currentPureTone - currentRoot + 12) % 12] - defaultTuning[currentTemperament].offsets[0] // 0 because definitions are aligned to root C / pure C | ||
| var pureToneOffset = tuningModel[currentTemperament].offsets[(currentPureTone - currentRoot + 12) % 12] - tuningModel[currentTemperament].offsets[0] // 0 because definitions are aligned to root C / pure C |
There was a problem hiding this comment.
We have been over this - changing this causes unwanted behaviour
There was a problem hiding this comment.
I dont know where this has been discussed. This change is necessary so that custom tuning can also be rotated.
|
|
||
| property bool chromatic: true | ||
| property int currentTemperament: 1 | ||
| property int currentTemperament: 1 |
| if (tuningModel[temperamentIndex].id == "separatorLine" || temperamentIndex == currentTemperament) { | ||
| return | ||
| } | ||
| } |
| temperamentClicked(tuningModel.length - 1) | ||
| } | ||
|
|
||
| function customCount() { |
There was a problem hiding this comment.
Would tuningModel.length - readDefaults().length work instead? Likely faster
There was a problem hiding this comment.
Sure, I'll make that change!
| } | ||
| } | ||
| } | ||
| } |
| var data = JSON.parse(options.data) | ||
| for (var i in data) { | ||
| if (data[i].id == "separatorLine") { | ||
| continue | ||
| } | ||
| for (var j in newValues) { | ||
| if (newValues[j].id == data[i].id) { | ||
| var name = newValues[j].name | ||
| newValues[j] = data[i] | ||
| newValues[j]["name"] = name | ||
| } | ||
| } | ||
| } | ||
| newValues = JSON.parse(options.data) |
There was a problem hiding this comment.
Can you explain why this change works?
There was a problem hiding this comment.
Iterating over and comparing each item is not wanted here, specially since custom temperaments are not built in the default array.
There was a problem hiding this comment.
The reason for this is to automatically update the old saved model if changes are made (by future plugin authors) to the non-custom tunings. How can this still be guaranteed?
There was a problem hiding this comment.
I see. I'll push another fix for that shortly.
Ash-86
force-pushed
the
Tuning-plugin-–-UI-refinements
branch
from
4026fa1 to
6c7e2e5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
share/plugins/tuning/tuning.qml (2)
358-359:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGenerate custom IDs from the max existing suffix, not the current count.
After a delete,
customCount() + 1can reuse an existingcN, which breaks restore and selection byid. Please keep the numeric suffix monotonic or derive it from the current max.🤖 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 `@share/plugins/tuning/tuning.qml` around lines 358 - 359, The id generation currently uses "id": "c" + (customCount() + 1) which can collide after deletions; change this to compute the next suffix from the maximum numeric suffix among existing custom items (scan existing items' ids like /^c(\d+)$/, parse digits, take max, then use max+1) so the generated id is monotonic; update the creation logic in tuning.qml (the place that builds the "id" and "name") to call a helper that returns nextCustomSuffix instead of using customCount(), ensuring restore/selection by id remains stable.
439-444:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRename mode is still wired to a non-existent
doubleClickedsignal.Unless
MU.RoundedRadioButtonexplicitly re-emitsdoubleClicked, this handler never runs, so custom tunings cannot enter inline rename mode.#!/bin/bash set -e fd -i 'RoundedRadioButton.qml' --exec sh -c 'printf "\n== %s ==\n" "$1"; sed -n "1,220p" "$1"' _ {} rg -n --type qml 'signal\s+doubleClicked|onDoubleClicked|doubleClicked\s*:'Expected result:
RoundedRadioButton.qmldoes not declare or re-emitdoubleClicked, and this file is the new call site relying on it.🤖 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 `@share/plugins/tuning/tuning.qml` around lines 439 - 444, The onDoubleClicked handler is attached to MU.RoundedRadioButton which does not emit doubleClicked, so the inline rename never triggers; replace the nonexistent signal usage by adding a MouseArea over the radio button (or parent row) and move the logic into its onDoubleClicked handler: create a MouseArea with anchors.fill: parent (or appropriate bounds), implement onDoubleClicked { if (modelData.id.startsWith("c")) { tuningItem.editing = true; textField.text = modelData.name } }, or if you intended a single click use the radio button's supported clicked/checkedChanged signal instead of onDoubleClicked.
🤖 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 `@share/plugins/tuning/tuning.qml`:
- Around line 357-363: The new custom tuning currently initializes "offsets" to
twelve zeros, losing the user's current temperament; update the newCustomTuning
creation to copy the current temperament offsets instead of hardcoded zeros.
Locate the newCustomTuning object (variable name newCustomTuning) and replace
the "offsets": [0,...] entry with a clone of the current offsets array (e.g.,
use whatever current offsets variable/property is used elsewhere such as
currentOffsets or offsetsFromCurrentState and copy it via
slice()/concat()/Array.from to avoid referencing by pointer). Keep the other
fields (id, name, root, pure, globalOffset) unchanged.
---
Duplicate comments:
In `@share/plugins/tuning/tuning.qml`:
- Around line 358-359: The id generation currently uses "id": "c" +
(customCount() + 1) which can collide after deletions; change this to compute
the next suffix from the maximum numeric suffix among existing custom items
(scan existing items' ids like /^c(\d+)$/, parse digits, take max, then use
max+1) so the generated id is monotonic; update the creation logic in tuning.qml
(the place that builds the "id" and "name") to call a helper that returns
nextCustomSuffix instead of using customCount(), ensuring restore/selection by
id remains stable.
- Around line 439-444: The onDoubleClicked handler is attached to
MU.RoundedRadioButton which does not emit doubleClicked, so the inline rename
never triggers; replace the nonexistent signal usage by adding a MouseArea over
the radio button (or parent row) and move the logic into its onDoubleClicked
handler: create a MouseArea with anchors.fill: parent (or appropriate bounds),
implement onDoubleClicked { if (modelData.id.startsWith("c")) {
tuningItem.editing = true; textField.text = modelData.name } }, or if you
intended a single click use the radio button's supported clicked/checkedChanged
signal instead of onDoubleClicked.
🪄 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: 778e63d0-3e3d-4ac2-a14e-e415370c9caa
📒 Files selected for processing (1)
share/plugins/tuning/tuning.qml
| var newCustomTuning = { | ||
| "id": "c" + (customCount() + 1), | ||
| "name": "Custom " + (customCount() + 1), | ||
| "offsets": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], | ||
| "root": currentRoot, | ||
| "pure": currentPureTone, | ||
| "globalOffset": currentGlobalOffset |
There was a problem hiding this comment.
Clone the current temperament offsets when adding a custom tuning.
This flow is meant to preserve the tuning the user is looking at, but the new entry starts with twelve zero offsets. That drops the current temperament state immediately.
Suggested fix
function addCustomTuning() {
var newCustomTuning = {
"id": "c" + (customCount() + 1),
"name": "Custom " + (customCount() + 1),
- "offsets": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
+ "offsets": tuningModel[currentTemperament].offsets.slice(),
"root": currentRoot,
"pure": currentPureTone,
"globalOffset": currentGlobalOffset
}📝 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.
| var newCustomTuning = { | |
| "id": "c" + (customCount() + 1), | |
| "name": "Custom " + (customCount() + 1), | |
| "offsets": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], | |
| "root": currentRoot, | |
| "pure": currentPureTone, | |
| "globalOffset": currentGlobalOffset | |
| var newCustomTuning = { | |
| "id": "c" + (customCount() + 1), | |
| "name": "Custom " + (customCount() + 1), | |
| "offsets": tuningModel[currentTemperament].offsets.slice(), | |
| "root": currentRoot, | |
| "pure": currentPureTone, | |
| "globalOffset": currentGlobalOffset | |
| } |
🤖 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 `@share/plugins/tuning/tuning.qml` around lines 357 - 363, The new custom
tuning currently initializes "offsets" to twelve zeros, losing the user's
current temperament; update the newCustomTuning creation to copy the current
temperament offsets instead of hardcoded zeros. Locate the newCustomTuning
object (variable name newCustomTuning) and replace the "offsets": [0,...] entry
with a clone of the current offsets array (e.g., use whatever current offsets
variable/property is used elsewhere such as currentOffsets or
offsetsFromCurrentState and copy it via slice()/concat()/Array.from to avoid
referencing by pointer). Keep the other fields (id, name, root, pure,
globalOffset) unchanged.
3 participants
Resolves: #31366
Some changes to dynamically add/remove/rename custom temperaments.