-
Notifications
You must be signed in to change notification settings - Fork 28
Implement simple completer for Modelica annotations (task#5333) #219
base: master
Are you sure you want to change the base?
Implement simple completer for Modelica annotations (task#5333) #219
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some documentation to the functions so we know what they are doing?
AFAIK there is no database of annotations from where we can fetch the information. I believe we need to build it inside OMEdit. Maybe something like,
typedef struct {
QString name;
QStringList parents;
} Annotation;
QList<Annotation> annotations;
annotations.append(Annotation("Documentation", QStringList()));
annotations.append(Annotation("info", QStringList() << "Documentation"));
annotations.append(Annotation("revision", QStringList() << "Documentation"));
annotations.append(Annotation("experiment", QStringList()));
annotations.append(Annotation("StartTime", QStringList() << "experiment"));
Then have a function which fetches the annotation based on the parent.
QStringList fetchAnnotations(QString &parent)
And finally add the fetched list of annotations as CompleterItem
.
Note that I used parents
as QStringList
so we don't have to repeat the same annotation twice. Of course there could be other better ways as well. I haven't given it enough thought.
|
||
void ModelicaEditor::getCompletionAnnotations(const QStringList &stack, QList<CompleterItem> &annotations) | ||
{ | ||
QString key = stack.join("."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of having a QStringList
here. I guess you are always expecting a scalar value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, it is always converted to scalar QString
value, but it is at least supposed to be sequence like annotation(a(b(cd<cursor is here>=1)))) --> ["a","b"]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should build a tree structure of annotations then.
Well anything that works is fine. The only thing is it should be good enough to extend it with future annotations. Right now we have so many annotations and I prefer just one place to specify them once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe collect them in some help package in their record
-like form? Then we could specify description strings and other metadata, not only names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then we have to do this in the OMCompiler. I prefer some solution in OMEdit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify: I don't propose to implement some auto-generated package in OMCompiler, I just propose to make some Modelica library just like ModelicaReference.Annotations
that would be made just of records closely resembling actual annotations. Unfortunately, current documentation looks human-, but not very machine-readable.
This would make annotation descriptions:
- easy to edit "in their textbook form" :)
- inherently hierarchical, no need to reimplement the wheel in C++
- they would have data types and descriptions
- they would be edited by
OMCompiler
team :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meanwhile, it would be interesting to automatically fetch descriptions for completer keywords from ModelicaReference
.
Added documentation comments |
We do have the annotation information as constants strings in OMC for the different lang standards: |
Maybe it would be worth having some "suggestable documentation"? Such as |
@atrosinenko: i agree, we should have such a library, we can put it where ModelicaBuiltin.mo is or just make it part of it. |
Constants.mo only has the graphical annotations (mostly the records used to define them). For auto-completion you would probably like to have things like IncludeDirectory and also let OMEdit know what type(s) this annotations expects. |
@adrpo Should I add it myself? If yes, then maybe use some user accessible place like |
@atrosinenko: yes, you can add it yourself. We should not make this part of the ModelicaReference as we do not have control over that library, is distributed as part of Modelica. I guess we can have our own |
I think this could be of general use for the Modelica Association, it would be a shame to keep it on our own. I'll check with the MAP-LIB leader @beutlich. Do we have a description or sample to demonstrate how this thing will look like? |
@casella On screenshots, with a simple modification to this PR (specifically, to the On annotation records in a library, it is not yet implemented, but supposed to be dispalyed like in recent code completion with description strings. |
I had a brief e-mail exchange with @dietmarw, library officer for ModelicaReference. He's fine that we put this in ModelicaReference and he waits for a PR. I would suggest to start ASAP with a stub that he can give a look at, and then complete it afterwards. |
@atrosinenko, ModelicaReference is part of of the Modelica Standard Library repo |
@casella then it means we can only specify standardized annotations. What about the vendor specific annotations? |
@adeas31 technically, it would probably be useful to have a reference for "foreign" annotations to aid understanding, porting, etc. Is vendor specific documentation explicitly prohibited there? For example, |
Good question. I think the @atrosinenko, there is an agreement that the Modelica Standard Library does not depend on vendor-specific annotations for the models to work correctly. In other words, only standardized annotations can show up there, which makes sense if you think of it. It took some time to get rid of all the Dymola-specific annotations (or to turn them into standard annotations), since the MSL has mostly been developed using Dymola, and there may still be some left in older versions, but version 3.2.3 should be completely free of them. If you still find some of them left, please open an issue on the Modelica Standard Library GitHub website. |
@atrosinenko Only |
@hkiel And what on reference of vendor annotations? On one hand, Wikipedia does not depend on LHC for its operation, and still has article on it -- looks like presence of all the vendor annotations in one place only makes user experience better. On the other hand, this would mean that some vendors are worth of including their annotations and others are not, like if some implementations are officially better than other... But, after all, on FMI standard page there exists large list of test results for different implementations. |
@atronsinenko, as long as tool vendors take the responsibility of maintaining their own references, I see no problems. I suggest you move the discussion on the PR to ModelicaReference, so it can be discussed with the library officer and the MAP-LIB group |
Sent a PR to ModelicaReference, modelica/ModelicaStandardLibrary#2837, so we can discuss content-related questions there. |
modelica/ModelicaStandardLibrary#2837 (comment) |
Fine for me |
What if place it to a subpackage of
Or was it already suggested by @adrpo? |
Fetches completions from OpenModelica.AutoCompletion.Annotations
Use the current cursor position instead of end of word in PlainTextEdit::insertCompletionItem Some other minor improvements
@atrosinenko I fixed few things. This works fine now. Only issue is it is showing all the proposals even when inside the annotation. If you can fix it then its good to go in. |
@atrosinenko I added more annotations in OpenModelica/OMCompiler#2958. I also added default values for them. I am thinking we should update to generate the whole annotation with default values e.g., right now when you start typing |
@adeas31 Hmm... This depends on the default use-case. I fear adding all the options would be too much clutter. After all, maybe I just want to change Next, it may solve the both use-cases, if, when cursor is after |
Eliminated keyword, etc. completions when showing suggestions for annotation. Meanwhile, what is the proper way to fetch modifiers, so I can get default values, etc.? |
Use |
Moved to OpenModelica/OpenModelica#115 |
This PR implements merely a heuristic for annotation detection that works in common cases, though it should be helpful enough.
For now, it lacks actual annotations and their descriptions that can be suggested (just some examples). It would be great to get them from somewhere programmatically, still it probably wouldn't hurt to merge it as-is, for further extension.