From 58ebc93eb5953fa342775ea2cbc6f66d10d7cecd Mon Sep 17 00:00:00 2001
From: Tony Garnock-Jones <tonyg@leastfixedpoint.com>
Date: Wed, 15 May 2024 10:49:30 +0200
Subject: [PATCH] Repair comment reading and trailing comments in pexprs

---
 .../javascript/packages/core/src/pexpr.ts     | 34 +++++++++----------
 .../javascript/packages/core/src/reader.ts    |  5 ++-
 .../packages/core/test/pexpr.test.ts          | 18 ++++++++++
 3 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/implementations/javascript/packages/core/src/pexpr.ts b/implementations/javascript/packages/core/src/pexpr.ts
index 9d15420..c294f58 100644
--- a/implementations/javascript/packages/core/src/pexpr.ts
+++ b/implementations/javascript/packages/core/src/pexpr.ts
@@ -165,37 +165,34 @@ export class Reader extends ReaderBase<never> {
     }
 
     readCompound(c: Compound, terminator: string): Compound {
-        while (true) {
-            this.state.skipws();
-            if (this.state.peek() === terminator) {
-                this.state.advance();
-                return c;
-            }
-            if (!this.readExpr(c)) {
-                this.state.error(`Missing "${terminator}"`, this.state.copyPos());
-            }
-        }
+        while (this.readExpr(c, terminator)) {}
+        return c;
     }
 
     readSimpleExpr(c: BaseCompound<SimpleExpr>): boolean {
         return this._readInto(c, false);
     }
 
-    readExpr(c: BaseCompound<Expr>): boolean {
-        return this._readInto(c as BaseCompound<SimpleExpr> /* yuck */, true);
+    readExpr(c: BaseCompound<Expr>, terminator: string | null = null): boolean {
+        return this._readInto(c as BaseCompound<SimpleExpr> /* yuck */, true, terminator);
     }
 
-    _readInto(c: BaseCompound<SimpleExpr>, acceptPunct: boolean): boolean {
+    _checkTerminator(actual: string, expected: string | null, startPos: Position): false {
+        if (actual === expected) return false;
+        this.state.error('Unexpected ' + actual, startPos);
+    }
+
+    _readInto(c: BaseCompound<SimpleExpr>, acceptPunct: boolean, terminator: string | null = null): boolean {
         while (true) {
             this.state.skipws();
-            if (this.state.atEnd()) return false;
+            if (this.state.atEnd() && terminator === null) return false;
             const startPos = this.state.copyPos();
             const ch = this.state.nextchar();
             switch (ch) {
                 case '"':
                     return c.push(this.state.readString('"'), startPos);
                 case '|':
-                    return c.push(Symbol.for(this.state.readString('|')), startPos);                    return true;
+                    return c.push(Symbol.for(this.state.readString('|')), startPos);
                 case ';':
                     if (acceptPunct) {
                         return (c as BaseCompound<Expr>).push(new Punct(';'), startPos);
@@ -264,9 +261,10 @@ export class Reader extends ReaderBase<never> {
                 case '<': return c.push(this.readCompound(new Record(), '>'), startPos);
                 case '[': return c.push(this.readCompound(new Sequence(), ']'), startPos);
                 case '{': return c.push(this.readCompound(new Block(), '}'), startPos);
-                case '>': this.state.error('Unexpected >', startPos);
-                case ']': this.state.error('Unexpected ]', startPos);
-                case '}': this.state.error('Unexpected }', startPos);
+                case ')': return this._checkTerminator(ch, terminator, startPos);
+                case '>': return this._checkTerminator(ch, terminator, startPos);
+                case ']': return this._checkTerminator(ch, terminator, startPos);
+                case '}': return this._checkTerminator(ch, terminator, startPos);
                 case ',':
                     if (acceptPunct) {
                         return (c as BaseCompound<Expr>).push(new Punct(','), startPos);
diff --git a/implementations/javascript/packages/core/src/reader.ts b/implementations/javascript/packages/core/src/reader.ts
index fca350a..42c46a8 100644
--- a/implementations/javascript/packages/core/src/reader.ts
+++ b/implementations/javascript/packages/core/src/reader.ts
@@ -254,10 +254,9 @@ export class ReaderState {
     readCommentLine(): string {
         let acc = '';
         while (true) {
+            if (this.atEnd()) return acc;
             const c = this.nextchar();
-            if (c === '\n' || c === '\r') {
-                return acc;
-            }
+            if (c === '\n' || c === '\r') return acc;
             acc = acc + c;
         }
     }
diff --git a/implementations/javascript/packages/core/test/pexpr.test.ts b/implementations/javascript/packages/core/test/pexpr.test.ts
index 30ffb25..1e74746 100644
--- a/implementations/javascript/packages/core/test/pexpr.test.ts
+++ b/implementations/javascript/packages/core/test/pexpr.test.ts
@@ -25,3 +25,21 @@ describe('basics', () => {
         expect(v).is(P(s));
     });
 });
+
+describe('trailing comments', () => {
+    it('basics 1', () => {
+        const d = new Pexpr.Reader('# a comment with nothing after').nextDocument();
+        expect(d.annotations?.[d.exprs.length].get(0)?.item).toBe('a comment with nothing after');
+    });
+
+    it('basics 2', () => {
+        const d = new Pexpr.Reader('# a comment with nothing after\n').nextDocument();
+        expect(d.annotations?.[d.exprs.length].get(0)?.item).toBe('a comment with nothing after');
+    });
+
+    it('inside a sequence', () => {
+        const d = new Pexpr.Reader('[\n1\n# a comment with nothing after\n]\n').nextDocument();
+        const seq = d.get(0)?.item as Pexpr.Compound;
+        expect(seq.annotations?.[seq.exprs.length].get(0)?.item).toBe('a comment with nothing after');
+    });
+});
\ No newline at end of file