search

tree-sitter / node-tree-sitter

Node.js bindings for tree-sitter
https://www.npmjs.com/package/tree-sitter
MIT License
649 stars 114 forks source link

feat: Add iterator methods to Query #207

Open segevfiner opened 4 months ago

segevfiner commented 4 months ago

Add iterator methods to Query

Still need to do capturesIter and CapturesIterator, typings, etc.

I'm not that happy about the code duplication though... We could probably implement matches and captures in terms of matchesIter and capturesIter to reduce it. But that might be slower due to all the additional calls that would entail between C++ and JS, and transferring the nodes one by one.

P.S. LookaheadIterator[@@iterator] can probably be implemented in C++, and ts_query_cursor can probably be allocated on Query instead of in AddonData if we want too.

Fixes #178

amaanq commented 2 months ago

I think it'd make more sense to just implement matches and captures with IterableIterator by default, what do you think?

segevfiner commented 2 months ago

I'm worried about backwards compat abd performance. Which is why I kept the existing ones for now.

amaanq commented 2 months ago

If performance is a concern, why even add this then? What's the benefit/purpose exactly? (really curious because I'm all for iterators)

segevfiner commented 2 months ago

Memory consumption. As using iterators means we don't need to hold all results in memory at once. We still need to measure if it's really significantly slower or not. I'm not sure if it's really slower. But if it is, it is always possible to chunk the iteration internally so we don't transfer one by one.