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

refactor: migrate markers as first class plugins #951

Closed
wants to merge 5 commits into from

Conversation

miurahr
Copy link
Member

@miurahr miurahr commented Feb 7, 2024

  • Deprecate Core.registerMarker and Core.getMarkers
  • Remove MarkerController.init and WhitespaceMarkerFactory.init

Pull request type

  • Other (describe below)
    refactor

Which ticket is resolved?

What does this PR change?

  • Make NBSPMarker as true plugin
  • NBSP marker menu are created in the plugin
  • Removal of hard coded menu creation
  • When registering maker plugins with its class, and loading the plugin, static fields are initialized when loadPlugins static function is called. it breaks colors of markers.

Other information

Copy link

github-actions bot commented Feb 7, 2024

❌ Run Gradle test failed:

Copy link

github-actions bot commented Feb 8, 2024

❌ Run Gradle test failed:

@miurahr miurahr changed the title refactor: migrate all markers as first citizen plugins refactor: migrate markers as first class plugins Feb 8, 2024
Copy link

github-actions bot commented Feb 8, 2024

❌ Run Gradle test failed:

3 similar comments
Copy link

github-actions bot commented Feb 8, 2024

❌ Run Gradle test failed:

Copy link

github-actions bot commented Feb 8, 2024

❌ Run Gradle test failed:

Copy link

github-actions bot commented Feb 8, 2024

❌ Run Gradle test failed:

@miurahr
Copy link
Member Author

miurahr commented Feb 8, 2024

Now these 6 maker become true plugins.

    org.omegat.core.spellchecker.SpellCheckerMarker \
    org.omegat.gui.glossary.TransTipsMarker \
    org.omegat.gui.editor.mark.BidiMarkers \
    org.omegat.gui.editor.mark.ComesFromAutoTMMarker \
    org.omegat.gui.editor.mark.FontFallbackMarker \
    org.omegat.gui.editor.mark.ReplaceMarker \

Menu Items are generated dynamically using marker plugins IMarker interface.

Copy link

github-actions bot commented Feb 8, 2024

❌ Run Gradle test failed:

@miurahr
Copy link
Member Author

miurahr commented Feb 8, 2024

I propose addition of method void reloadDocument(); in IEditor core interface, that reload project contents and redraw markers when project is loaded.

Copy link

github-actions bot commented Feb 8, 2024

❌ Run Gradle test failed:

2 similar comments
Copy link

github-actions bot commented Feb 8, 2024

❌ Run Gradle test failed:

Copy link

github-actions bot commented Feb 8, 2024

❌ Run Gradle test failed:

Copy link

❌ Run Gradle test failed:

@miurahr miurahr marked this pull request as ready for review February 14, 2024 03:05
- Deprecate Core.registerMarker and Core.getMarkers
- Remove MarkerController.init and WhitespaceMarkerFactory.init

Signed-off-by: Hiroshi Miura <miurahr@linux.com>
@miurahr miurahr force-pushed the topic/miurahr/maker/register-as-plugins branch from c128c7f to 65241a1 Compare February 15, 2024 07:01
@brandelune
Copy link

@t-cordonnier review appreciated ! :-)

@t-cordonnier
Copy link
Contributor

@t-cordonnier review appreciated ! :-)

@brandelune I already sent some comments in this ticket.

I notice that now the menus are more dynamic as I suggested, good point. So, there was an evolution since my last comments, which I was not alerted about, so for me it was still pending.

The rest of the code seems unchanged so I maintain what I said: I would prefer one or more sub-interfaces to distinguish between non-skippable markers (the ones which are always active), menu markers (the ones which have a menu with an icon to activate/inactivate) and eventually, those which are skippable but whose configuration is done in another location than in a menu.
I remember in the past Aaron wanted to avoid casts, but maybe @miurahr can have a different position. I let to him the final decision but for the moment I saw not even a reaction to my comments.

Extend IEditor API to avoid cast to EditorController.

Signed-off-by: Hiroshi Miura <miurahr@linux.com>
Copy link

github-actions bot commented Mar 7, 2024

❌ Unit Tests, Quality checks, and Acceptance Tests failed.

Please look a Gradle Scan page for details:
https://gradle.com/s/5fvmgyfz4sao4

Signed-off-by: Hiroshi Miura <miurahr@linux.com>
Copy link

github-actions bot commented Mar 8, 2024

❌ Unit Tests, Quality checks, and Acceptance Tests failed.

Please look a Gradle Scan page for details:
https://gradle.com/s/s44bp44nv2ppw

Signed-off-by: Hiroshi Miura <miurahr@linux.com>
Copy link

github-actions bot commented Mar 8, 2024

❌ Unit Tests, Quality checks, and Acceptance Tests failed.

Please look a Gradle Scan page for details:
https://gradle.com/s/tysmlnn7dboyo

Signed-off-by: Hiroshi Miura <miurahr@linux.com>
@miurahr
Copy link
Member Author

miurahr commented Mar 20, 2024

The change does not provide a benefit over the task.

@miurahr miurahr closed this Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants