Add new compiler pass to move anchors #371
Conversation
| use phpDocumentor\Guides\Nodes\CompoundNode; | ||
| use phpDocumentor\Guides\Nodes\DocumentNode; | ||
|
|
||
| class MoveAnchorPass implements CompilerPass |
There was a problem hiding this comment.
Shouldn't this be a node transformer instead? That already has all code to traverse the tree of nodes.
The way I differentiate between them is:
- Node transformers operate within a single document (and therefor can even be cached)
- Compiler passes operate in the project, managing cross document logic (like toctrees).
There was a problem hiding this comment.
I think the current node tranformer cannot handle this, as I need to move elements. But I could be wrong...
I have to make sure that I move elements at the right level, and in the node transformer, I don't know that?
There was a problem hiding this comment.
Hmm, maybe I have a wrong idea about the AST that we are dealing with here. But if it looks like this:
COMPOUND
...
ANCHOR
ANCHOR
COMPOUND
...
I think a transformer that supports compound and anchor nodes should work. It'll then store the anchors in a property when it discovers them, and push them in the compound node when it reaches one (clearing the property).
There was a problem hiding this comment.
I will check if it is possible to do this in a transformer, I think the main issue I will have is that the transformer will first handle the child nodes an then go to the next node. And its state is shared between the nodes.
ANCHOR1
COMPOUND
ANCHOR2
ANCHOR3
COMPOUND
ANCHOR4
COMPOUND
Should be come
COMPOUND
ANCHOR1
COMPOUND
ANCHOR2
ANCHOR3
COMPOUND
ANCHOR4
jaapio
force-pushed
the
feature/references
branch
from
d273fba to
2254c6e
Compare
jaapio
force-pushed
the
feature/references
branch
3 times, most recently
from
e152806 to
9991600
Compare
jaapio
force-pushed
the
feature/references
branch
from
3d365f9 to
ef99ff5
Compare
| @@ -50,7 +50,7 @@ private function traverseForTransformer( | |||
| } | |||
|
|
|||
| foreach ($shadowNode->getChildren() as $shadowChild) { | |||
| $this->traverseForTransformer($transformer, $shadowChild, $compilerContext); | |||
| $this->traverseForTransformer($transformer, $shadowChild, $compilerContext->withShadowTree($shadowChild)); | |||
There was a problem hiding this comment.
This was a bug, we should pass the current node in the shadow tree. So we can move around at the correct level.
jaapio
force-pushed
the
feature/references
branch
from
ef99ff5 to
9720a27
Compare
jaapio
requested review from
wouterj and
linawolf
and removed request for
wouterj
packages/guides/src/Compiler/NodeTransformers/MoveAnchorTransformer.php
Outdated
Show resolved
Hide resolved
packages/guides/src/Compiler/NodeTransformers/MoveAnchorTransformer.php
Outdated
Show resolved
Hide resolved
packages/guides/src/Compiler/NodeTransformers/MoveAnchorTransformer.php
Outdated
Show resolved
Hide resolved
packages/guides/tests/unit/Compiler/NodeTransformers/MoveAnchorTransformerTest.php
Outdated
Show resolved
Hide resolved
jaapio
force-pushed
the
feature/references
branch
from
941e15c to
4c61b31
Compare
Anchors should be part of the first compound node after their definition. Anchors at the end of a compound node are positioned wrong by the parser So they are moved to the next compoundnode.
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
jaapio
force-pushed
the
feature/references
branch
from
4c61b31 to
ee89ac4
Compare
Anchors should be part of the first compound node after their definition. Anchors at the end of a compound node are positioned wrong by the parser So they are moved to the next compoundnode.