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

Block filter passed to find_by method on Block should be able to skip subtree #2067

Closed
mojavelinux opened this issue Mar 11, 2017 · 6 comments
Assignees
Milestone

Comments

@mojavelinux
Copy link
Member

mojavelinux commented Mar 11, 2017

Currently, if the block filter passed to Block#find_by returns false, the corresponding node is not included in the result set. However, the method still continues to discover descendant nodes (i.e., descend the subtree). It should be possible to return a value, such as :skip, that tells the method to skip the subtree altogether.

Current (does not include dlist, but includes all children)

doc.find_by {|b| b.context == :dlist ? false : true }

Proposed (does not include dlist or its children)

doc.find_by {|b| b.context == :dlist ? :skip : true }
@mojavelinux mojavelinux added this to the v1.5.6 milestone Mar 11, 2017
@mojavelinux mojavelinux self-assigned this Mar 11, 2017
@mojavelinux mojavelinux modified the milestones: v1.5.7, v1.5.6 Mar 30, 2017
@mojavelinux mojavelinux modified the milestones: v1.5.7, v1.5.8 Apr 7, 2018
@mojavelinux
Copy link
Member Author

This is related to, but different from, #2900.

mojavelinux added a commit to mojavelinux/asciidoctor that referenced this issue Oct 8, 2018
@mojavelinux
Copy link
Member Author

An open question is whether it should be possible to include the node, but skip its children.

@mojavelinux
Copy link
Member Author

I think we need to support both :skip and :skip_subtree. :skip is the same as false, except it also stops descending. :skip_subtree is the same as true, except it stop descending.

@mojavelinux
Copy link
Member Author

To be consistent with NodeFilter, I think we should change the definition we are using for :skip to mean consider children. Hence, here's the list of possible return values:

  • true - accept node and continue descending
  • :skim - accept node, but stop descending (alternatives :skip_subtree, :skip_children)
  • false - reject node and any children
  • :skip - reject node, but still consider children
  • (from 2900) raise StopIteration - stop traversing and return result

@mojavelinux
Copy link
Member Author

The problem with that proposal is that it would break a lot of uses of find_by that rely on the existing true/false behavior. Therefore, we'll need to stick with :skip meaning to reject the node and any children.

  • true - accept node and continue descending
  • false - reject node, but still consider children
  • :skip - reject node and any children
  • :skip_children - accept node, but stop descending
  • (from 2900) raise StopIteration - stop traversing and return result

mojavelinux added a commit to mojavelinux/asciidoctor that referenced this issue Oct 8, 2018
…and its descendants or just descendants

- honor :skip return value from filter block to skip node and its descendants
- honor :skip_children return value from filter block to accept node, but skip its descendants
@mojavelinux
Copy link
Member Author

@Mogztter do you like this proposal? (see #2902 for the impl)

mojavelinux added a commit to mojavelinux/asciidoctor that referenced this issue Oct 9, 2018
…find_by to skip subtrees

- interpret :skip return value of filter block to mean skip node and its descendants
- interpret :skip_children return value of filter block to mean accept node, but skip its descendants
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant