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

New Feature: NodeIterator #683

Closed
colinodell opened this issue Jun 27, 2021 · 0 comments · Fixed by #684
Closed

New Feature: NodeIterator #683

colinodell opened this issue Jun 27, 2021 · 0 comments · Fixed by #684
Assignees
Labels
enhancement New functionality or behavior implemented Change has been implemented performance Something could be made faster or more efficient
Milestone

Comments

@colinodell
Copy link
Member

colinodell commented Jun 27, 2021

Description

The NodeWalker we use today works well in some cases but is quite inefficient in other cases for the following reasons:

  1. Blocks and container nodes are visited twice (upon entering and leaving), but in most cases we only care about visiting them once
  2. It requires wrapping the node in a NodeWalkerEvent object
  3. It's not easy to skip certain parts of the AST

I'd therefore like to add a NodeIterator which offers the same simplified approach to looping through AST nodes but with the following features:

  1. Each node should be visited exactly once
  2. Each node object should be returned directly - no intermediate wrapping classes
  3. It should support skipping branches of the tree which only contain inlines (in cases where we only care about blocks)
  4. It should make AST iteration easier for developers

In the best-case scenario, this should result in AST iteration times being cut in half since each node is only visited once instead of twice.

Example

Here's what the usage would look like in practice - notice how much of the boilerplate goes away!

-$walker = $document->walker();
-while ($event = $walker->next()) {
-    if (! $event->isEntering()) {
-        continue;
-    }
-
-    $node = $event->getNode();
+foreach ($document->iterator() as $node) {
     if ($node instanceof Link) {
          // TODO: do something
     }
 }
@colinodell colinodell added enhancement New functionality or behavior do not close Issue which won't close due to inactivity labels Jun 27, 2021
@colinodell colinodell self-assigned this Jun 27, 2021
@colinodell colinodell added this to the v2.0 milestone Jun 27, 2021
@colinodell colinodell added the performance Something could be made faster or more efficient label Jun 27, 2021
@close-label close-label bot added the implemented Change has been implemented label Jun 27, 2021
@colinodell colinodell removed the do not close Issue which won't close due to inactivity label Jun 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New functionality or behavior implemented Change has been implemented performance Something could be made faster or more efficient
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant