Skip to content

KeybindingService.ParseChord/ParsePhrase duplicate Chord.Parse/Phrase.Parse, the drift that caused #107 #171

Description

@matt-edmondson

What's wrong

There are two independent parsers for the same strings:

  • KeybindingService.ParsePhrase and ParseChord (Keybinding/Services/KeybindingService.cs:188-227)
  • Chord.Parse and Phrase.Parse (Keybinding/Models/MusicalTypes.cs:312-336 and :439-451)

Each tokenizes with KeyStringTokenizer and builds Notes by hand. They produce the same result today only because both copies have been patched by hand:

The copies still differ in exception messages and paramName. The notes.Count == 0 guard at KeybindingService.cs:226 is dead code.

Why it matters

Every new parsing rule has to be added twice, for example the key-name equivalences in #166. Forgetting one copy reintroduces a #107-style bug: a binding that parses one way through the service and another way through the model, so it silently never matches.

Suggested fix

  • Make KeybindingService.ParseChord return Chord.Parse(chordString).
  • Make ParsePhrase return Phrase.Parse(phraseString).
  • Delete the duplicated loops and the dead guard.

Acceptance criteria

  • Only one implementation of chord and phrase parsing remains.
  • A test asserts that, over the existing alias, separator and invalid inputs, both entry points return equal values and throw the same exception types.

Activity

  1. matt-edmondson commented on Oct 6, 2026

    @matt-edmondson
    ContributorAuthor
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

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions