Opened 4 years ago

Last modified 3 years ago

#6123 new defect

Preserve order of modifiers in OMEdit

Reported by: Francesco Casella Owned by: Adeel Asghar
Priority: blocker Milestone: 1.19.0
Component: OMEdit Version:
Keywords: Cc: Adrian Pop, Per Östlund, Andrea Bartolini

Description

When parameter values are changed in OMEdit, for some reason the way the changes are applied to the code is that all the modifiers are removed first, and then they are re-applied with modifications.

Sometimes this causes the order of the modifiers to change in the declaration of the component instance, particularly if the original model was created by another Modelica tool. This has no consequence per se on the semantics of the model, but it can create lots of bogus diff's in the source code if revision control systems are used, and even create completely bogus merge conflicts, which are particularly annoying to fix.

The attached test package demonstrates the issue. Model M has five parameters declared in a certain order. When I create S by dragging an instance of M in it, if I then open the parameter window, the parameters are shown in the same order they are declared. However, if I change them, the modifiers that are saved in the .mo file are lexicographically ordered, as the code of S demonstrates.

I then duplicated S into S1, where I kept the same order of the modifiers, but I put each of them on a separate line, which is something people often do to make the source code easier to read, or to allow more fine-grained tracking of modifications on a RCS.

I then duplicated S1 into S2, doubled-clicked on m, changed the value of b, and saved the file. In this case, the order of the modifiers was preserved, so that only one line in the source code (the one containing the modifier for b) was actually changed.

Subsequently, I duplicated S1 into S3, where I changed the order of the modifiers with respect to the lexicographical one. This is perfectly legitimate, and may represent how code generated by other Modelica tools, or possibly written manually using a text editor, could look like.

Finally, I duplicated the rearranged S3 into S4, double-clicked on m, changed the value of b and saved.

The result, as you can see, is that the modifiers were reordered lexicographically, thus causing a lot more diff's in the source code than required. Additionally, something went wrong with the diff module, so that some whitespace was added and some was removed.

This is a critical issue, because if you open a model created by anther tool (e.g. Dymola) and just change one parameter value, the whole set of modifiers is garbled up. As a consequence, for example, it is not possible to use OMEdit to maintain Modelica Standard Library models that were developed with other tools without messing them up badly. In fact, I always end up using a text editor when working with the MSL, which is safe but not particularly user friendly, because I can't check the code for correctness and I always have to unload and reload the whole MSL all the time, which is really ugly.

Thus, I set this as a blocker for 1.17.0. @adeas31, I guess this can be fixed easily by not enforcing lexicographical ordering and by preserving the order of declaration of existing modifiers in the code.

I understand this will also need to be applied to the handling of modifiers in redeclared classes and components.

@adrpo mentioned an existing ticket on this topic. I couldn't find it, maybe it was on some forum post. If you find it out, please handle it as required.

Attachments (1)

TestModifierOrder.mo (1.3 KB ) - added by Francesco Casella 4 years ago.

Download all attachments as: .zip

Change History (4)

by Francesco Casella, 4 years ago

Attachment: TestModifierOrder.mo added

comment:1 by Francesco Casella, 4 years ago

Milestone: 1.17.01.18.0

Rescheduled to 1.18.0

comment:2 by Francesco Casella, 3 years ago

Milestone: 1.18.0

Ticket retargeted after milestone closed

comment:3 by Francesco Casella, 3 years ago

Milestone: 1.19.0

1.18.0 blocker tickets moved to 1.19.0

Note: See TracTickets for help on using tickets.