diff --git a/chess/pgn.py b/chess/pgn.py index 3d4d7e5ff..5451762a6 100644 --- a/chess/pgn.py +++ b/chess/pgn.py @@ -1206,6 +1206,7 @@ def begin_game(self) -> None: self.variation_stack: List[GameNode] = [self.game] self.starting_comments: list[str] = [] + self.starting_comments_stack: List[list[str]] = [] self.in_variation = False @override @@ -1227,10 +1228,32 @@ def begin_variation(self) -> None: self.variation_stack.append(parent) self.in_variation = False + # Starting comments are scoped to the variation they were + # collected in. Save any pending comments of the enclosing + # variation (or game) so they are not mixed with this + # variation's starting comments. + self.starting_comments_stack.append(self.starting_comments) + self.starting_comments = [] + @override def end_variation(self) -> None: self.variation_stack.pop() + if self.starting_comments: + # The variation contained no move that the pending starting + # comments could attach to (for example an empty variation, + # or a variation whose moves were all skipped after an + # error). Attach the comments to the node the variation + # branched from, rather than leaking them onto the next + # move: that move does not start a variation, so it must + # not carry starting comments, and exporting them there + # would silently re-attach them to the preceding move. + node = self.variation_stack[-1] + node.comments.extend(self.starting_comments) + node.comments = list(filter(None, node.comments)) + + self.starting_comments = self.starting_comments_stack.pop() if self.starting_comments_stack else [] + @override def visit_result(self, result: str) -> None: if self.game.headers.get("Result", "*") == "*": diff --git a/test.py b/test.py index a481c2adb..7b2902ea5 100755 --- a/test.py +++ b/test.py @@ -2430,6 +2430,89 @@ def test_game_starting_variation(self): self.assertFalse(node.comments) self.assertEqual(node.starting_comments, ["Start of variation"]) + def test_starting_comment_in_empty_variation(self): + # A comment inside a variation that contains no move must not + # leak onto the next move: only nodes that actually start a + # variation can have starting comments. Instead, the comment is + # preserved on the node the variation branched from. + game = chess.pgn.read_game(io.StringIO("1. e4 e5 ({lonely}) 2. Nf3 Nc6 *")) + e5 = game[0][0] + nf3 = e5[0] + self.assertFalse(nf3.starts_variation()) + self.assertEqual(nf3.starting_comments, []) + self.assertEqual(e5.comments, ["lonely"]) + + # Export round trips are the identity: the comment is no longer + # re-attached to a different node by exporting and re-parsing. + exporter = chess.pgn.StringExporter(columns=None) + game.accept(exporter) + reparsed = chess.pgn.read_game(io.StringIO(str(exporter))) + self.assertEqual(reparsed[0][0].comments, ["lonely"]) + self.assertEqual(reparsed[0][0][0].starting_comments, []) + + # Nested empty variation. + game = chess.pgn.read_game(io.StringIO("1. e4 e5 2. Nf3 (2. Bc4 ({lonely}) Bc5 {gioco}) Nc6 *")) + bc4 = game[0][0][1] + bc5 = bc4[0] + self.assertEqual(bc5.starting_comments, []) + self.assertEqual(bc4.comments, ["lonely"]) + self.assertEqual(bc5.comments, ["gioco"]) + + # A comment in an empty variation at the end of the game is + # preserved, not dropped. + game = chess.pgn.read_game(io.StringIO("1. e4 e5 ({lonely end}) *")) + self.assertEqual(game[0][0].comments, ["lonely end"]) + + # A comment in an empty variation does not become the starting + # comment of a sibling variation either. + game = chess.pgn.read_game(io.StringIO("1. e4 e5 ({lonely}) (1... c5 {sic}) 2. Nf3 Nc6 *")) + c5 = game[0][1] + self.assertEqual(c5.starting_comments, []) + self.assertEqual(c5.comments, ["sic"]) + self.assertEqual(game[0][0].comments, ["lonely"]) + + # Pending starting comments are scoped to their variation: an + # outer starting comment survives an inner empty variation. + game = chess.pgn.read_game(io.StringIO("1. e4 e5 ({outer} ({inner-empty}) 1... c5) 2. Nf3 *")) + c5 = game[0][1] + self.assertTrue(c5.starts_variation()) + self.assertEqual(c5.starting_comments, ["outer"]) + all_comments = [] + + def collect(node): + all_comments.extend(node.comments) + for child in node.variations: + collect(child) + + collect(game) + self.assertIn("inner-empty", all_comments) + + # Invariant: no node carries starting comments without starting + # a variation. + samples = [ + "1. e4 e5 ({lonely}) 2. Nf3 Nc6 *", + "1. e4 e5 2. Nf3 (2. Bc4 ({lonely}) Bc5) Nc6 *", + "1. e4 e5 ({a}) (1... c5) 2. Nf3 *", + "1. e4 ({pre} 1. d4 d5) e5 *", + "1. e4 e5 ({x} 1... c5 {y} ({z}) 2. Nf3) 2. Bc4 *", + ] + for movetext in samples: + game = chess.pgn.read_game(io.StringIO(movetext)) + + def check(node): + for child in node.variations: + if child.starting_comments: + self.assertTrue(child.starts_variation()) + check(child) + + check(game) + + # Legitimate starting comments are unaffected. + game = chess.pgn.read_game(io.StringIO("1. e4 e5 ({the french} 1... e6 2. d4 d5) 2. Nf3 Nc6 *")) + e6 = game[0][1] + self.assertTrue(e6.starts_variation()) + self.assertEqual(e6.starting_comments, ["the french"]) + def test_annotation_symbols(self): pgn = io.StringIO("1. b4?! g6 2. Bb2 Nc6? 3. Bxh8!!") game = chess.pgn.read_game(pgn)