Skip to content

Section.ToString() doesn't round-trip labels containing "(" or a quote: "Verse (reprise)" fails to parse and a quoted word truncates the label #312

Closed

Description

@matt-edmondson

What's wrong

Section.ToString() writes the label raw between double quotes (Semantics.Music/Section.cs:79: sb.Append(" \"").Append(Label).Append('"')). Section.TryParse reads the header back with plain IndexOf:

  • The label ends at the first " after the opening quote (Section.cs:135-144).
  • The key starts at the first ( anywhere after ] (Section.cs:148). That includes a ( inside the label.

Label is documented as a free-form human label, and ToString as the canonical section text. The existing round-trip tests for Section and Arrangement don't use such labels.

Reproduction (run against the built assembly)

var prog = Progression.Parse("4/4 C F G C");
Section.TryParse(Section.Create(SectionType.Verse, prog, "Verse (reprise)").ToString(), out _)
    // false: "(reprise)" is handed to Key.TryParse
Section.TryParse(Section.Create(SectionType.Verse, prog, "Outro (fade)", Key.Parse("C major")).ToString(), out _)
    // false
Section.TryParse(Section.Create(SectionType.Verse, prog, "The \"big\" one").ToString(), out var r)
    // true, but r.Label == "The " (silently truncated)
Arrangement.TryParse(Arrangement.Create(Key.Parse("C major"), [sectionWithParenLabel]).ToString(), out _)
    // false: one such section makes the whole arrangement unparseable

A label containing a newline also splits the header line.

Why it matters

Labels such as "Chorus (x2)", "Verse (reprise)" and "Outro (fade)" are ordinary chart annotations. Today a section with one of them serializes to text the library can't read back, and a quote in a label is silently lost. This breaks the Parse(x.ToString()) == x contract that the type-safe-factories plan set for every music type.

Suggested fix / acceptance criteria

  • Pick one of these:
    • Escape " and \ inside the label on output, reject or escape newlines, and parse the quoted label with an escape-aware scanner.
    • Reject such labels in Section.Create.
  • Either way, search for the key's ( only after the label's closing quote.
  • Tests: Section.Parse(s.ToString()) == s for labels containing (, ), " and ], with and without a local key, and the same round trip through Arrangement.

Activity

  1. matt-edmondson commented on Sep 28, 2026

    @matt-edmondson
    ContributorAuthor

    Triage

    Notes: Escaping, rather than rejecting, keeps free-form labels useful. Whichever you choose, locate the key's ( after the label's closing quote. A round-trip test over the music types with adversarial labels would catch this whole family.


    Generated by Claude Code

  2. added a commit that references this issue on Oct 12, 2026
    671c888
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