Opened 6 years ago

Closed 4 years ago

Last modified 4 years ago

#4781 closed defect (fixed)

OMEdit places the "end Model;" statement at the end of annotation line

Reported by: Andrea.Bartolini Owned by: sjoelund.se
Priority: blocker Milestone: 1.16.0
Component: OMEdit Version: v1.13.0-dev-nightly
Keywords: Cc: adeas31, adrpo

Description

Generate a new modelica class Model1 from OMEdit, the generated .mo file looks like this:

model Model1
equation

annotation(
    Icon(coordinateSystem(grid = {0.1, 0.1})),
    Diagram(coordinateSystem(extent = {{-200, -100}, {200, 100}})));end Model1;

in which the "end Model1; statement is placed at the end of the annotation line instead of in a new line.

OMEdit 1.13.0~dev-102-g86eb441
Connected to OpenModelica 1.13.0~dev-708-g6803667
sysop Ubuntu 16.04 - 64 Bit

Attachments (7)

log.txt (7.7 KB) - added by casella 5 years ago.
log2.txt (3.8 KB) - added by casella 5 years ago.
log3.txt (23.3 KB) - added by casella 5 years ago.
M.mo (482 bytes) - added by casella 5 years ago.
MM.mo (29 bytes) - added by adeas31 5 years ago.
MM.mos (520 bytes) - added by adeas31 5 years ago.
diff.png (51.1 KB) - added by adrpo 5 years ago.

Download all attachments as: .zip

Change History (23)

comment:1 follow-up: Changed 6 years ago by casella

  • Owner changed from adeas31 to sjoelund.se
  • Status changed from new to assigned

@sjoelund.se, can you please have a look at this? I guess it's a quick fix.

Thanks!

comment:2 in reply to: ↑ 1 Changed 6 years ago by sjoelund.se

Replying to casella:

@sjoelund.se, can you please have a look at this? I guess it's a quick fix.

Thanks!

It's not

comment:3 Changed 6 years ago by casella

Do we have a plan B to avoid releasing 1.13.0 with this annoying behaviour?

comment:4 Changed 5 years ago by casella

  • Milestone changed from 1.13.0 to 1.14.0

Rescheduled to 1.14.0 after 1.13.0 releasee

comment:5 Changed 5 years ago by casella

  • Cc adeas31 adrpo added
  • Priority changed from high to blocker

