Opened 9 years ago

Closed 6 years ago

#3724 closed enhancement (fixed)

New approach to OMEdit text editing

Reported by: Francesco Casella Owned by: Adeel Asghar
Priority: blocker Milestone: 2.0.0
Component: OMEdit Version: v1.9.4-dev-nightly
Keywords: Cc: omusability@…

Description (last modified by Adrian Pop)

I report here the results of a discussion on omusability@….

Keeping the formatting consistent on models and packages spread over multiple files while hiding the file structure is an implementation nightmare, and also obscure and confusing to people using file-based revision control systems (SVN, GIT, etc.).

Proposal: whenever a specific class is selected in the package browser, the entire file where the class is defined is shown in the text mode editor, with this additional features:

  1. the part of the file where the code of the selected class resides is clearly marked (e.g, with a white background, while the rest of the file might have a grey background or something similar)
  2. the cursor is brought at the first line of the selected class automatically
  3. there is proper indentation support when hitting the newline key: if the model is nested several levels in a subpackage contained in one file, and the package is organized with proper indentations, one should not have to enter 12 or 16 spaces manually to keep the indentation consistent. Note that Dymola hides the indentation of a class definition in a package when showing it in the editor, and then somehow reapplies it back to the code, which is somewhat arbitrary and potentially error- prone, see above)
  4. the line numbering of errors and warnings given by omc matches the line numbering in the editor

At this point, whatever changes one makes to the entire file (possibly not limited to the selected class!) are accepted by OMEdit (as long as they are syntactically correct) when the focus is changed to another window, and the package browser will automatically reflect the new situation.

This should be relatively straightforward to implement, much easier to understand for end-user, and a lot better for managing files on RCSs without messing up things.

Members of the OMEdit usability group, please state your opinion on this ticket.

Change History (27)

comment:1 by Adeel Asghar, 9 years ago

I think you meant "package with multiple classes stored in one file" instead of "models and packages spread over multiple files".

This is possible but will make OMEdit very slow. Consider a package P with 20 nested classes named as A1, A2....A20. If user just want to update A10, we actually have to reload all classes since we are displaying the whole package contents and user has the liberty to update whatever he wants. This will affect the performance by the factor of N. The bigger the package the slower OMEdit.

Right now we do it as Dymola, we store the indentations and removes them before showing the nested class text to user. Once the user is done with editing we glue the nested class back in the package and put back the stored indentations.

in reply to:  1 ; comment:2 by Francesco Casella, 9 years ago

Replying to adeas31:

I think you meant "package with multiple classes stored in one file" instead of "models and packages spread over multiple files".

Yes, of course, sorry.

This is possible but will make OMEdit very slow. Consider a package P with 20 nested classes named as A1, A2....A20. If user just want to update A10, we actually have to reload all classes since we are displaying the whole package contents and user has the liberty to update whatever he wants. This will affect the performance by the factor of N. The bigger the package the slower OMEdit.

Not necessarily. One can make a diff between the new text and the old text, and only reload those classes that are touched by the patch file. In most cases, one would only work on the class he has selected, and then there will only be one class that needs to be reloaded. A text diff should be no big deal.

in reply to:  2 comment:3 by Adeel Asghar, 9 years ago

Replying to casella:

Replying to adeas31:

I think you meant "package with multiple classes stored in one file" instead of "models and packages spread over multiple files".

Yes, of course, sorry.

This is possible but will make OMEdit very slow. Consider a package P with 20 nested classes named as A1, A2....A20. If user just want to update A10, we actually have to reload all classes since we are displaying the whole package contents and user has the liberty to update whatever he wants. This will affect the performance by the factor of N. The bigger the package the slower OMEdit.

Not necessarily. One can make a diff between the new text and the old text, and only reload those classes that are touched by the patch file. In most cases, one would only work on the class he has selected, and then there will only be one class that needs to be reloaded. A text diff should be no big deal.

No that's not entirely true. OK we can avoid loading all classes but we anyway have to update the text of all classes. We also have a to update the text of all classes, when lets say user adds A21. This change should be shown in text of all classes then.

To be honest, I was the one who discussed and proposed this solution with Martin. I already tried to implement it but its not that easy. The way we have it now is quite fine and preserves indentations rather well.

comment:4 by Christoph <buchner@…>, 9 years ago

To be honest, OMEdit hiding the file structure, and faking "files" for classes (making up line numbers and all that) was really really confusing when I started working with OM (using an RCS, too, of course).
Add the fact that at the time OMEdit was messing up package structure when you tried to divide your code in several files (has been fixed already), and I tried to correlate what OMEdit showed with what was in the text files, and what changes I saw in version control when OM made my code disappear, and nothing matched up! Quite a nasty surprise, and I'd also prefer that OM show the plain file structure, especially considering new users.

comment:5 by Francesco Casella, 9 years ago

Cc: omusability@… added
Description: modified (diff)

I agree with Christoph completely. People developing Modelica code (or any kind of code, actually) should always use an RCS. I'm trying hard to educate my students and colleagues to this basic concept, when they start sending me zipped files with Modelica code, God forbid. Trying to hide the file structure from the GUI just makes the whole thing confusing and unnecessarily harder to grasp.

