Repository navigation
Parse music charts strictly and write bar lines where the bars are - #487
Merged
Merged
Conversation
…are [minor] - Integer fields in Duration, Velocity, TimeSignature, Interval and Pitch take no whitespace or '+', and a Duration denominator no sign; Tempo's BPM takes no whitespace or sign (#471). - Note, Rest and ChordEvent reject a zero or negative duration in Create and TryParse, as does a Progression "chord@n/d" cell. Duration itself stays a signed rational (#353). - Progression.Parse rejects a "/" after a "chord@n/d" cell instead of dropping it (#328). "|" stays a visual aid that Parse ignores, which the spec now says, since "4/4 C | G" shorthand is used throughout. - Progression.ToString tracks the bar position exactly and writes "|" wherever a bar ends between tokens, including inside a held chord (#475). - Arrangement.Parse treats any run of blank or whitespace-only lines as one section separator (#339). - Form.Of names AABA the 32-bar form only when every section is eight bars (#384), recognizes VerseChorus from section types (#321, #384), and Form text carries the name when the pattern does not imply it, as "A (TwelveBarBlues)", so every Form round-trips (#447). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zHCYynCj4HwiWQqff3Kkf
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zHCYynCj4HwiWQqff3Kkf
|
This was referenced Oct 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Requested by Matt · project thread
Fixes #328, #339, #353, #384, #321, #447, #471, #475.
Parsing
Duration,Velocity,TimeSignature,Interval,NoteandTempoaccept whitespace and '+' thatToStringnever prints:Duration.Parse(" 1 / 4 "),Note.Parse("C4: 1/4 :v 64 ")#471: integer fields inDuration,Velocity,TimeSignature,IntervalandPitchgo through one helper (Notation.TryParseInteger) that rejects whitespace and a leading+. ADurationdenominator takes no sign at all, sinceToStringnever writes one, so"1/-4"and"-2147483648/-2"are now rejected (the test that accepted the latter is changed).Tempo's BPM takes no whitespace or sign but keeps the exponent that theRformat writes.Durationaccepts zero and negative lengths, soNote,Rest,ChordEventandProgression.Parseaccept"C4:-1/4:v64","R:0/4","C@-1/4"#353: I keptDurationas a signed rational and put the rule on the events.Note,RestandChordEventthrowArgumentOutOfRangeExceptionfromCreateand return false fromTryParsefor a zero or negative duration, and so does a progressionchord@n/dcell. The zero that seedsTotalDurationis unaffected.Progression.Parsesilently drops beat slashes after aChord@n/dcell:"4/4 C@1/2 / / | G"parses as C for 2 beats with G on beat 3 #328: a/after achord@n/dcell is rejected instead of dropped.|validation (Progression.Parsesilently drops beat slashes after aChord@n/dcell:"4/4 C@1/2 / / | G"parses as C for 2 beats with G on beat 3 #328 optional,Progression.ToString()puts bar lines in the wrong place: after a sub-beat chord|lands mid-bar, and a chord held across a barline gets none #475 "decide separately"): I chose not to validate bar lines. The"4/4 C | G"shorthand (one-beat chords with bar lines between them) appears in about 25 tests across cadences, key inference and chromatic analysis, and validating would reject all of them. The spec now saysParseignores|.Arrangement.Parsetreats any run of empty or whitespace-only lines as one section separator.Writing
Progression.ToString()puts bar lines in the wrong place: after a sub-beat chord|lands mid-bar, and a chord held across a barline gets none #475:Progression.ToStringtracks the bar position as an exactDurationand writes|wherever a bar ends between two tokens, including between the slashes of a held chord (G | /). Both examples from the issue now produce their "correct chart" output. A bar that ends inside an@n/dcell has no token boundary, so it gets no bar line.Form
Form.Ofnames AABAThirtyTwoBarAABAonly when all four sections are exactly eight bars long. Otherwise the result isUnknown.Form.Parse("AABA")has no bar data, so it still names the 32-bar form, and the XML docs say so.NamedForm.VerseChorusis declared butFormnever produces it; the spec's own verse–chorus example comes outUnknown#321 / Form names any AABA arrangement ThirtyTwoBarAABA regardless of section length, and NamedForm.VerseChorus can never be produced #384:VerseChorusis now recognized from section types. The rule is at least two verses and two choruses, and every verse is answered by a chorus before the next verse or the end. Intro, pre-chorus, post-chorus, bridge, solo, interlude, outro and coda sections are allowed anywhere; aRefrainorOthersection rules it out. Blues is checked first, then verse-chorus, then AABA, then the letter pattern.ToStringappends(Name)only when the name differs from the one the pattern implies, as inA (TwelveBarBlues),AABA (Unknown)andABABCB (VerseChorus). Every other form prints exactly as it did before.Parsereads the suffix (only an exactNamedFormmember name), so everyFormround-trips with equality unchanged.The README's AABA example now uses eight-bar sections so its comment stays true.
Tests
New
StrictParsingTestscover every input from the issues.FormTestsadds AABA length, verse-chorus, strophic, blues round-trip and malformed-name cases. The fullSemantics.Testsuite passes locally: 1813 passed, 12 skipped.Commit is tagged
[minor], because inputs that used to be accepted are now rejected andForm's text can carry a name.Generated by Claude Code