Add optional id property to the tree item

[sandy081]

Tree view associates the tree node state (expansion & selection) to the handle provided by the Tree item providers. Some of the tree view providers uses TreeItem as handle and they do not store it. They always create new TreeItems when asked for the children handles. State is not persisted for such trees as the handle changes always.

As we already maintain the map of handles, allowing to provide an optional id in TreeItem will be useful to persist the state even if new handle is created for the TreeItem with same id.

[jrieken]

They always create new TreeItems when asked for the children handles

The new TreeItems, will they be different as such that they have a different label, icon etc? Could you not just compare them on the properties we have defined (and we will use to render) and use that as 'synthetic' id?

[eamodio]

In GitLens for example, the vast majority of my TreeItem's are regenerated on refresh and will have the same properties (label, icon, etc), but there are some that are regenerated that will have the label changed (e.g. the label has a count in it). GitLens also can have duplicate (identical properties) TreeItems in the lists (as commits can be in multiple branches, etc) so parenting should be taken into account.

I liked the behavior of the synthetic id, that was based on something like parent and label for the vast majority of my TreeItems, but then to have the option be able to set an id property merely as a replacement for the label (in the synthetic computation) , for the items where the label is know to be unstable.

[sandy081]

Thanks @eamodio for chiming in.

@jrieken I am already generating an artificial id from the parent's id and item's label. I am using the incremental counter for duplicated labels. As @eamodio mentioned, if there are changing labels for the same item, there is a need for identity that is defined by the extensions.

This was discussed in the issue #40018 and we ended upon following strategy:

If TreeItem provides id, then this will be used as id which has to be unique for the complete tree. Otherwise, a unique id will be generated from its parent id and its label.

[jrieken]

this will be used as id which has to be unique for the complete tree.

And that's massive pita if you ask me. Our internal tree widget has that constraint and error telemetry is usually full of duplicate id errors... Anyways, I see the need for this. 🚢 it

[jrieken]

This needs a lot more documentation which explains what the id is and what it is used for, why it's optional etc. I'd say much of this: #40018 (comment)

[sandy081]

Added more doc to the id attribute explaining why it is used and the default behaviour.

[sandy081]

Added more doc to the id attribute explaining why it is used and the default behaviour.

[jrieken]

Not sure what that mean? With a re-generated id state will reset?

[sandy081]

Yes, if the label has changed, new id (different from old) is generated and as a result previous state is lost.

[jrieken]

Ok, how about merging that with sentence about how an id will be generated.

Optional id for the tree item that has to be unique across tree. The id is used to preserve the selection and expansion state of the tree item.

If not provided, an id is generated using the tree item's label. **Note** that when labels change, ids will change and that selection and expansion state cannot be kept stable anymore. 

[sandy081]

Looks better. Updated.