As a consequence, Modelica tools should make life as easy and intuitive as possible to them. And it shouldn't be and expensive add-on, it should be built-in by design.

Some competitor tools have never shined on this front, I'm afraid, and this has impacted the community for a long time. I wish I had a more "text-editor"-oriented Modelica tool back in 2005-2007, when I was actively contributing to the Modelica.Media library, and I had to use a text editor if I wanted my intended changes only to show up in the SVN diffs (just change that single line in package.mo, please...). I am still resorting to text editors to develop portable Modelica code, for the very same reason. OK, I could use Eclipse for that, buth then I'd lack the GUI when it makes sense to use it.

In general, commercial Modelica tools try to hide the Modelica details to various extents (up to and including not actually showing the Modelica code at all). I think OpenModelica's approach has to be more transparent, that is, more open and more Modelica oriented :)

Let's do this right this time, and let's try to avoid copying features of other tools that have not proven to be satisfactory for RCS-based code development.

OMusability group members, please give your opinion by adding a comment to this ticket.

comment:6 by Francesco Casella, 9 years ago

BTW, there is only one case where it would really be useful and nice to hide stuff: annotations. Reading code in the text mode with all the graphical annotations is really painful, because humans (unlike parsers) are not good at matching parentheses, annotations are usually not pretty printed, and as a consequence they obfuscate the equations and the declaration quite a bit.

I'm not sure how this can be handled in the framework of what I just proposed in comment:5. Maybe an easy starting point would be to have a button that (optionally!) changes the font color for annotations to a very light grey, so one can clearly distinguish the annotations from the rest of the code.

Another option is to (optionally) collapse the lines only showing up comments, and put some markers in place of annotations in those lines that hold annotation and non-annotation code. This is probably a bit harder to implement, possibly a bit nicer to see.

in reply to:  4 comment:7 by Martin Sjölund, 9 years ago

Replying to Christoph <buchner@…>:

I'd also prefer that OM show the plain file structure, especially considering new users.

I would even go as far as integrating OMEdit with git if we had enough time to implement such features... Quite often there are commits to github repositories where there are suddenly files missing (OM seems to be the only tool checking the package.order where it is possible to detect that files are missing; the other tool vendors voted for making such a check only allowed to give warnings, not errors).

comment:8 by Francesco Casella, 9 years ago

This would be nice to have, as long as the git plug-in tells me explicitly what it is going to commit or push, i.e., show me all the changed files and the changes in those files in clear before I hit the commit button.

I always inspect this stuff before committing, and I will never trust any tool that commits changes to repositories for me without telling me what it is doing exactly.

TortoiseGIT supports this in a very nice and intuitive way. Unfortunately it is only available in Windows. OK, we can always show the raw ouput of the git command, but that's not for the faint-hearted :)

comment:9 by Christoph <buchner@…>, 9 years ago

I think a git plugin whould be out-of-scope for OM. I mean, we're editing plain text files, git is already very good at handling this. Why not let the user just work with her preferred git interface that she is used to, be it command-line, Git Gui, gitg, Git Kraken,....?

Also, what if the user uses Mercurial/Perforce/SVN/... instead of Git? Using OM should be possible with those, too, given the underlying files are plain-text.

comment:10 by Christoph <buchner@…>, 9 years ago

Regarding @casella's point about annotations: This would be purely the job of Syntax Highlighting, see e.g. this example for how this could look nice.

in reply to:  10 comment:11 by Francesco Casella, 9 years ago

Replying to Christoph <buchner@…>:

Regarding @casella's point about annotations: This would be purely the job of Syntax Highlighting, see e.g. this example for how this could look nice.

I'll love to have something like this :)

comment:12 by Andrea Bartolini, 9 years ago

I completely agree with the concept to show and work directly on the "real" file structure, as usually is done with any programing IDE.
This is more clear for the modeler and easily allow to use any text-based code revision control tool (like SVN or GIT....) .
Moreover, I think it is not strictly necessary to have sophisticated text-editor feature, it is enough (at least to start) to have a behavior like a programming IDE, i.e. to have a browser to reach and place the cursor to the begin of a class or package...., to have syntax highlighting that recognize also the annotations and change their color to light gray (it is not strictly necessary to hide them), to have some function as "go to definition" of a model.... the same as an IDE.
Regarding the classes reloading, it is true, in a big package this may be slow... one solution could be give the modeler the possibility to choose (using a button) when the logical class view shall be updated, instead of an automatic update at any change (it may be a configuration flag). This concept should be applied also to the sintax control, that should be invoked by the user, not automatic (sometimes may be more productive to write an entire model before to check the syntax, like an IDE... now it is no possible if you want periodically save your work).
Finally.... it could be very nice to have a sintax completion... but this is another story...

comment:13 by Andrea Bartolini, 9 years ago

Last but not least... To have an auto-indentation of the code... this is fundamental in a text-base editor...

in reply to:  10 comment:14 by Bernhard Thiele, 9 years ago

Replying to Christoph <buchner@…>:

