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

Unify visitor implementation for Parser2 with ASTVisitor #143

Open
Argmaster opened this issue Feb 3, 2024 · 1 comment
Open

Unify visitor implementation for Parser2 with ASTVisitor #143

Argmaster opened this issue Feb 3, 2024 · 1 comment
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@Argmaster
Copy link
Owner

After careful consideration I have came to conclusion that current introspection API based on Parser2Hooks introduces too much unnecessary complexity in form of nested classes. I can't change it now, as it will break API compatibility in awful way. Therefore I will schedule this change for PyGerber 3.0.0. I will also provide some kind of adapter for current API to future API, allowing for transition by changing just inheritance or by some decorator, it was not decided yet.

Current approach:

class CustomHooks(Parser2Hooks):
    def __init__(self) -> None:
        super().__init__()
        self.comments: list[str] = []

    class CommentTokenHooks(Parser2Hooks.CommentTokenHooks):
        hooks: CustomHooks

        def on_parser_visit_token(
            self,
            token: Comment,
            context: Parser2Context,
        ) -> None:
            self.hooks.comments.append(token.content)
            return super().on_parser_visit_token(token, context)

Future approach:

class CustomHooks(Parser2Hooks):
    def __init__(self) -> None:
        super().__init__()
        self.comments: list[str] = []

    def visit_comment_token(self, token: Comment) -> None:
        self.hooks.comments.append(token.content)
        return super().visit_comment_token(token, context)
@Argmaster Argmaster added the enhancement New feature or request label Feb 3, 2024
@Argmaster Argmaster added this to the 3.0.0 milestone Feb 3, 2024
@Argmaster Argmaster self-assigned this Apr 13, 2024
@Argmaster
Copy link
Owner Author

Actually, it may be a better approach to just move hooks into Parser2 class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Todo
Development

No branches or pull requests

1 participant