From d71553c71b25cfbcc4ea1a02d5901f61b0067171 Mon Sep 17 00:00:00 2001 From: Sri Panyam Date: Mon, 21 Sep 2026 10:00:45 +0000 Subject: [PATCH 1/2] Fixing paths --- docs/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Makefile b/docs/Makefile index bc0c07e..79d50cd 100644 --- a/docs/Makefile +++ b/docs/Makefile @@ -43,7 +43,7 @@ gh-pages: build git add -A && \ git commit -m "Deploy to GitHub Pages" && \ git branch -M gh-pages && \ - git remote add origin git@panyam-github:panyam/notations.git && \ + git remote add origin git@github.com:panyam/notations.git && \ git push -f origin gh-pages @echo "Deployed! Enable GitHub Pages in repo settings to serve from gh-pages branch" @rm -rf dist/docs/.git From ed769aa98a4074ca5a5677fb43597c30e8b25780 Mon Sep 17 00:00:00 2001 From: Sri Panyam Date: Mon, 21 Sep 2026 10:08:44 +0000 Subject: [PATCH 2/2] Accept silent spaces and bar separators, and report unknown commands Three fixes in the grammar and the rule handlers, all reported from the docs audit that came out of the example rewrite. A standalone _ is a silent space. The Space -> UNDER_SCORE production has been in the grammar from the start with no token declared behind it, so _ lexed as an ordinary identifier character and rendered as visible text. The token is guarded by a lookahead, /_(?!{IdentChar})/, so only an underscore that stands alone becomes a space. ___ stays a literal, as does an underscore inside a word, which keeps anyone writing ___ as a label working. Write _ _ _ for three silent spaces. | and || in a role line are skipped. beatviews.ts already draws a line at the end of each bar and a double line at the end of the avartanam from the cycle, so a typed bar was always redundant with what gets drawn. Skipping means a misplaced bar is not an error either, which is the cost of the one-line version; checking a bar against the cycle needs the layout pass and is a feature rather than a grammar fix. An unknown command is recorded as a ParseError instead of thrown. createCommand runs inside a rule handler, so its throw escaped parse and load entirely and reached the caller as an exception while every other parse failure arrived in the errors array. load() had two failure shapes and its signature advertised one. The command and any block attached to it are dropped, and parsing carries on. One existing assertion changes. docs/tests/playground-patterns.spec.ts asserted .toThrow("Invalid command"), which pinned the old behaviour, and now asserts the error is reported. 20 assertions added, 1 replaced, none weakened. Fixes #17, fixes #19, fixes #28 --- docs/content/reference/syntax/index.html | 22 +++++- docs/tests/playground-patterns.spec.ts | 8 +- src/parser.ts | 23 +++++- src/tests/parser.spec.ts | 95 ++++++++++++++++++++++++ src/tests/regressions.spec.ts | 28 +++++++ 5 files changed, 165 insertions(+), 11 deletions(-) diff --git a/docs/content/reference/syntax/index.html b/docs/content/reference/syntax/index.html index 3a6ba16..f2a7877 100644 --- a/docs/content/reference/syntax/index.html +++ b/docs/content/reference/syntax/index.html @@ -54,6 +54,22 @@

Whitespace

Both lines produce identical output.

+

Bar Separators

+ +

A | or || inside a role line is ignored the same way whitespace is, so +you can mark avartanam boundaries in the source if you find them easier to read. The renderer draws +its own lines from the cycle either way, one at the end of each bar and a double line at the end of +the avartanam, so a bar you type is never what you see:

+ + +\cycle("|4|2|2|") +Sw: S R G M | P D | N S. || + + +

Because they are ignored rather than checked, a bar in the wrong place is not an error. The +| characters inside \cycle("|4|2|2|") are a different thing entirely, +since that argument is a quoted string and the bars in it define the cycle.

+

Spaces (Karvais)

Spaces represent empty durations where the previous note continues. Use commas:

@@ -69,7 +85,7 @@

Spaces (Karvais)

,Space (Karvai)1 unit ;Double space2 units - _Silent space (not implemented yet)1 unit (not displayed) + _Silent space1 unit (not displayed) @@ -77,7 +93,9 @@

Spaces (Karvais)

S , R ; G , , M -

Silent spaces are meant to consume duration without displaying the space symbol, to avoid clutter. They are not implemented yet, so use , for now.

+

Silent spaces consume duration without displaying the space symbol, to avoid clutter. Only a +standalone _ counts as one, so write _ _ _ for three. A run such as +___ is an ordinary literal, as is an underscore inside a word.

Rests

