Skip to content

[4.7] fixed #32468: Update Em7 and Dm7 fretboard diagrams#32528

Merged
Eism merged 3 commits intomusescore:4.7from
Eism:fretboard_diagrams_update_4.7
Apr 20, 2026
Merged

[4.7] fixed #32468: Update Em7 and Dm7 fretboard diagrams#32528
Eism merged 3 commits intomusescore:4.7from
Eism:fretboard_diagrams_update_4.7

Conversation

@Eism
Copy link
Copy Markdown
Contributor

@Eism Eism commented Mar 6, 2026

Resolves: #32468

Summary by CodeRabbit

  • Bug Fixes

    • Reworked and reorganized many guitar chord diagrams and fingering patterns across multiple chord names.
    • Adjusted barre spans, string markers, fret offsets and visual markers for more accurate and consistent chord rendering.
    • Reintroduced and replaced several chord entries with updated diagram definitions to improve display fidelity.
  • Tests

    • Updated fret-diagram reference data to reflect corrected barre span endpoints.

@Eism Eism requested a review from RomanPudashkin March 6, 2026 13:45
@RomanPudashkin RomanPudashkin linked an issue Mar 9, 2026 that may be closed by this pull request
@avvvvve
Copy link
Copy Markdown

avvvvve commented Apr 14, 2026

@Eism Just tried making the edits myself. Let's see if all the differences are still there if you use this file instead:

Chord-Diagram Database.mscz.zip

@Eism Eism force-pushed the fretboard_diagrams_update_4.7 branch from 0bd390b to 87b9203 Compare April 15, 2026 08:49
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 15, 2026

📝 Walkthrough

Walkthrough

Reorganizes and updates harmony-to-diagram mappings in src/engraving/data/harmony_to_diagram.xml (many <HarmonyToDiagram> entries: patterns, fret offsets, dot/marker placements, and explicit barre ranges). Also updates multiple test .mscx files changing several <barre> elements' end attribute from -1 to 5.

Changes

Cohort / File(s) Summary
Harmony Fretboard Diagram Mappings
src/engraving/data/harmony_to_diagram.xml
Large reordering and modifications of <HarmonyToDiagram> entries: changed <pattern> values and fretOffset, altered <FretDiagram> contents (string markers, <dot> frets, <marker> types), removed shorthand <barre>1</barre> uses and replaced/added explicit <barre start="..." end="...">...</barre> ranges for many harmonies; some entries removed and reintroduced with different diagrams.
Transpose Test FretDiagram References
src/engraving/tests/transpose_data/.../undoDiatonicTransposeFretDiagrams.mscx, src/engraving/tests/transpose_data/.../undoDiatonicTransposeFretDiagrams01-ref.mscx, src/engraving/tests/transpose_data/.../undoDiatonicTransposeFretDiagrams02-ref.mscx, src/engraving/tests/transpose_data/.../undoDiatonicTransposeFretDiagramsChordSymbols.mscx, src/engraving/tests/transpose_data/.../undoDiatonicTransposeFretDiagramsChordSymbols01-ref.mscx, src/engraving/tests/transpose_data/.../undoDiatonicTransposeFretDiagramsChordSymbols02-ref.mscx, src/engraving/tests/transpose_data/.../undoTransposeFretDiagrams.mscx, src/engraving/tests/transpose_data/.../undoTransposeFretDiagrams01-ref.mscx, src/engraving/tests/transpose_data/.../undoTransposeFretDiagrams02-ref.mscx, src/engraving/tests/transpose_data/.../undoTransposeFretDiagramsChordSymbols.mscx, src/engraving/tests/transpose_data/.../undoTransposeFretDiagramsChordSymbols01-ref.mscx, src/engraving/tests/transpose_data/.../undoTransposeFretDiagramsChordSymbols02-ref.mscx
Test/reference .mscx updates only: multiple <barre> elements changed end attribute from -1 to 5 while preserving start and displayed barre text. No other structural changes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • miiizen
🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title states it updates Em7 and Dm7 fretboard diagrams, but the actual changes include many additional chord diagrams and extensive barre attribute modifications across the entire harmony database. Update the PR title to reflect the comprehensive scope: e.g., 'Update fretboard diagrams and fix barre end attributes' or describe the actual extent of changes beyond just Em7 and Dm7.
Description check ⚠️ Warning The PR description is minimal (only 'Resolves: #32468') and lacks details about the actual changeset, which includes extensive modifications to the harmony database beyond just Em7 and Dm7. Expand the description to explain what was changed in harmony_to_diagram.xml, why barre end attributes were modified, and clarify the scope of updates to multiple chord diagrams.
Out of Scope Changes check ⚠️ Warning The changeset includes extensive modifications to many harmony diagrams beyond the Em7 and Dm7 mentioned in issue #32468, including systematic barre end attribute changes from -1 to 5 across multiple test files. Clarify whether all modifications align with the reviewed database file provided in comments, or separate out-of-scope changes into a separate PR to maintain focus on the primary objective.
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR title and initial description reference issue #32468 (Em7 and Dm7 updates), but the actual changeset significantly exceeds these requirements with modifications to numerous additional harmonies and systematic barre attribute corrections.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

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

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.

