Skip to content

Chord.Parse drops everything after the slash-bass note: "C/Gm", "C/E/G" and "C/Bb7" parse as "C/G", "C/E" and "C/A#" #344

Description

@matt-edmondson

What's wrong

After a /, Chord.TryParse reads the bass note with TryParseRoot (Semantics.Music/Chord.cs:108-116, 213-225). TryParseRoot reads one letter plus any accidentals and stops there. Nothing then checks that the rest of the bass text was used.

The leftover-text check added for #280 (Chord.cs:80-86, IsQualityVocabulary) only looks at the text before the slash. Anything after the bass note is therefore dropped without an error.

PitchClass.TryParse already has the missing check: at PitchClass.cs:141-144 it returns false when index != text.Length.

Reproduced

These results come from running each input through the built library at 7167117:

Input Result today Expected
C/Gm C/G reject
C/E/G C/E reject
Am/G/F Am/G reject
C7/G#m7b5 C7/G# reject
C/Bb7 C/A# reject
C/Gxyz C/G reject

Progression.Parse, ChordEvent.Parse and Section/Arrangement parsing all go through Chord.TryParse. A typo in a chart is therefore accepted silently instead of failing.

Suggested fix

Parse the bass with PitchClass.TryParse(symbol[(slash + 1)..], …), or require bassIndex == bassText.Length. A bass that does not parse cleanly must still fall through to the existing 6/9 rewrite, so that C6/9 and C6/9/G keep working.

Acceptance criteria

  • Every input in the table is rejected.
  • C/G, Dm7/G, C6/9, Cm6/9, C6/9/G and Db/Cb still parse.
  • The round-trip corpus still passes.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions