SPICE-0008: Member deletion #9
Conversation
| [0] = super[0] - 14 | ||
| 496 | ||
| ["quux"] = foo | ||
| ["baz"] = delete // <1> |
There was a problem hiding this comment.
Have you consider an alternate syntax for delete?
This seems very similar to normal assigning syntax and, specially without syntax highlighting, may fly under the radar while reviewing code.
What about
prop {
delete ["entry"] // for entries
delete 0 // for indexes
delete prop // for properties
delete [[foo == 10]] // for predicates
}It's easier to see the change and totally unambiguous for the parser.
There was a problem hiding this comment.
I had considered this, but opted against it for a few reasons (that I'll include in Alternatives considered); I like that in your change, deletes are emphasised over other amends/overwrites. The downsides, however:
- currently and with my proposed syntax, you can skim the left-margin of an amends expression to find all things that are affected
- your changes requires member predicates and entries to occur in different places (namely in the argument position of
delete) than they do now delete prop, to me, reads like an expression that would be evaluated as usual, i.e. resolving and evaluatingpropto use its value rather than its name, which is particularly painful considering your index suggestion (without square brackets); surely that could be done parametrically ;)- as shown here, such alternative syntax makes room for all sorts of small variations (here, that element indices don't have square brackets) that allow subtle bugs (we've been talking about ways to disambiguate entry-indices from integer-valued entry keys), and require more syntax knowledge of the user.
There was a problem hiding this comment.
Besides that; I'm not entirely convinced the emphasis of deletes over other changes is actually warranted. Does it deserve more emphasis than e.g. foo = Undefined()?
There was a problem hiding this comment.
I actually like what @stackoverflow 's proposal better.
- currently and with my proposed syntax, you can skim the left-margin of an amends expression to find all things that are affected
Is this just for readability, or has any impact to the implementation? This feels like a nice to have feature. But = delete feels odd semantically. Most people would think the right side of = to be a value instead of an operator.
delete prop, to me, reads like an expression that would be evaluated as usual, i.e. resolving and evaluatingpropto use its value rather than its name, which is particularly painful considering your index suggestion (without square brackets); surely that could be done parametrically ;)
Would delete [0] address the concern? It feels reasonable to add brackets around indices.
There was a problem hiding this comment.
Overall, this approach makes sense to me.
Some general concerns about deletion:
- Shifting object elements seems quite dangerous. The element index is also the member's name. This means that deletion isn't just a delete, it's also a rename of existing members. This can produce quite confusing errors and make Pkl code harder to reason through.
- As another feature in the future, we want to improve provenance of object members. For example, we want to show in error messages where an object value was originally defined. If we shift object elements, it probably makes this task more complicated; we would need to convey "this object defined
[1] = 4, and then a deletion happened so it changed to[0] = 4" - I'm curious how the implementation should work here. This changes our object model quite a bit--right now, the evaluation of an object works by collecting all member names from the root-most object in the prototype chain, then evaluating all member values from the top-most object down (see
VmObject.forceAndIterateMemberValues()). If our members can change their names (as in the case of element deletion), our model of evaluation will need to change. Also: how does this affect performance?
As far as I can tell, none of the other config languages offer a first-class delete mechanism, so there is not much prior art to draw from.
Another idea: perhaps we should not allow deletions of elements, because both of the possible solutions (maintain a hole, shift members) are dangerous.
|
Hiding instead of deleting members would keep references intact while still removing members from output. This might offer a decent compromise between power, simplicity, and safety. |
|
The problem with hiding members is that the indices become non-contiguous. This means that, for example, |
I don’t see why. By “hiding” I mean the same behavior as |
|
The issue that I see with "hiding" these members is that you get different names when you then l1 = new Listing { 1; 2; 3 } { [0] = delete } // new Listing { (hidden) 1; 2; 3 }
l2 = l1.toList() // List(2, 3)
res1 = l1[0] // 1
res2 = l2[0] // 2And also that causes havoc with equals: l1 = new Listing { 1; 2; 3 } { [0] = delete }
l2 = new Listing { 2; 3 }
res = l1 == l2 // is this true or false?
res2 = l1[0] == l2[0] // if the above is true, shouldn't this be true?Going one further: l1 = new Listing { 1; 2; 3 } { [0] = delete }
l2 = l1.toList().toListing()
res = l1 == l2 // shouldn't this be true? |
|
@bioball If Some benefits of this solution:
|
Question; would this enumeration of TODOs have non-contiguous numbers, or would hidden things show up here? |
I’d imagine that iterating over hidden elements/entries would behave the same as iterating over hidden properties does today. |
No description provided.