command: unset metadata key if string is empty

[myQwil]

Followup to #17603

There is currently no way to completely remove a key from a metadata dictionary. This change makes it so that if a key's value is set to an empty string, the key itself will also be removed.

[CounterPillow]

Is there a reason why we instantiate a bstr from ka->arg here to check for v.len == 0 rather than checking !ka->arg? I.e., is it possible that ka->arg points to a string that's just '\0' and this should also be handled?

[myQwil]

That's essentially what we're already checking when we ask if v.len == 0, since bstr0 calls strlen to determine the string's length. Either way works, i just thought this way was a little bit more readable.

[CounterPillow]

Yeah, I'm wondering if the check is redundant (i.e. we're never passed a pointer to a \0) to turn this into this

if (!ka->arg)
    mp_tags_remove_bstr(tags, k);
else
    mp_tags_set_bstr(tags, k, bstr0(*(char **)ka->arg));

but if there's any chance that we do get passed a pointer to \0 as an empty string, then your way is both correct and necessary

[kasper93]

There is no redundant check bstr0 is inlined there and the flow will be exactly the same. It's more safe this way if we ever pass "" there, and if not there is zero overhead. Also we need bstr anyway.

[CounterPillow]

Right, due to the inlining the compiler will fold those two checks into one, so this is strictly better. Sorry for the noise.

[myQwil]

Also, it's not possible to pass, from the lua end, a value of nil. You'll get an error saying that that's not a valid type for this property. It has to be a string. So !ka->arg will never be true. You have to double-dereference it, first to get the char pointer, and then again to find out if its first character is '\0'.