Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pretty-printing of '*' '/' and '^' should include no space #9611

Closed
casella opened this issue Oct 28, 2022 · 2 comments · Fixed by #9616
Closed

Pretty-printing of '*' '/' and '^' should include no space #9611

casella opened this issue Oct 28, 2022 · 2 comments · Fixed by #9616
Assignees
Labels
COMP/OMC/Frontend Issue and pull request related to the frontend
Milestone

Comments

@casella
Copy link
Contributor

casella commented Oct 28, 2022

Description

Sometimes, e.g. when running conversion scripts, pretty-printing is triggered. One thing that is pretty annoying of that is that it adds spaces before and after the *, / and ^ operators. A cursory glance to the Modelica Language Specification clearly shows that the preferred formatting for those three operators is without spaces, which are instead used for + and -. This is particularly annoying when running MSL 4.0.0 conversion scripts, because it mangles the code formatting even in some lines which are not at all affected by the conversion.

Steps to reproduce

Load the following model in OMEdit

model M
  Real x, y, w, z;
equation
  w = 0;
  y = 1;
  x = (y + w)/2*z^2.5;
  annotation(
    uses(Modelica(version = "3.2.3")));
end M;

Right click on the package and select "convert to newer versions of used libraries". The converted model is formatted like this

model M
  Real x, y, w, z;
equation
  w = 0;
  y = 1;
  x = (y + w) / 2 * z ^ 2.5;
  annotation(
    uses(Modelica(version = "4.0.0")));
end M;

Desired Behaviour

The formatting of *, / and ^ should include no leading and trailing space.

perost added a commit to perost/OpenModelica that referenced this issue Oct 28, 2022
perost added a commit to perost/OpenModelica that referenced this issue Oct 28, 2022
perost added a commit to perost/OpenModelica that referenced this issue Oct 28, 2022
perost added a commit to perost/OpenModelica that referenced this issue Oct 28, 2022
perost added a commit that referenced this issue Oct 28, 2022
@perost
Copy link
Member

perost commented Oct 28, 2022

Fixed in #9616 by changing the Absyn dumper in the proposed way. For the sake of consistency we should also make the same change to the DAE dumper that's used to dump flat models, but that would require updating thousands of test cases. So that's probably not going to happen anytime soon.

@casella
Copy link
Contributor Author

casella commented Oct 28, 2022

Thanks @perost. Fixing this in the flat Modelica output would be nice, but less important, as this output is normally meant to be read by machines, not by humans. The important thing is not to screw up human-readable source code.

@casella casella added the COMP/OMC/Frontend Issue and pull request related to the frontend label Oct 28, 2022
@casella casella added this to the 1.20.0 milestone Oct 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
COMP/OMC/Frontend Issue and pull request related to the frontend
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants