Provide comment-preserving parsing and unparsing

[Francesco Casella]

The OMEdit GUI is based on OMC for all editing activities. When OMC passes the text of a class to the GUI, all original formatting information is lost, so that, as soon as the modified class is saved, the original formatting is lost on the file as well.

This makes it very hard to use OMEdit on modelling projects that use revision control (SVN, GIT, etc.) - all serious ones do. This also makes it impossible to use OMEdit to work on the Modelica Standard Library without screwing up the code. Last, but not least, this makes it impossible to use OMEdit on the same project in conjunction with other Modelica tools, which is a very likely user scenario in the near future.

The solution to this problem has already been studied and partially implemented 6 years ago (see: ​https://www.modelica.org/events/modelica2008/Proceedings/sessions/session6a3.pdf).

It is now of the utmost importance that this solution is implemented in OMC, so that OMEdit becomes usable in the above-mentioned scenarios.

[massimo ceraolo]

A special case is reported here.
Consider the following model

model aa
  Real x;
algorithm
  if time > 0.5 then
  //this is a comment
    x := 2;
  else
    x := 1;
  end if;
end aa;

when saved it becomes:

model aa
  Real x;
algorithm
  if time > 0.5 then
    x := 2;
  else
    x := 1;
  end if;
  //this is a comment
end aa;

This makes it particularly difficult to make clear documentation of code.

Another example is constituted by the block comments: /*...*/.
The text of the rows following the first one is moved right, often by several characters.
I've not checked the details, but after some work it often happens that the commented lines are moved by several tens of characters right, making them hardly visible when scrolling the code.

[Christoph <buchner@…>]

I have also observed this previously, and it was pretty annoying. And yes, I also use revision control for projects (kind of insane not to, to be honest)

[Francesco Casella]

I had some discussion with Martin Sjoelund about how to get this done. One possibility, which is referred to in this ticket, is to parse comments and whitespace in the AST, so that it is possible to re-generate the same code when saving the modified model. However, this might be unnecessarily complicated and time-consuming to implement and debug.

Another option to solve this issue is that OMEdit works directly on the source text files, instead of working on their internal representation in OMC after parsing them. The easiest way to implement this is that OMEdit just edits the text file, and then has OMC re-load and re-parse it when the model is saved, checked or simulated. If it is possible to do this without having to re-load the entire root-level package the class being edited belongs to, this might turn out to be fast enough, after the recent improvemens in DLL communication between OMEdit and OMC. Alternatively, it would be possible to keep track of the correspondence between the tokens in the text file and the tokens in the parsed AST, so that it is possible to update the latter after changes to the text file without reloading everything from scratch.

Last, but not least, if OMEdit starts working on the text files directly, it would probably be much easier to eventually build direct support of revision control systems in OMEdit, as all these systems use files as basic storage entities. RCS support is essential for serious industrial use of OMEdit.

I would suggest the developers to discuss these options out of the box - maybe a good solution is easier (and less bug-prone) to implement than we thought.

[Martin Sjölund]

Well. My idea would be to re-parse after every automatic change by OMEdit (opening the parameters dialog and setting a modifier for example). And comparing the old file to the tokens in the new one and simply performing the changes in the old file to get a minimal diff.

When manually editing a model in OMEdit, the positions in the source file should be noted and exactly those changes be passed to the updated version of the file. No other changes should be incorporated.

Probably, each opened file needs to have one representation in OMEdit (the actual text) and another in omc (the AST).

It is also essential that OMEdit does not change the directory structure when saving files (this happens quite frequently). Because doing this makes it impossible to support any RCS.

[Martin Sjölund]

Milestone changed to 1.9.3 since 1.9.2 was released.

[anonymous]

There is a special case of reformatting that maybe can be solved with a limited programming effort (while waiting for a more complete solution to this ticket's issue).
Consider the following model:

model CommentBug " Model"
  /* This is a several-line comment
     line two  
     line three
     line four
  */
end CommentBug;

Every time this model is saved in OMEdit lines two, three, four, and the comment ending, are displaced to right by two spaces.
If this comment is part of a large model that is worked with for several weeks, in the end it might get very far from the beginning of line.
Or the user must continually manually reposition all the multi-line comments or, better, totally avoid them.

Tested with r24813.

[Martin Sjölund]

I added a listFile API as a first step: 6293f8f/OMCompiler

Now I just need to decide between using an OMCCp lexer and compare files in OpenModelica, or to use flex.

[Martin Sjölund]

I now locally have a version of Myer's diff algorithm (the greedy one) implemented in MetaModelica. It can produce diffs for sequences of integers or strings (see omc-diffalgorithm.svg​).

With an OMCC-based lexer we should also be able to lex two Modelica files to create some sequences of tokens to compare. And then we process the diff (edits) to insert the changes in a way that gives as few changes as possible to the original text-file (despite the OMC unparsing messing up the formatting).

[Martin Sjölund]

The ModelicaDiff lexer and testcase has been merged into the compiler. Missing is an API function for OMEdit to call, which does the work. And a whole bunch of testcases to make sure that everything works correctly. The examples in this ticket have already been added to the test suite and produce an empty diff as expected.

The way the diff works right now is:

• Lex before and after
• Compare before and after
• If a comment has been moved (deleted+added), move it back to its original place
• Lex before and the partial merge
• Compare before and partial merge
• Print the diffs (or final output, depending on what you wanted)

[Martin Sjölund]

c5516d9/OMCompiler includes an API that OMEdit can use to reduce the number of changes done to the source code.

[Martin Sjölund]

Moved to new milestone 1.9.4

[Adrian Pop]

The following tickets need to be fixed for new OMEdit: #3417, #3460, #3552, #3574

[Francesco Casella]

See also #3642 and b1e94ad

[Adeel Asghar]

Seems like this ticket is done. I think we should close it now.

[Francesco Casella]

Unfortunately, not yet. We need to fix #3698 first, I discovered it today. I hope there are no other such problems around. It might be wise to do some more testing...

[Francesco Casella]

Still one more issue left.

Create a model M, add some stuff in it with some extra spaces and save it. Then duplicate the model. The duplicated model does not preserve the spaces and formatting of the original one, and is rather formatted to OMEdit's liking.

[Adeel Asghar]

Yes, because duplicate uses the copyClass API and omc pretty-printing.

[Francesco Casella]

I guess it shouldn't, then :)

The basic principle should be that any automatic pretty-printing should be optional, not automatic

[Adeel Asghar]

But then how are you going to duplicate the class? I mean how can you duplicate a package saved in directories? How can you duplicate a class and put it into another package and then update the components relative paths and so on.

[Francesco Casella]

If the model is duplicated in the same scope, there is actually nothing to change in the code of the duplicated model, except the name of the model in the first and last lines. It seems weird that the formatting is completely changed in this case. Maybe we could at least try to preserve the formatting in this specific circumstance. If you duplicate in a different scope and there are component paths to change, let OMC pretty-printing kick in.

Duplicating a model and then modifying it is a very commmon development pattern, so losing the formatting each time and having to reapply it manually seems really a big waste of time.

[Martin Sjölund]

Adeel: You can use the diff API using the original file(s) for the class and compare them to the new ones. This should be able to preserve some things better.

[Martin Sjölund]

Milestone pushed to 1.9.5

[Martin Sjölund]

Milestone renamed

[Francesco Casella]

I guess this historical ticket can be closed now, as almost all the pending issues have been solved. We are using OMEdit routinely for the development of a prototype power system library and we haven't encountered any big problems for a long time now.

The only issue left is moved to #3912

[Martin Sjölund]

Milestone deleted