quartiq / miniconf

Rust serialize/deserialize/access reflection for trees (no_std, no alloc)
MIT License
25 stars 3 forks source link

Add mutable iteration support #88

Closed ryan-summers closed 1 year ago

ryan-summers commented 2 years ago

When iterating across the settings array, the settings structure is immutably borrowed. This means that settings cannot be modified during iteration.

It would be useful to allow mutable iteration of settings.

There are some implications here. If we allow mutably iterating, we will need to specifically address iterating over Options. If the option were replaced during iteration, it could have unintended consequences on structure iteration.

jordens commented 2 years ago

Do we support Option? I though it's only simple enum. Otherwise this problem (mutable walk) would apply to all enums, right?

ryan-summers commented 2 years ago

We support Option as a special case. An Option represents a branch of the settings tree that may or may not exist. If the Option is Some() then there are settings available down that branch. If it is None, that branch is empty.

We use it for settings that may or may not exist depending on run-time detection. In Booster's case, this is whether or not an RF channel is installed.

ryan-summers commented 2 years ago

This doesn't impact enums, as we don't actually walk across the values of the setting, but rather just the settings paths. Modification of a single leaf of the settings tree doesn't impact iteration across the rest of the leaves.

That only could happen with Option modifications mentioned above, since that's equivalent to adding a new branch to the tree.

jordens commented 2 years ago

Ah right. Yeah. On the second point I meant that it would apply to all non-simple enums. But Option is the only one supported now.

jordens commented 1 year ago

I think the only problem here is just that: Option. One approach could be to produce all paths in next_path regardless of the runtime state. Then next_path could become an associated function. The user (and re-publication) would only have to take care with paths that may not exist at runtime i.e. just try get()/set() and ask for forgiveness.

ryan-summers commented 1 year ago

next_path takes a self parameter, so if the Option is None, we can't iteratively recurse through the wrapped type.

I think that would be a viable strategy if we removed self from next_path (I don't think it's required, it's very similar to metadata()). Sure there's some annoyance about asking for a value that doesn't actually exist during republication, but that's an easy match-ignore.

jordens commented 1 year ago

If next_path is an associated function, it doesn't take self.