luci-base: form.Value.rmempty documentation implies it's like retain, but actually similar to optional

[wryun]

Is there an existing issue for this?

• [x] I have searched among all existing issues (including closed issues)

screenshots or captures

No response

Actual behaviour

The documentation for rmempty and retain seem to imply that they serve the same purpose: keeping the uci.value when the form.Value is hidden due to dependencies.

https://github.com/openwrt/luci/blob/master/modules/luci-base/htdocs/luci-static/resources/form.js#L1366 https://github.com/openwrt/luci/blob/master/modules/luci-base/htdocs/luci-static/resources/form.js#L1386

However, though 'retain' follows the documentation (removing the item if inactive and retain = false), rmempty seems only to be used when active, and if set acts very similarly to 'optional' (many uses of this.rmempty || this.optional).

Related commit: https://github.com/openwrt/luci/commit/f5fbecf132b6c18141896b093935fb4c035ab33d

Expected behaviour

Minimally, the documentation should clarify the actual role of rmempty. But perhaps it is also possible to collapse rmempty/optional or rmempty/retain to reduce the number of options.

[systemcrash]

In my experience, rmempty=true removes that element when empty from the temporary edited config so that you don't get stuck in a dependency chain that depends on it having a value, whereas retain does the opposite. It keeps any value there, whether in the permanent config or edited config.

[wryun]

If I've read the code correctly (in parse()), rmempty is not considered at all in the case where there are unsatisified dependencies (active), and retain is only used in that case (!active), so it seems like the documentation is misleading.

My understanding is that:

• if it's active (i.e. visible), rmempty = true allows one to (a) set it to empty and (b) have this remove the option. However, if rmempty = false and optional = false and the option is empty, this will cause a TypeError (i.e. the option can't be empty)
• if it's inactive (i.e. hidden due to dependencies), retain = true prevents it being removed

I haven't raised a PR because I think the best resolution is probably some consolidation of this options, and it would be interesting if @jow- had any thoughts on this.