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

Whole tree iteration #1

Open
mortenpi opened this issue May 17, 2022 · 3 comments
Open

Whole tree iteration #1

mortenpi opened this issue May 17, 2022 · 3 comments

Comments

@mortenpi
Copy link
Member

mortenpi commented May 17, 2022

CommonMark iterates over the whole tree if you do for node in tree, which is currently not implemented here (we only have children).

I would argue, however, that it is not intuitively clear which type of iteration (over direct children? over whole tree? over direct and indirect children, but not parents?) should be the default. Hence I would advocate that for each iterator there should be a function (like children) that returns the iterator. For the whole-tree iteration it could be called tree(node).

@MichaelHatherly
Copy link
Member

Perhaps make use of AbstractTrees.jl for this?

Any particular use cases you have in mind where different iteration schemes are preferable?

@mortenpi
Copy link
Member Author

AbstractTrees crossed my radar as well, but it wasn't clear from the docs how useful it would be. However, it seems to be undergoing an overhaul (JuliaCollections/AbstractTrees.jl#96, JuliaCollections/AbstractTrees.jl#99, dev docs), so it might make sense to wait until that is done for final decision.

I guess one question is whether we want to add a dependency for this. If the goal is to upstream the Markdown handing to core Julia eventually, then I think we'd have to drop the dependency again. But I guess maybe we can still keep this package around to provide bells and whistles on top of a minimal implementation in a standard library.

As for different iteration schemes, I don't have very specific use cases in mind, but my main gripe is that it is not obvious that for node in node should iterate over the whole tree.

  • Iterating over the tree vs children is one example for sure where it would be ambiguous.
  • There is also the question of whether you want to visit branch nodes twice (i.e. entering and exiting). I reckon there are use cases for different cases, and so it might make sense to e.g. have a keyword argument to control this. For what it's worth, AbstractTrees also distinguishes between the entering-only (PreOrderDFS) and exiting-only (PostOrderDFS)) modes.

In all cases it simply seems to me that being explicit would be better.

@ExpandingMan
Copy link

I recommend waiting for v0.4 of AbstractTrees to be tagged, which should be coming soon. There will be much more extensive documentation.

mortenpi added a commit that referenced this issue Aug 9, 2022
Mostly takes care of #1, except that the current CommonMark behaviour is
to call the callback both on the way up and down a branch, which
AbstractTrees does not seem to support currently.
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

No branches or pull requests

3 participants