Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace TreeAssociator with a function #125

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Pennycook
Copy link
Contributor

Using the visitor pattern allows TreeAssociator to be replaced by an associate() function inside of ParserState.

Related issues

Part of #101.

Proposed changes

  • Remove TreeAssociator class.
  • Add associate() function to visit each node in the tree and update ParserState directly.

One interesting thing here is that the old TreeAssociator class used some pretty complicated recursive logic to decide whether to expand specific branches when processing a given set of #if/#else/#endif nodes. I think the new code is clearer, but because the visitor isn't being called recursively, the association logic requires an explicit stack to track the state of each branch.

Using the visitor pattern allows TreeAssociator to be replaced by an
associate() function inside of ParserState.

Signed-off-by: John Pennycook <[email protected]>
@Pennycook Pennycook added the refactor Improvements to code structure label Nov 5, 2024
@Pennycook Pennycook added this to the 2.0.0 milestone Nov 5, 2024
@Pennycook Pennycook mentioned this pull request Nov 5, 2024
@Pennycook Pennycook closed this Nov 5, 2024
@Pennycook Pennycook reopened this Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Improvements to code structure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant