Skip to content

Duration, Velocity, TimeSignature, Interval, Note and Tempo accept whitespace and '+' that ToString never prints: Duration.Parse(" 1 / 4 "), Note.Parse("C4: 1/4 :v 64 ") #471

Closed

Description

@matt-edmondson

What's wrong

#307 changed Pitch.TryParse to parse the octave with NumberStyles.AllowLeadingSign only. The comment at Pitch.cs:~89-93 explains why: whitespace or '+' "would parse to a pitch that prints back differently".

The sibling Music value types still use NumberStyles.Integer, which allows leading and trailing whitespace and a leading sign:

File What it uses
Duration.cs:113-114 NumberStyles.Integer | AllowLeadingSign for numerator and denominator
Velocity.cs:68 NumberStyles.Integer
TimeSignature.cs:76-77 NumberStyles.Integer
Interval.cs:61 NumberStyles.Integer | AllowLeadingSign
Tempo.cs:92 NumberStyles.Float, which also allows surrounding whitespace; the beat goes through Duration.TryParse

Note, Rest, ChordEvent and Progression cells inherit the same leniency.

Reproduction (all return true)

Input Prints as
Duration.TryParse(" 1 / 4 "), Duration.TryParse("+1/+4") 1/4
Velocity.TryParse(" 64 "), Velocity.TryParse("+64") 64
TimeSignature.TryParse(" 3 / 4 "), TimeSignature.TryParse("+3/4") 3/4
Interval.TryParse(" 7 "), Interval.TryParse("+7") 7
Note.TryParse("C4: 1/4 :v 64 "), Note.TryParse("C4:+1/4:v+64") C4:1/4:v64
Tempo.TryParse(" 120 bpm@1/4"), Tempo.TryParse("120bpm@ 1/4 ") 120bpm@1/4

Pitch.TryParse("C 4") and Pitch.TryParse("C+4") return false, so the Music types disagree about whether to accept a non-canonical spelling.

Suggested fix / acceptance criteria

  • Parse integer fields with NumberStyles.None.
  • Where a sign is meaningful (Interval semitones, and a negative Duration numerator if that is kept), allow a leading - and reject a leading +. Reuse the same helper as Pitch.
  • In Tempo, reject whitespace before or after the BPM.
  • Tests: every input above returns false, and X.Parse(x.ToString()) == x still holds.

Related but distinct: #434 (the Duration.TryParse overflow on "-2147483648/-1").

Activity

  1. matt-edmondson commented on Oct 18, 2026

    @matt-edmondson
    ContributorAuthor

    Triage


    Generated by Claude Code

  2. matt-edmondson commented on Oct 29, 2026

    @matt-edmondson
    ContributorAuthor

    Fixed by #487 (commit 47dffbb, now on main). The commit cites this issue inline as "(#471)" rather than with a "Fixes" keyword, so GitHub didn't close it. Closing by hand.


    Generated by Claude Code

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

Metadata

Metadata

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