diff --git a/docs/tests/playground-patterns.spec.ts b/docs/tests/playground-patterns.spec.ts index 80de327..0b2da32 100644 --- a/docs/tests/playground-patterns.spec.ts +++ b/docs/tests/playground-patterns.spec.ts @@ -152,12 +152,10 @@ describe("Playground Patterns", () => { expect(addAtomsCmd!.atoms.length).toBe(3); }); - it("should throw on invalid commands", () => { + it("should report invalid commands as errors rather than throwing", () => { const parser = new Parser(); - // Invalid commands throw an error - expect(() => { - parser.parse("S R G \\invalidCommand"); - }).toThrow("Invalid command"); + parser.parse("S R G \\invalidCommand"); + expect(parser.errors.map((e: any) => e.message)).toEqual(["Invalid command: invalidcommand"]); }); }); }); diff --git a/src/parser.ts b/src/parser.ts index 036aad4..ea1371d 100644 --- a/src/parser.ts +++ b/src/parser.ts @@ -67,11 +67,13 @@ const [parser, tokenFunc /*itemGraph*/] = G.newParser( %token DOTS_IDENT /(\.+)({IdentChar}+)/ { toOctavedNote } %token IDENT_DOTS /({IdentChar}+)(\.+)/ { toOctavedNote } %token IDENT_COLON /{IdentChar}+:/ { toRoleSelector } + %token UNDER_SCORE /_(?!{IdentChar})/ %token IDENT /{IdentChar}+/ %token BSLASH_IDENT /\\{IDENT}/ { toCommandName } %token BSLASH_NUMBER /\\{NUMBER}/ %token HYPHEN /-/ %skip /[ \t\n\f\r]+/ + %skip /\|\|?/ %skip /\/\/[^\n]*/ %skip /\/\*.*?\*\// @@ -477,6 +479,9 @@ export class Parser { const blockCommands = children[2].value as Command[] | null; const innerCommand = this.createCommand(name, params); + // The block's commands were already spliced out of this.commands by endBlock, so an unknown + // command takes its block down with it rather than leaving those commands at the top level. + if (innerCommand === null) return null; if (blockCommands !== null) { // Wrap in BlockCommand @@ -503,8 +508,8 @@ export class Parser { return children[1].value; }, appendCommand: (rule: G.Rule, parent: G.PTNode, ...children: G.PTNode[]) => { - const command = children[1].value as Command; - this.addCommand(command); + const command = children[1].value as Command | null; + if (command !== null) this.addCommand(command); const atoms = children[2].value as Atom[]; if (atoms.length > 0) { @@ -540,7 +545,16 @@ export class Parser { // config = config || {}; } - createCommand(name: string, params: CmdParam[]): Command { + /** + * Builds the command a `\\name(...)` directive names. + * + * Returns null for a name with no command behind it, after recording a ParseError on this + * parser, rather than throwing. Rule handlers run inside galore's parse loop, so a throw + * escapes `parse` and `load` entirely and reaches the caller as an exception while every other + * parse failure arrives in the errors array. Callers get one failure shape by being handed a + * null here and reading `errors`. + */ + createCommand(name: string, params: CmdParam[]): Command | null { const lName = name.trim().toLowerCase(); params = params || []; if (lName == "line") { @@ -564,7 +578,8 @@ export class Parser { return new Repeat(params); } else { // Try to set this as the current role - throw new Error("Invalid command: " + lName); + this.errors.push(new G.ParseError("Invalid command: " + lName, "InvalidCommand", lName)); + return null; } } diff --git a/src/tests/parser.spec.ts b/src/tests/parser.spec.ts index 4a7e146..68741b2 100644 --- a/src/tests/parser.spec.ts +++ b/src/tests/parser.spec.ts @@ -977,3 +977,98 @@ describe("Comment Tests", () => { ]); }); }); + +describe("Silent Space Tests", () => { + function atomsOf(input: string): any[] { + const parser = new Parser(); + parser.parse(input); + expect(parser.errors).toEqual([]); + const addAtoms = parser.commands.map((c: any) => c.debugValue()).find((c: any) => c.atoms); + return addAtoms.atoms; + } + + test("A lone underscore is a silent space", () => { + expect(atomsOf(`Sw: S _ R`)).toEqual([ + { type: "Literal", value: "S" }, + { type: "Space", isSilent: true }, + { type: "Literal", value: "R" }, + ]); + }); + + test("Separated underscores are one silent space each", () => { + expect(atomsOf(`Sw: _ _ _`)).toEqual([ + { type: "Space", isSilent: true }, + { type: "Space", isSilent: true }, + { type: "Space", isSilent: true }, + ]); + }); + + test("A comma is a space that is not silent", () => { + expect(atomsOf(`Sw: S , R`)).toEqual([ + { type: "Literal", value: "S" }, + { type: "Space", isSilent: false }, + { type: "Literal", value: "R" }, + ]); + }); + + // The token is guarded by a lookahead so that only a standalone underscore becomes a space. + // Runs and embedded underscores stay literals, which is what they were before the token existed. + test("An underscore run stays a literal", () => { + expect(atomsOf(`Sw: S ___ R`)).toEqual([ + { type: "Literal", value: "S" }, + { type: "Literal", value: "___" }, + { type: "Literal", value: "R" }, + ]); + }); + + test("An underscore inside a word stays part of the word", () => { + expect(atomsOf(`Sw: S_N_D _abc`)).toEqual([ + { type: "Literal", value: "S_N_D" }, + { type: "Literal", value: "_abc" }, + ]); + }); + + test("Inside a group a silent space differs from a comma only by isSilent", () => { + const silent = atomsOf(`Sw: [S _ R]`); + const comma = atomsOf(`Sw: [S , R]`); + expect(silent[0].atoms[1]).toEqual({ type: "Space", isSilent: true }); + expect(comma[0].atoms[1]).toEqual({ type: "Space", isSilent: false }); + expect({ ...silent[0], atoms: null }).toEqual({ ...comma[0], atoms: null }); + }); +}); + +describe("Bar Separator Tests", () => { + function parseOf(input: string): Parser { + const parser = new Parser(); + parser.parse(input); + return parser; + } + + function expectSameAsUnbarred(barred: string, unbarred: string): void { + const withBars = parseOf(barred); + expect(withBars.errors).toEqual([]); + expect(withBars.commands.map((c: any) => c.debugValue())).toEqual( + parseOf(unbarred).commands.map((c: any) => c.debugValue()), + ); + } + + test("Single and double bars in a role line are ignored", () => { + expectSameAsUnbarred(`Sw: S R G M | P D | N S. ||`, `Sw: S R G M P D N S.`); + }); + + test("A bar with no surrounding whitespace is ignored", () => { + expectSameAsUnbarred(`Sw: S R|G M`, `Sw: S R G M`); + }); + + test("A bar between commands is ignored", () => { + expectSameAsUnbarred(`\\beatDuration(2) | Sw: S R`, `\\beatDuration(2) Sw: S R`); + }); + + // The skip rule must not reach inside a quoted string, which is where the cycle spec lives. + test("Bars inside the cycle spec still define the bars", () => { + const parser = parseOf(`\\cycle("|4|2|2|")\nSw: S R G M | P D | N S. ||`); + expect(parser.errors).toEqual([]); + const cycle = parser.commands[0].debugValue().params[0].value; + expect(cycle).toEqual("|4|2|2|"); + }); +}); diff --git a/src/tests/regressions.spec.ts b/src/tests/regressions.spec.ts index b45eb3e..d2cb3f8 100644 --- a/src/tests/regressions.spec.ts +++ b/src/tests/regressions.spec.ts @@ -68,3 +68,31 @@ describe("Parser State Isolation", () => { ]); }); }); + +describe("Invalid Commands", () => { + const BAD_COMMAND = `\\cycle("|4|")\nSw: S R G M\n\\nosuchcommand\n`; + + test("An unknown command is reported rather than thrown", () => { + const [, , errors] = load(BAD_COMMAND); + expect(errors.map((e: any) => e.message)).toEqual(["Invalid command: nosuchcommand"]); + }); + + test("Parsing continues past an unknown command", () => { + const parser = new Parser(); + parser.parse(BAD_COMMAND); + const names = parser.commands.map((c: any) => c.debugValue().name); + expect(names).toEqual(["SetCycle", "ActivateRole", "AddAtoms"]); + }); + + test("An unknown command carrying a block drops the block without crashing", () => { + const parser = new Parser(); + parser.parse(`\\cycle("|2|")\n\\nosuchcommand {\n mrid: tham ,\n}\nmrid: thi ,\n`); + expect(parser.errors.map((e: any) => e.message)).toEqual(["Invalid command: nosuchcommand"]); + expect(parser.commands.map((c: any) => c.debugValue().name)).toEqual(["SetCycle", "ActivateRole", "AddAtoms"]); + }); + + test("A later load is unaffected by an earlier invalid command", () => { + expect(load(BAD_COMMAND)[2].length).toBeGreaterThan(0); + expect(load(GOOD)[2]).toEqual([]); + }); +});