Regarding @casella's point about annotations: This would be purely the job of Syntax Highlighting, see e.g. this example for how this could look nice.

I even wonder whether embedding components from the atom editor (linked to above) into OMEdit could be feasible approach. Apparently Microsoft used components from atom in its cross-platform Visual Studio Code app http://thenextweb.com/apps/2015/04/30/microsofts-cross-platform-visual-studio-code-app-is-based-on-githubs-atom-editor/.

No idea if this is possible, but maybe it would be an interesting master thesis topic to try so?

comment:15 by Christoph <buchner@…>, 9 years ago

To short-cut any searching someone might do: The "foundation" (for want of a more precise word) of Atom was split out into something called "Electron". VS Code also uses Electron as the foundation, so that's the component bthiele talks about.

comment:16 by Dietmar Winkler, 9 years ago

Using Atom or rather Electron as a basis looks definitely like a good idea since then one can use the folding (and preview) power of: https://atom.io/packages/language-modelica

comment:17 by Christoph, 9 years ago

Be aware though that Atom is rather heavy - it basically pulls in a whole Chrome web stack, just for a text editor.
What component is currently used? I am not well versed in editor components, but many seem to use CodeMirror, which [apparently https://codemirror.net/mode/modelica/index.html] also has some Modelica support.

comment:18 by Dietmar Winkler, 9 years ago

One of the missing features in CodeMirror though is the annotation-folding (at least I've never seen that).

comment:19 by massimo ceraolo, 9 years ago

As a member of the OMEdit usability group I try to say my opinion about this ticket.
I premise that I occasionally use IDE for C++ programming and I love the one I use (Qt creator).

I understand the request of having an IDE-like OMEdit editor, with all the features requested by users in this tickets, in particular those listed by Andrea Bartolini.

However I think that there are a lot of things on the OMEdit wish list that have still to be solved. Except this ticket, I think that the priority order should be the following one:
1) correct problems that cause very frequent crashes on windows (#3318)
2) implement the features that are still missing and listed in ticket #2894, mainly correct management of conditional connectors and replaceable classes
3) solve the efficiency issues discussed in ticket #2960
4) improve plotting features, at least adding the feature requested in ticket #2166
5) add some post-processing capability such as summing or subtracting curves, making FFT analysis and plotting the resulting spectra in bar plots (no tickets here. Maybe the new MSL will help on this).

Where should we put the "new approach in OMEdit text editing" in this list?
If I were in charge of this decision, I would put it probably just before or after item N. 5.

So, code completion and "go to definition" of a model would be nice, but in my opinion better to deal with them after having solved more urgent issues.

comment:20 by massimo ceraolo, 9 years ago

Naturally, I understand that not everything in my list is directly due to OMEdit issues or limitations, i.e. 1 and 3, but they directly affect the OMEdit user's experience.
I'm not sure, but probably OM users that work directly from OMC do not need any of the issues/enhancements in my list addressed. That's why I mentioned them here.

in reply to:  19 comment:21 by Bernhard Thiele, 9 years ago

Replying to ceraolo:

Where should we put the "new approach in OMEdit text editing" in this list?
If I were in charge of this decision, I would put it probably just before or after item N. 5.

Agree, IDE-like feature are not the top of the priority list. However, the initial proposal by casella is more restricted and I like it a lot. So if it should turn out that it could be implemented rather quickly one could also argue to prioritize it a bit higher.

comment:22 by Martin Sjölund, 9 years ago

Milestone: 1.9.52.0.0

Milestone renamed

comment:23 by Adrian Pop, 9 years ago

Isn't this already the case? OMEdit has a file oriented editing now.
Update: OK 1) 2) and 4) seems to be OK now, 3) would need implementing.

Last edited 9 years ago by Adrian Pop (previous) (diff)

comment:24 by Adrian Pop, 9 years ago

Description: modified (diff)

in reply to:  23 comment:25 by Adeel Asghar, 9 years ago

Replying to adrpo:

Isn't this already the case? OMEdit has a file oriented editing now.
Update: OK 1) 2) and 4) seems to be OK now, 3) would need implementing.

No. You missed the whole point here. This ticket is about a package saved in one file. So lets say we have a package saved in P.mo with contents,

package P
  model M end M;
end P;

Now if you open P.M in OMEdit you only see the contents of P.M and OMEdit magically removes the leading spaces, allows you to modify the class and then glue back the class in the package with proper leading spaces and saves it.
So what is requested in this ticket is to show whole contents of P and mark the upper and bottom section of P.M as read-only (grey background) and only allow user to modify P.M.

comment:26 by Francesco Casella, 9 years ago

The idea is to let users understand what happens to the source code files directly.

In principle, this is only useful if one uses a RCS. In practice, everybody should :)

comment:27 by Francesco Casella, 6 years ago

Resolution: fixed
Status: newclosed

I think what we have now in OMEdit is pretty close to what I was asking. In particular, line numbers are always present and always coincide with those of the text files, which ae reported by the compiler in case of errors. So, I guess I can close this ticket now.

Note: See TracTickets for help on using tickets.