search

OpenModelica / BioChem

BioChem is a package for biochemical modeling and simulation with Modelica
7 stars 12 forks source link

Move annotations to the correct position in a class #18

Closed sjoelund closed 3 years ago

sjoelund commented 3 years ago

The Modelica grammar does not allow annotations except at the end of a class.

This prepares the package for using the conversion script and is related to #17.

sjoelund commented 3 years ago

I could possibly try to improve OMC to make this diff a little smaller by not adding whitespace between some of the modifiers. I don't think that's intended. After this I could add the conversion script, which would make that diff much easier to see what actually changes for MSL 4.0.0

sjoelund commented 3 years ago

Ah, those are due to the merging failing and defaulting to put poorly formatted code in there...

AtiyahElsheikh commented 3 years ago

Difficult to see the differences due to formatting

sjoelund commented 3 years ago

This is now a better diff with annotations moved manually. It should make the actual conversion script possible to see what the changes are. Moving the annotations automatically seemed to be quite difficult for the diff algorithm. It's not been allowed since Modelica 2.x as far as I know (but OpenModelica always supported it for backwards compatibility)

sjoelund commented 3 years ago

Note that this is still not that easy to see what the changes are because there are similar lines in the middle of what was moved. But it's simply cut-and-paste of annotations to ends of classes...

AtiyahElsheikh commented 3 years ago

I assume that there is no need to merge this branch and it is enough to merge Pull Request # 19

sjoelund commented 3 years ago

Well, this should probably also be merged since BioChem is following the Modelica language 2.x grammar instead of the 3.x grammar. It would also make working with OMEdit produce nicer diffs in the future since it automatically moves annotations to the end of files when you make changes to a file.

sjoelund commented 3 years ago

So I think this should be merged first and part of the next release.