@avvvvve
Copy link
Copy Markdown

avvvvve commented Apr 15, 2026

@Eism Here's a final update to the file for this. Took it a step further by identifying any chords that appeared more than once (whether their diagrams were the same or not) and kept one diagram for each. In the process of working through duplicates, I found some other diagrams that were not accurate, so those have been removed. Could you push this file, then I can update the PR name/description to reflect the broader fixes? Thanks!

Chord-Diagram Database.mscz.zip

@Eism Eism force-pushed the fretboard_diagrams_update_4.7 branch from 87b9203 to f7edd1f Compare April 16, 2026 06:57
@avvvvve
Copy link
Copy Markdown

avvvvve commented Apr 16, 2026

Approved! Thanks @Eism

@Eism Eism force-pushed the fretboard_diagrams_update_4.7 branch from f7edd1f to b5f0403 Compare April 17, 2026 05:39
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b14f4fdf-82a5-477e-8f23-c06bc2e8cd87

📥 Commits

Reviewing files that changed from the base of the PR and between f7edd1f and b5f0403.

📒 Files selected for processing (2)
  • src/engraving/data/harmony_to_diagram.xml
  • tools/harmony_to_diagram/Chord-Diagram Database.mscz

Comment thread src/engraving/data/harmony_to_diagram.xml Outdated
@avvvvve
Copy link
Copy Markdown

avvvvve commented Apr 17, 2026

Chord-Diagram Database.mscz.zip

@Eism FINAL_final_forrealthistime_version.zip

@Eism Eism force-pushed the fretboard_diagrams_update_4.7 branch from 616359e to c00ec52 Compare April 17, 2026 13:54
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: fa178bc8-a47f-477d-9d5e-941854e4362f

📥 Commits

Reviewing files that changed from the base of the PR and between 616359e and c00ec52.

📒 Files selected for processing (14)
  • src/engraving/data/harmony_to_diagram.xml
  • src/engraving/tests/transpose_data/undoDiatonicTransposeFretDiagrams.mscx
  • src/engraving/tests/transpose_data/undoDiatonicTransposeFretDiagrams01-ref.mscx
  • src/engraving/tests/transpose_data/undoDiatonicTransposeFretDiagrams02-ref.mscx
  • src/engraving/tests/transpose_data/undoDiatonicTransposeFretDiagramsChordSymbols.mscx
  • src/engraving/tests/transpose_data/undoDiatonicTransposeFretDiagramsChordSymbols01-ref.mscx
  • src/engraving/tests/transpose_data/undoDiatonicTransposeFretDiagramsChordSymbols02-ref.mscx
  • src/engraving/tests/transpose_data/undoTransposeFretDiagrams.mscx
  • src/engraving/tests/transpose_data/undoTransposeFretDiagrams01-ref.mscx
  • src/engraving/tests/transpose_data/undoTransposeFretDiagrams02-ref.mscx
  • src/engraving/tests/transpose_data/undoTransposeFretDiagramsChordSymbols.mscx
  • src/engraving/tests/transpose_data/undoTransposeFretDiagramsChordSymbols01-ref.mscx
  • src/engraving/tests/transpose_data/undoTransposeFretDiagramsChordSymbols02-ref.mscx
  • tools/harmony_to_diagram/Chord-Diagram Database.mscz

Comment thread src/engraving/data/harmony_to_diagram.xml
@Eism Eism merged commit 652adea into musescore:4.7 Apr 20, 2026
13 checks passed
@Eism Eism deleted the fretboard_diagrams_update_4.7 branch April 20, 2026 06:46
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.

Fix Em7 and Dm7 fretboard diagrams

3 participants