#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)
Change History (23)
comment:1 follow-up: ↓ 2 Changed 6 years ago by casella
- Owner changed from adeas31 to sjoelund.se
- Status changed from new to assigned
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:
- 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
- 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
Fixed in ed07d20/OpenModelica.
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
@sjoelund.se, can you please have a look at this? I guess it's a quick fix.
Thanks!