Opened 8 years ago
Closed 8 years ago
#4065 closed defect (fixed)
Linebreaks, comments in component argument list are mangled after moving it
Reported by: | Owned by: | Martin Sjölund | |
---|---|---|---|
Priority: | high | Milestone: | 1.9.x |
Component: | Interactive Environment | Version: | |
Keywords: | Cc: |
Description
I often align the arguments in a component instantiation with linebreaks (a must if you have many arguments, to avoid running endless lines). Sometimes I add in-line comments to some arguments, or even comment out arguments when testing/changing models.
This can look like this (simple minimum example, see component boundary
, but imagine 5+ arguments):
model test_linebreaks Modelica.Fluid.Sources.FixedBoundary boundary(use_p = false, //some comment use_T = false) annotation(Placement(visible = true, transformation(origin = {-54, 42}, extent = {{-10, -10}, {10, 10}}, rotation = 0))); Modelica.Fluid.Sources.Boundary_pT boundary1 annotation(Placement(visible = true, transformation(origin = {-30, -30}, extent = {{-10, -10}, {10, 10}}, rotation = 0))); equation connect(boundary.ports[1], boundary1.ports[1]) annotation(Line(points = {{-44, 42}, {-44, 9}, {-20, 9}, {-20, -30}}, color = {0, 127, 255})); annotation(uses(Modelica(version = "3.2.2")), Icon(graphics = {Polygon(origin = {-13.1502, 17.2813}, points = {{-20.8498, 26.7187}, {75.1502, 6.71867}, {15.1502, 54.7187}, {-74.8498, 22.7187}, {-42.8498, -55.2813}, {-20.8498, 26.7187}})})); end test_linebreaks;
In current OMEdit, as soon as I move the component in the diagram, (or, it seems, change the position of a connection line attached to it), OMEdit somehow reorders the code, and removes all the linebreaks, and pulls comments out of the argument list, like this:
model test_linebreaks Modelica.Fluid.Sources.FixedBoundary boundary(use_p = false, use_T = false) annotation(Placement(visible = true, transformation(origin = {-54, 42}, extent = {{-10, -10}, {10, 10}}, rotation = 0))); //some comment Modelica.Fluid.Sources.Boundary_pT boundary1 annotation(Placement(visible = true, transformation(origin = {-30, -30}, extent = {{-10, -10}, {10, 10}}, rotation = 0))); equation connect(boundary.ports[1], boundary1.ports[1]) annotation(Line(points = {{-44, 42}, {-44, 19}, {-20, 19}, {-20, -30}}, color = {0, 127, 255})); annotation(uses(Modelica(version = "3.2.2")), Icon(graphics = {Polygon(origin = {-13.1502, 17.2813}, points = {{-20.8498, 26.7187}, {75.1502, 6.71867}, {15.1502, 54.7187}, {-74.8498, 22.7187}, {-42.8498, -55.2813}, {-20.8498, 26.7187}})})); end test_linebreaks;
This is not helpful, and on the contrary very frustrating. One has to often either undo the changes with Git (as OMEdit Ctrl-Z/Undo does not even pick up the change), which crashes OMedit if you don't unload the package first, or redo the changes (not fun for 5+ arguments).
Can this behaviour be fixed or circumvented?
Change History (17)
comment:1 by , 8 years ago
comment:3 by , 8 years ago
Component: | *unknown* → Interactive Environment |
---|---|
Milestone: | Future → 1.11.0 |
Release notes of version 1.9.4 list the improvements:
- Undo/Redo support.
- Preserving text formatting, including indentation and whitespace. This is especially important for diff/merge with several collaborating developers possibly using several different Modelica tools.
The issue should be fixed until 1.11.0.
comment:4 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 8 years ago
Smaller model to test the diff API:
diffModelicaFileListings( "model M\n FixedBoundary boundary(use_p = false, //some comment\n use_T = false) annotation();equation\n connect(a, bb);\nend M;", "model M\n FixedBoundary boundary(use_p = false, use_T = false) annotation();\n //some comment\nequation\n connect(a,b);\nend M;", OpenModelica.Scripting.DiffFormat.plain);
Fixing it still would not fix the complaints regarding OMEdit though...
comment:6 by , 8 years ago
Fixed in db9157b/OMCompiler for the master branch. I'll see if I can port it to 1.9 as well.
comment:7 by , 8 years ago
Thank you! Does it also fix the removal of the linebreaks between arguments (even when no comment is there)?
comment:9 by , 8 years ago
Test model:
model test_linebreaks2_orig Modelica.Fluid.Sources.FixedBoundary boundary(use_p=false, use_T=false, p=1e5) annotation(Placement(visible = true, transformation(origin = {-54, 42}, extent = {{-10, -10}, {10, 10}}, rotation = 0))); Modelica.Fluid.Sources.Boundary_pT boundary1 annotation(Placement(visible = true, transformation(origin = {-58, -28}, extent = {{-10, -10}, {10, 10}}, rotation = 0))); equation connect(boundary.ports[1], boundary1.ports[1]) annotation(Line(points = {{-44, 42}, {-44, 7}, {-48, 7}, {-48, -28}}, color = {0, 127, 255})); annotation(uses(Modelica(version = "3.2.2")), Icon(graphics = {Polygon(origin = {-13.1502, 17.2813}, points = {{-20.8498, 26.7187}, {75.1502, 6.71867}, {15.1502, 54.7187}, {-74.8498, 22.7187}, {-42.8498, -55.2813}, {-20.8498, 26.7187}})})); end test_linebreaks2_orig;
becomes
model test_linebreaks2_orig Modelica.Fluid.Sources.FixedBoundary boundary(use_p = false, use_T = false, p = 1e5) annotation(Placement(visible = true, transformation(origin = {0, 62}, extent = {{-10, -10}, {10, 10}}, rotation = 0))); Modelica.Fluid.Sources.Boundary_pT boundary1 annotation(Placement(visible = true, transformation(origin = {-24, -32}, extent = {{-10, -10}, {10, 10}}, rotation = 0))); equation connect(boundary.ports[1], boundary1.ports[1]) annotation(Line(points = {{10, 62}, {10, 7}, {-14, 7}, {-14, -32}}, color = {0, 127, 255})); annotation(uses(Modelica(version = "3.2.2")), Icon(graphics = {Polygon(origin = {-13.1502, 17.2813}, points = {{-20.8498, 26.7187}, {75.1502, 6.71867}, {15.1502, 54.7187}, {-74.8498, 22.7187}, {-42.8498, -55.2813}, {-20.8498, 26.7187}})})); end test_linebreaks2_orig;
comment:10 by , 8 years ago
No, it does not fix this yet. The first commit only fixes the problem with comments moving around. Fixing the moving of whitespace for a component whose annotation does change might be possible to fix as well.
Of course OMEdit could solve test_linebreaks2_orig by doing the diff after changing each annotation. Having the diff do 3 modifications at once is what makes everything tricky...
comment:11 by , 8 years ago
A smaller test for the second part:
echo(false); a1 := "model test_linebreaks2_orig Boundary boundary(use_p=false, use_T=false, p=1e5) annotation(Placement(visible = true, transformation(origin = {-54, 42}, extent = {{-10, -10}, {10, 10}}, rotation = 0))); equation connect(a, b) annotation(Line(points = {{-44, 42}, {-44, 7}, {-48, 7}, {-48, -28}}, color = {0, 127, 255})); end test_linebreaks2_orig;"; b := "model test_linebreaks2_orig Boundary boundary(use_p = false, use_T = false, p = 1e5) annotation(Placement(visible = true, transformation(origin = {0, 62}, extent = {{-10, -10}, {10, 10}}, rotation = 0))); equation connect(a, b) annotation(Line(points = {{10, 62}, {10, 7}, {-14, 7}, {-14, -32}}, color = {0, 127, 255})); end test_linebreaks2_orig;"; echo(true); "a1"; diffModelicaFileListings( a1, b, OpenModelica.Scripting.DiffFormat.color); getErrorString();
comment:12 by , 8 years ago
I have a small and simple fix for that small example, but it fails on your original test_linebreaks2_orig... even corrupting the model.
comment:13 by , 8 years ago
e0cca96/OMCompiler should handle the second example (also backported to maint/1.9).
comment:14 by , 8 years ago
Please do also have a look at #4110 -- additional newline introduced if changing a model parameter.
Moreover: when creating new models, then everything seems to get arranged without newlines, i.e. long lines with modifiers and annotations. How could this be improved?
comment:15 by , 8 years ago
Martin: Tack saa mycket!
rfranke: Personally, I find the endlessly long lines that seem to be prevalent in Modelica pretty annoying, too. To mitigate this, I try to break after every function argument and indent to get a vertical(ish) list with not too much width, and let the accompanying annotation run to the right after the closing parens, so it stays mostly out of the way (see comment:11)
One way to approach this problem would be to formulate/propose some style guide to handle breaking lines between multiple function arguments, see e.g. the way Python does this here - I find that readable and usable. Also, I keep to not using whitespace around keyword argument equal signs:
Yes: def complex(real, imag=0.0): return magic(r=real, i=imag) No: def complex(real, imag = 0.0): return magic(r = real, i = imag)
Another way, to at least mitigate, would be to color annotations and their arguments in light gray or whatever, since (IMO) annotations are often pretty unimportant in understanding the code, and just add much visual noise, which would be thus reduced.
comment:16 by , 8 years ago
Such guidelines are really missing. Interesting that Python uses no spaces for "modifiers". Common Modelica styling considers the length of a line. Having no spaces for modifiers simplifies automatic linebreaks:
parameter Real a "my parameter" annotation(Evaluate=true);
vs.
parameter MyPackage.ManyManyManySubPackages.MoreSubPackages.LongModelName a "my parameter with long doc string and a long elaborative description text" annotation(Evaluate=true);
comment:17 by , 8 years ago
Milestone: | 1.11.0 → 1.9.x |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
I just realized that also spaces in the argument list are changed/added for no apparent reason:
becomes