As the 1.14.0 release is approaching, we should really fix this issue, which is quite annoying. In fact, the problem is twofold:

  1. Create a new model M in OMEdit. The message browser shows:
    [1] 17:49:02 Scripting Warning
    Could not preserve the formatting of the original model when duplicating it. The duplicate model was created with internal pretty-printing algorithm.
    
    This is totally confusing and not really acceptable, as a) there is no such thing as an "original" model (this is a new model, isn't it?) and b) nobody asked to duplicate anything
  1. Drag one component into it (e.g. Modelica.Thermal.HeatTransfer.Components.HeatCapacitor). The message browser goes again with:
    [2] 17:52:07 Scripting Warning
    Could not preserve the formatting of the original model when duplicating it. The duplicate model was created with internal pretty-printing algorithm.
    
    [3] 17:52:07 Scripting Warning
    Could not preserve the formatting of the original model when duplicating it. The duplicate model was created with internal pretty-printing algorithm.
    
    If I now switch to the Text View, I see:
    model M
      Modelica.Thermal.HeatTransfer.Components.HeatCapacitor heatCapacitor1 annotation(
        Placement(visible = true, transformation(origin = {44, -16}, extent = {{-10, -10}, {10, 10}}, rotation = 0)));
    equation
    
    annotation(
        uses(Modelica(version = "3.2.3")));end M;
    

The overall impression we convey in terms of software quality is really very, very low here.

@sjoelund.se, in comment:2 you wrote it's not a quick fix. Can you please elaborate a bit why it isn't? I think we cannot really ship OMEdit with this kind of behaviour.

Thanks!

comment:6 Changed 5 years ago by casella

I checked the omedicommunication log when creating the new model M. The relevant excerpt of the log is attached. It starts with

loadString("model M equation end M;","M","UTF-8",false) 18:07:04:106

without any newlines. Why is that the case? Are they added automatically by some pretty-printing algorithm?

Then, OMEdit asks for various features of the models, and that works fine, as there are none (the model is so far empty).

Then, the model is listed twice (why?)

listFile(M,true) 18:07:04:219
"model M
equation

end M;" 18:07:04:219
#s#; 0.000000; 3.126000; 'listFile(M,true)'

errors:=getMessagesStringInternal() 18:07:04:219
{} 18:07:04:222
#s#; 0.003000; 3.129000; 'errors:=getMessagesStringInternal()'

size(errors,1) 18:07:04:222
0 18:07:04:223
#s#; 0.001000; 3.130000; 'size(errors,1)'

listFile(M,true) 18:07:04:367
"model M
equation

end M;" 18:07:04:368
#s#; 0.000000; 3.130000; 'listFile(M,true)'

errors:=getMessagesStringInternal() 18:07:04:368
{} 18:07:04:372
#s#; 0.004000; 3.134000; 'errors:=getMessagesStringInternal()'

size(errors,1) 18:07:04:372
0 18:07:04:373
#s#; 0.001000; 3.135000; 'size(errors,1)'

obviously producing the same result, and then the two results are compared (why?)

diffModelicaFileListings("model M\nequation\n\nend M;", "model M\nequation\n\nend M;", OpenModelica.Scripting.DiffFormat.plain) 18:07:04:373
"model M
equation

end M;" 18:07:04:376
#s#; 0.003000; 3.138000; 'diffModelicaFileListings("model M\nequation\n\nend M;", "model M\nequation\n\nend M;", OpenModelica.Scripting.DiffFormat.plain)'

and then an array of 7 (7!) error messages is reported. I guess that corresponds to the warning message reported in the OMEdit log.

@sjoelund.se, @adeas31, can you make sense out of this thing and explain it to me? I have some difficulty doing so...

Thanks!

Changed 5 years ago by casella

comment:7 Changed 5 years ago by casella

OK, I managed to sort out issue 1. in comment:5. The root cause was that I had set -d=execstat in the compiler flags. Apparently, the corresponding notifications are somehow confusing OMEdit. I guess this should be fixed, though maybe it is not that high-priority, as most users will never set that flag.

Once I removed the flag, I didn't get any notifications. The corresponding log is attached as log2.txt. I still don't get why OMEdit runs the diff algorithm on a new model, but anyway the result is ok.

comment:8 Changed 5 years ago by casella

I then proceeded to check the log corresponding to issue 2. in comment:5, see the attached log3.txt.

At some point (line 321) OMEdit adds the new component, i.e. the heat capacitor

addComponent(heatCapacitor1, Modelica.Thermal.HeatTransfer.Components.HeatCapacitor,M,annotate=Placement(visible=true, transformation=transformation(origin={26,10}, extent={{-10,-10},{10,10}}, rotation=0))) 18:37:11:214
true 18:37:11:215

and the uses annotation, since M is now using the MSL

addClassAnnotation(M, annotate=$annotation(uses(Modelica(version="3.2.3")))) 18:37:11:441

then it lists the model, which looks just fine

listFile(M,true) 18:37:11:442
"model M
  Modelica.Thermal.HeatTransfer.Components.HeatCapacitor heatCapacitor1 annotation(
    Placement(visible = true, transformation(origin = {26, 10}, extent = {{-10, -10}, {10, 10}}, rotation = 0)));
equation

  annotation(
    uses(Modelica(version = "3.2.3")));
end M;" 18:37:11:442

The next thing I see at line 360 is a call to diffModelicaFileListings, where I see the string of the original model, and the string of the updated model, with all the newline escapes \n in the proper place.

diffModelicaFileListings("model M\nequation\n\nend M;", "model M\n  Modelica.Thermal.HeatTransfer.Components.HeatCapacitor heatCapacitor1 annotation(\n    Placement(visible = true, transformation(origin = {26, 10}, extent = {{-10, -10}, {10, 10}}, rotation = 0)));\nequation\n\n  annotation(\n    uses(Modelica(version = \"3.2.3\")));\nend M;", OpenModelica.Scripting.DiffFormat.plain) 18:37:11:509

Unfortunately, the output of that function call has the end M; statement in the wrong place.

"model M
  Modelica.Thermal.HeatTransfer.Components.HeatCapacitor heatCapacitor1 annotation(
    Placement(visible = true, transformation(origin = {26, 10}, extent = {{-10, -10}, {10, 10}}, rotation = 0)));
equation

annotation(
    uses(Modelica(version = \"3.2.3\")));end M;" 18:37:11:512

I'm not sure why. Then, the model is again listed, and still has the correct formatting (lines 378-386):

listFile(M,true) 18:37:11:526
"model M
  Modelica.Thermal.HeatTransfer.Components.HeatCapacitor heatCapacitor1 annotation(
    Placement(visible = true, transformation(origin = {26, 10}, extent = {{-10, -10}, {10, 10}}, rotation = 0)));
equation

  annotation(
    uses(Modelica(version = "3.2.3")));
end M;" 18:37:11:526

Then there is another call to diffModelicaFileListings (why?) at line 397, again with the wrong formatting. And that was it.

At that point, the Text View already showed the wrong formatting. However, when I later moved the capacitor model, after the call to updateComponents, the log reported

listFile(M,true) 18:39:12:799
"model M
  Modelica.Thermal.HeatTransfer.Components.HeatCapacitor heatCapacitor1 annotation(
    Placement(visible = true, transformation(origin = {4, -26}, extent = {{-10, -10}, {10, 10}}, rotation = 0)));
equation

  annotation(
    uses(Modelica(version = "3.2.3")));
end M;" 18:39:12:799

followed by more diffModelicaListings with the wrong formatting.

In any case, when I saved M.mo, I got the wrong formatting, see attachment.

@sjoelund.se, @adeas32, I'm not sure I understand the logic of all this, but I hope you can fix it for good.

Thanks!

Changed 5 years ago by casella

Changed 5 years ago by casella

Changed 5 years ago by casella

comment:9 Changed 5 years ago by adeas31

I made a really small script that shows the problem.

Changed 5 years ago by adeas31

Changed 5 years ago by adeas31

comment:10 Changed 5 years ago by casella

I see that the output of diffModelicaListings in the MM.mos script is

"model MM[4;32m
  Modelica.Thermal.HeatTransfer.Components.HeatCapacitor heatCapacitor1 annotation(
    Placement(visible = true, transformation(origin = {-10, 36}, extent = {{-10, -10}, {10, 10}}, rotation = 0)));[0m
equation

[4;32mannotation(
    uses(Modelica(version = \"3.2.3\")));[0mend MM;"

which is obviously wrong, and also contains some weird characters (not UTF, not ASCII).

I'm not sure if this is the root cause of the problem. What I noticed in the OMEdit log3.txt, line 438, is that diffModelicaListings is called with two literal strings, which differ precisely because of the missing \n. Where does OMEdit get the first of those two strings? Is this the output of a previous broken diffModelicaListings?

comment:11 Changed 5 years ago by adrpo

The output is correct as it has DiffFormat.color as default third argument.
In my msys terminal I get the attached image.

Changed 5 years ago by adrpo

comment:12 Changed 5 years ago by adeas31

Yeah as I said earlier it could be the output of the previous diffModelicaFileListings.

For new models the base string is from listFile and for existing models OMEdit reads the contents from the file.

comment:13 Changed 5 years ago by adrpo

I have debugged this a bit, seems the \n after the annotation is somehow gone after the diff of the parse tree. I will look more tomorrow.

comment:14 Changed 4 years ago by casella

  • Milestone changed from 1.14.0 to 1.15.0

Releasing 1.14.0 which is stable and has many improvements w.r.t. 1.13.2.

This issue, previously marked as blocker for 1.14.0, is rescheduled to 1.15.0

comment:15 Changed 4 years ago by adeas31

  • Resolution set to fixed
  • Status changed from assigned to closed

comment:16 Changed 4 years ago by casella

  • Milestone changed from 1.15.0 to 1.16.0

Release 1.15.0 was scrapped, because replaceable support eventually turned out to be more easily implemented in 1.16.0. Hence, all 1.15.0 tickets are rescheduled to 1.16.0

Note: See TracTickets for help on using tickets.