-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFCs]: Magnition Proposal - Localised Layout Options #4
base: main
Are you sure you want to change the base?
Conversation
This PR proposes RFCs for the following two features: - Localized Layout Options - Template typename passing with inheritance Signed-off-by: mkhubaibumer <[email protected]>
Hi, I suggest one RFC per PR otherwise the discussion might be hard to follow. |
Regarding localized layout options: I think the main interface for this might be the layout options view (not the sidebar) for klighd-vscode kieler/klighd-vscode#83. This view should have the possibility to serialize the changes, which would be implemented via a service interface on the server side. Whether the option serialization would take a separate file could be language specific and it would work as long as the option file is loaded in the synthesis. My take would be to contribute to klighd-vscode first and see whether a simple tree view with all layout options set is enough for your use case. If it is not, the client could be extended for a different kind of interaction using the klighd-vscode LSP messages build for the new view. |
Thanks for submitting this PR @mkhubaibumer! Could I ask you to split the two RFCs into two PRs? This makes it a lot easier for us to review and discuss them individually. Also, it seems like a bunch of additional files were added to the diff of this PR. |
@soerendomroes @cmnrd sure. I'll split this in 2 PRs asap |
Signed-off-by: mkhubaibumer <[email protected]>
Signed-off-by: mkhubaibumer <[email protected]>
@cmnrd @soerendomroes @lhstrh I've split the PR into 2. |
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.
Thanks! I think this is an interesting idea. Do you have a concrete idea how this could be integrated with the LF language? One obvious solution that comes to my mind is adding annotations within LF code using our attribute syntax. So we could expose some (or all) of the available options as attributes and allow for local overrides.
Another important question is feasibility on the KIELER side. @soerendomroes @a-sr Does the framework only consider global rendering options, or is there support for local options too?
Also, I wonder if filtering could be an alternative solution to your problem. If you can somehow select which specific reactors to render, you could better select the appropriate rendering options. Would that make sense?
@cmnrd that's an interesting idea! But would lingua-franca be open to generating annotations in code by changing layout option for certain reactor instantiations Its much easier to do it from diagram then write these annotations in code in my opinion |
That is something we should put up for discussion in the broader team, but personally, I am open to that. Doing it in the code and also updating the code automatically certainly has some downsides, but I think the main advantage would be that we still have the diagrams representing only the code and there would be no external metadata that gets out of sync. However, before we get into the details of this, I think it would be most important to understand the feasibility on the side of the diagram backend. |
I agree that this is a big open question and also not a question that I am well-prepared to answer quickly.
For this the big open question is not whether we can add the annotations in the code (we can do that, it is easy) but rather what the graphical UI would look like. UI design is a complex issue that is outside my expertise and I am worried that it could get pretty difficult to support this without causing a lot of confusion for non-expert users. |
In our diagram framework, we distinguish two types of configuration options. Synthesis options affect how the diagram is created (e.g. if state variables are visible); these are located in the side bar. Layout options are set implicitly by the synthesis and affect only the layout/arrangement of the diagram. Yet, if you are interested in synthesis options here, this will require an adjustment of the LF diagram synthesis because it treats the settings in the side bare as a global configuration, e.g. In terms of usability, if writing annotations is not desired, one could think of adding them via a context menu for model elements, similar to structure-based editing. However, this is not yet support in Klighd but @soerendomroes could say more to this topic. |
Interesting. Thanks for the explanation. I actually completely forgot about the |
We are planning to add structure-based editing into Klighd and will rework the interactive layout constraint framework, which was a very similar workflow to what you may be envisioning. For me, the question remains, whether layout options are enough for you or whether you want to change synthesis options |
The advantage of annotations is that it gives you a way to serialize (i.e., save) layout options. If not done that way, what would be an appropriate place to save them? Do sprotty or klighd perhaps already have an established mechanism for this, @soerendomroes? |
@lhstrh We are currently serializing them if possible. This might be in the next klighd/klighd-vscode release, maybe in the one shortly after that. For interactively setting layout constraint to constrain the topology of the layered layout. We are currently we are trying to serialize a constraint if the given language has a serializer, otherwise, we only work on the view model (KGraph) such that this changed will not be persisted and will be gone on reload. The synthesis options are usually saved in the browser for a corresponding language (which is the way VS Code usually handles things). |
rendered
This PR proposes RFCs for the following feature: