Fix sefer hachinukh linker #1948
Merged
Conversation
This file contains 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
…ResolvedRefs when at least one is mapped to an AltStructNode
…eferenceable_book_node.py
…bleBookNode child This makes a test past for ReferenceableBookNode.contains() in a case that may not come up in production but is possible to happen based on the function's contract.
…erenceableBookNode
…s shared between Named Numbered and Map nodes
…cts instead of ReferenceableBookNodes
yonadavGit
approved these changes
Oct 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The linker requires a ResolvedRef to match all RefParts in the input to be considered valid. In the case where there are redundant parts we end of with two (or more) ResolvedRefs each of which only matches some of the RefParts. However, there may a some combination of ResolvedRefs that overlap and when combined span all the RefParts.
For example: ספר חיונוך לך לך ב. Currently the linker catches two partial matches: ספר החינוך ב and ספר החינוך לך לך. There is already logic to try to merge these. However since ספר החינוך לך לך matches an AltStructNode which has no associated ref (since this node has ArrayMapNode children but is not itself an ArrayMapNode) there wasn't an easy way to check for overlapping matches. This PR allows for this.