Expand remove API

[mxsdev]

What problem does this solve or what need does it fill?

This issue continues discussion from #511. In that PR, remove behavior sets the children's parents to None as a conservative fix to #510. This is not the only way to manage a deleted node's children, however.

What solution would you like?

I would suggest an API such as the following:

• remove_reparenting - Remove node and reparent children to grandparent
• remove_orphaning- Remove node and orphan children
• remove_cascading - Remove node and all its children recursively

And then we can select one of these to stand for the default remove. One suggestion here was:

Having said that, I wonder if a better fix would be to recursively remove and drop all children?

I would advise against this as default behavior from the perspective of avoiding implicitly destructive defaults. In practice, the library consumer will end up with potentially a ton of stale NodeId references, which may be better as opt-in rather than opt-out.

That being said, the recursive remove is probably the most common use case. And orphaning by default can lead to potential memory leaks as orphaned trees continue to exist unnecessarily in memory.

Once a specific API is agreed upon, I am happy to draft a PR. (I'll write test cases/docs this time :p)

What alternative(s) have you considered?

We merged a simple fix in #511

If it would be preferable we can use more verbose names like remove_and_orphan_children. Or less verbose like remove_orphan, remove_cascade, remove_reparent. No preference there personally.

Additional context

@nicoburns pointed out:

I think the usecase of reparenting children to the grandparent might be better served by display: contents support.

So maybe we don't want a remove_reparenting endpoint just yet?

[alice-i-cecile]

remove_and_reparent would be the grammatical form I'd use here.

We could also pass in an enum like RemovalBehavior and then branch, but I think that just having multiple methods with an alias for remove is going to be cleaner here.

I think we should have remove_and_orphan and remove_and_cascade for now, and then add other options as needed.

[nicoburns]

I think we might also want a reparent API (could also be called set_parent or move_node). Part of me also wonders whether the tree implementation to be split out into a generic slotmap_tree crate or similar, although we'd need to work out how to handle the measure functions that are stored in the secondary slotmap.