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

Make class directive work for simple cases #221

Merged
merged 3 commits into from
Mar 20, 2023

Conversation

linawolf
Copy link
Contributor

@linawolf linawolf commented Mar 5, 2023

class directives applied to neighboring nodes post-phoned for now

@linawolf linawolf force-pushed the class-directive branch 2 times, most recently from e734c83 to f702f23 Compare March 5, 2023 15:04
@wouterj
Copy link
Contributor

wouterj commented Mar 5, 2023

class directives applied to neighboring nodes post-phoned for now

When the changes from #21 are merged (I believe Jaap started working based on my commits there), this should be relatively easy by looping over the node tree and applying class nodes to the next node.

@jaapio
Copy link
Member

jaapio commented Mar 5, 2023

The compiler is already there. So this can be done with a new compiler pass.

@jaapio
Copy link
Member

jaapio commented Mar 7, 2023

I would suggest adding a none rendered node that represents the class in the node tree. This will help us to do some compiling after the parser is ready.

Let's suggest that we have a ClassNode that contains the class / classes set by the directive.

Then we could implement a new \phpDocumentor\Guides\Compiler\NodeTransformer like this.

final class ClassNodeTransformer implements \phpDocumentor\Guides\Compiler\NodeTransformer
{
    public function enterNode(Node $node): Node
    {
       if ($node instanceOf ClassNode) {
          $this->class = $node->getClasses();
          return $node;
       }
       
       $node->setClasses($this->class);
    }

    public function leaveNode(Node $node): ?Node
    {
        if ($node instanceOf ClassNode) {
           //Remove the class node from the tree. 
           return null;
        }
    }

    public function supports(Node $node): bool
    {
        //Should we do something here to detect nodes are supported? Or could every node have a class?
        return true;
    }
} 

I did not test this and the code may contain issues, but I think it could be something like this.

If you need more complex logic I would recommend you have a look at the \phpDocumentor\Guides\Compiler\CompilerPass interface, a compiler pass will get all Documents and allows you to apply more complex transformations. But if you think a node transformer might work please use that. As the compiler passes are more complex that will leave you on your own about our AST.

class directives applied to neighboring nodes post-phoned for now
@jaapio
Copy link
Member

jaapio commented Mar 18, 2023

Appart from the question about the reset of classes, this is great! thanks for this contribution

Move tests to integration tests as NodeTransformers are not applied in the functional tests
@jaapio jaapio merged commit 28a503e into phpDocumentor:main Mar 20, 2023
@jaapio
Copy link
Member

jaapio commented Mar 20, 2023

Cool to have this feature in. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants