Add Scanner option to expand variations #56
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit is part of a series which adds a feature to the Scanner to expand variations into individual Game instances during parsing. This commit specifically adds Move.Clone(), a utility function to deep copy a Move instance.
This commit is part of a series which adds a feature to the Scanner to expand variations into individual Game instances during parsing. This commit specifically adds Game.Split(), the core functionality introduced by this series. Given a Game with a main line and possibly many variations, Game.Split() will return a slice of Games, 1 per variation, which each individually have only a single main line. This makes it convenient for callers who can then utilize Game.Moves() and other methods which only work on the main line.
This commit is part of a series which adds a feature to the Scanner to expand variations into individual Game instances during parsing. This commit specifically adds Scanner.ParseNext(). Scanner.ParseNext() is a higher level convenience iterator combining the functionality of ScanGame(), TokenizeGame(), NewParser(), and Parse(). This will later be extended to support expanded variations in part 4.
This commit is part of a series which adds a feature to the Scanner to expand variations into individual Game instances during parsing. This commit specifically adds ScannerOption, and the first ScannerOption WithExpandVariations(). Additionally this commit implements the option by taking advantage of part 2's Game.Split() function within Scanner.ParseNext() and caches the result in Scanner.nextParsedGames to be utilized in subsequent invocations.
This commit is part of a series which adds a feature to the Scanner to expand variations into individual Game instances during parsing. This commit specifically updates the documentation on scanning PGNs.
Contributor
There was a problem hiding this comment.
Pull Request Overview
Adds a new Scanner option to emit each variation as its own Game and simplifies parsing loops.
- Introduce
WithExpandVariationsand internal buffering (nextParsedGames) plus a newParseNextmethod. - Implement
Game.Splitto derive separate games from variations and addMove.Clonewith deep‐copy tests. - Update tests (
*_test.go) to useParseNextand cover expansion, splitting, and cloning behavior.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scanner.go | Added ScannerOpts, WithExpandVariations, updated HasNext, implemented ParseNext. |
| scanner_test.go | New tests for expanded and non-expanded variation parsing. |
| pgn_test.go | Refactored tests to call ParseNext instead of manual scan/tokenize/parse. |
| game.go | Added Game.Split, collectPaths, buildOneGameFromPath. |
| game_test.go | Tests for Game.Split with and without variations. |
| move.go | Added Move.Clone method. |
| move_test.go | assertMovesAreEqual helper and TestMoveClone for deep copy. |
| README.md | Updated examples to demonstrate ParseNext and WithExpandVariations. |
Comments suppressed due to low confidence (3)
README.md:396
- The example shows log.Fatal with formatting verbs. To match intended behavior, use log.Fatalf so the error is formatted correctly.
log.Fatal("Failed to parse game: %v", err)
move_test.go:469
- The error message references 's2' but is checking the 'promo' field; update the message to reflect 'promo'.
t.Fatalf("cloned mv %s s2 is not the same", m1)
move_test.go:483
- This message is in the branch checking children length but mentions 'len(command)'; it should say 'len(children)'.
t.Fatalf("cloned mv %s len(command) is not the same", m1)
Co-authored-by: Copilot <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These commits in aggregate add a feature to the Scanner to expand variations into individual Game instances during parsing. Together they address #52. Please note that tests will not pass until #55 (comment) is merged as the new tests added here depend on that bugfix. I intentionally didn't re-include it with this PR to avoid confusion.