| Re: [henshin-user] Creating "same" edge as "forbid" and "create" |
hi,
I am not super sure whether fixing one edge case with special code for another edge case is a good idea.Good point, I see the general problem about that. However, I noticed that in this case the patch addresses a more fundamental issue: our meta-model does not support parallel <<preserve>> and <<forbid>> edges in the first place, as it does not provide a means to specify that the NAC-edge has a seperate identity from the LHS-edge (in particular, we don't have edge mappings). Edges with identical source and target nodes are assumed to be identical, so the NAC-edge is interpreted as a copy of the LHS-edge.
Probably, i do not know this specific situation well enough. But i thought that a <<forbid>> edge is always in a negated condition graph (at least that is what is shown in the tree editor). As a preserve edge is in the LHS/RHS, they should be two different edge objects.
Probably, this stems from the decision that the meta model uses LHS / RHS but the graphical editor shows a concise notation with markups. That is the reasons that we did not use in the textual Henshin syntax the Henshin meta model but instead provide transformations to it when running the transformation.
Therefore, even without the clean-up, the contraintutive behavior that Joel observed is an accurate representation of what's going on: creating a <<preserve>> effectively removes the <<forbid>> edge. The fix helps avoid this behavior by not creating a <<preserve>> edge, but a <<create>> edge.
Particularly, i am not sure whether the code works always as intended, e.g., if you have complex nested conditions.It is specified and implemented for <<forbid>> edges. This includes edges in multiple NACs/PACs connected by boolean operators, and excludes edges in nested application condition graphs of nesting depth >=2 (which are not supported by the graphical editor anyways). As the code only changes the default behavior, I don't think it can break something that would otherwise work.
ok, i +1 the change in gerrit (though there is a merge conflict). It would be awesome if you could add junit-test cases for this method just to be on the safe side :-)
I think a better solution would be to remove the special clean up functionality and instead work with validations and provide the special clean up functionalities as quick fixes. Then the user can decide on the correct solution. What do you think?I like the idea of providing better validation and quick fix support a lot - currently, we don't have any quick fixes at all. This is a long-term goal that could be addressed in the context of a student project - it's on my to-do list now.
great! /mtt
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature