verso icon indicating copy to clipboard operation
verso copied to clipboard

Add nested fragments

Open elidhu opened this issue 10 months ago • 4 comments

I have added nested fragment parsing. I have not had to touch any tests except:

  • Removing the "double open" test as it is no longer an error.
  • Adding a "halt while open" test that I noticed was missing.
  • Adding a "nested fragment" test.

The original behaviour should be identical as far as the tests go.

The new behaviour can be seen in the example I added. The most notable part is the handling of nested fragment tags when outputting a fragment that has children. Nested tags are excluded as if they weren't there.

Let me know your thoughts.

Thanks!

elidhu avatar Apr 12 '24 13:04 elidhu

Addressed all of the above, and updated the README.

Thanks for taking the time to review!

elidhu avatar Apr 14 '24 23:04 elidhu

I did have the thought while I was making the changes that this adds support for "nested" fragments, not necessarily "overlapping". This would required some sort of ID on the closing tag (I guess).

I'm not sure if that was an important distinction

elidhu avatar Apr 14 '24 23:04 elidhu

Yes, good point — I don't think it's a hugely important distinction, and as you said, once the fragment structure is no longer tree-like it becomes necessary to annotate the closing tags. This seems like a good next step, adding a nice feature without complecting the system too much.

nickpascucci avatar Apr 15 '24 07:04 nickpascucci

RIght, had to fix all my signatures. It's good-to-go now!

elidhu avatar Apr 15 '24 13:04 elidhu

Would you be able to get this merged and cut a release? 🙏

elidhu avatar May 15 '24 12:05 elidhu

@elidhu sorry for the wait — I've just published v0.3.0 which includes your change 🎉

nickpascucci avatar Jul 07 '24 08:07 nickpascucci