Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#3658 closed defect (fixed)

OMEdit doesn't allow to add or change comments

Reported by: casella Owned by: sjolund.se
Priority: blocker Milestone: 1.9.4
Component: OMEdit Version:
Keywords: Cc: andrea.bartolini@…, dersh

Description

Load the attached test package in OMEdit.

Select M1 and add a comment above the line Real x = 1;. Save the model. The comment disappears from the screen and is never passed onto the file.

Now select M2, which already has a comment. Edit the comment somehow. Save the model. The applied changes disappear, and are never passed onto the file.

Basically, it is no longer possible to add or edit comments with OMEdit...

Attachments (2)

TestComment.mo (138 bytes) - added by casella 9 years ago.
TestDiff.mos (622 bytes) - added by sjoelund.se 8 years ago.

Download all attachments as: .zip

Change History (13)

Changed 9 years ago by casella

comment:1 Changed 9 years ago by casella

  • Cc andrea.bartolini@… added

comment:2 Changed 9 years ago by dersh

  • Cc dersh added

FYI, there are a number of tickets that appear to be for this same issue already. It really makes OM unusable, so hopefully will be fixed at some point.
3635
3624 and
3634

For my own use, I have reverted to 1.9.4~dev-621-ga2f446d which still has some oddities with comments and moving text around, but is basically functional (at least I haven't had it just make text disappear completely)

comment:3 Changed 9 years ago by adeas31

Basically its not an OMEdit issue. Infact all the above tickets mentioned by @dersh are related to #3642. Martin recently updated the diff algorithm we were using to make it more faster and that unfortunately broke everything.

comment:4 Changed 9 years ago by casella

  • Owner changed from adeas31 to sjolund.se
  • Status changed from new to assigned

Martin, can you please roll back your commits until this is fixed, and work on a separate fork to fix the bug? A slow implementation is preferable to a broken one!

Thanks

comment:5 Changed 9 years ago by anonymous

Done in ee4e7b6 (using the old list()-style unparsing; will change the models but preserve all relevant information). I also have an idea on how to add some sanity checks to the diff algorithm: after you get the merged result, parse both the merged and the after strings, unparse them and compare the strings. If they are not equal, the diff algorithm failed: report an error and just return the after string.

comment:6 Changed 8 years ago by casella

  • Resolution set to fixed
  • Status changed from assigned to closed

Problem solved

comment:7 Changed 8 years ago by sjoelund.se

  • Resolution fixed deleted
  • Status changed from closed to reopened

It was not resolved. It was just hacked around.

Changed 8 years ago by sjoelund.se

comment:8 Changed 8 years ago by sjoelund.se

I will consider this fixed once the attached testcase works.

comment:9 Changed 8 years ago by sjoelund.se

  • Resolution set to fixed
  • Status changed from reopened to closed

comment:10 Changed 8 years ago by sjoelund.se

  • Milestone changed from 1.9.4 to 1.9.4-1.9.x

Milestone renamed

comment:11 Changed 8 years ago by sjoelund.se

  • Milestone changed from 1.9.4-1.9.x to 1.9.4

Milestone renamed

Note: See TracTickets for help on using tickets.