Skip to content
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

Enable override of Tracing editor #1005

Merged
merged 6 commits into from
Jul 15, 2024
Merged

Conversation

mgronover
Copy link
Collaborator

@mgronover mgronover commented Jul 10, 2024

This Ticket addresses: JetBrains/MPS-extensions#855

A ConditionalEditor by default has the hints conditionalEditor and conditionalEditor_doNotUseThisHint. These hints are enabled by default when the plugin becomes initialized.
If someone wants to overload a conditionalEditor it is not enough to just give it a higher priority. The priority just defines the order of the applied editors (a conditionalEditor uses [NextEditor]) see http://127.0.0.1:63320/node?ref=r%3A8d20232d-87e2-425b-b4d7-a9790e401b85%28de.slisson.mps.conditionalEditor.structure%29%2F2877762237607058140)" (s. screenshot on the mps-extension ticket above). So these can be cascaded somehow.

To enable overloading:
Generally an implementation of a conditional editor should also get an new defined editor hint if this editor should be able to be overloadable. The editor implementation that overloads that one must disable that and enable its own editor hint.

For the Tracing editor this is done here.

@mgronover mgronover marked this pull request as ready for review July 11, 2024 06:54
@mgronover
Copy link
Collaborator Author

See also the issue #974 and the according PR: #980

Copy link
Member

@arimer arimer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the invetigation and the changes. 👍🏼

  • The implementation looks fine and I like the idea of setting the editor-hint on startup of the plugin
  • I cleaned up some deps and updated the build

However we should definitively document that change in the changelog before merging. Could you please update it?

@arimer
Copy link
Member

arimer commented Jul 15, 2024

@mgronover we should also think about about updating our NextEditor to allow removing not needed editor hints. Could you please create a ticket in mps-extensions and reference your PR there?

@arimer arimer self-requested a review July 15, 2024 07:20
Copy link
Member

@arimer arimer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go 🚀

@arimer arimer merged commit a3933cd into maintenance/mps20222 Jul 15, 2024
1 check passed
@arimer arimer deleted the bugfix/855-Tracing branch July 15, 2024 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants