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

[StickyScrolling] Introduce enhancement point #2756

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Christopher-Hermann
Copy link
Contributor

In order to implement editor/language specific sticky lines provider, a new extension point is introduced.

See issues:

The idea is that the package of the extension point is internal in the first place.

  1. We from SAP will provide a extension for the SAP language ABAP.
  2. In [Help Needed] Implement provider for sticky scrolling in JAVA/JDT eclipse-jdt/eclipse.jdt.ui#1851 we provide a implementation for JDT.

If both implementation works as expected, the API is most probably stable and we can change to package of the extension point to public usage.

@@ -15,7 +15,8 @@ Export-Package:
org.eclipse.ui.internal.editors.text.codemining.annotation;x-internal:=true,
org.eclipse.ui.internal.texteditor;x-internal:=true,
org.eclipse.ui.internal.texteditor.stickyscroll;x-internal:=true,
org.eclipse.ui.texteditor
org.eclipse.ui.texteditor,
org.eclipse.ui.texteditor.stickyscroll;x-internal:=true
Copy link
Contributor

Choose a reason for hiding this comment

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

so we would remove the "x-internal:=true" part to make this "real" API once we are confident enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that would be the idea. But I'm not sure if this is the right approach or if this is a good idea.

Copy link
Contributor

github-actions bot commented Jan 24, 2025

Test Results

0 files   -  1 821  0 suites   - 1 821   0s ⏱️ - 1h 35m 36s
0 tests  -  7 719  0 ✅  -  7 489  0 💤  - 228  0 ❌  - 2 
0 runs   - 24 318  0 ✅  - 23 567  0 💤  - 749  0 ❌  - 2 

Results for commit b5eec35. ± Comparison against base commit 55481d3.

♻️ This comment has been updated with latest results.

@mickaelistria
Copy link
Contributor

Can this share a lot of code with the folding range?
Instead of a new dedicated provider API, could it be implemented through reconciler and annotations (just like Folding) ?

@Christopher-Hermann
Copy link
Contributor Author

Can this share a lot of code with the folding range? Instead of a new dedicated provider API, could it be implemented through reconciler and annotations (just like Folding) ?

investigated the possibility, and it appears that annotations can likely be reused for sticky scrolling functionality. We could attach a depth attribute to each line that is a potential sticky line candidate. The sticky line handler could then determine which line should be attached based on the scroll index and the depth specified in the annotation.

However, utilizing annotations may reduce flexibility, which can be beneficial in certain scenarios. For example, if you want to apply different styles to the sticky lines or even add text that is not originally part of the editor, relying solely on annotations could be limiting.
See this example where I added a custom text with custom styling:
Bildschirmfoto 2025-01-29 um 14 54 29

@Christopher-Hermann Christopher-Hermann force-pushed the stickyScrollingEnhancementPoint branch from 3c397ec to 907344a Compare January 31, 2025 09:59
In order to implement editor/language specific sticky lines provider, a new extension point is introduced.
@Christopher-Hermann Christopher-Hermann force-pushed the stickyScrollingEnhancementPoint branch from 907344a to 536aa91 Compare January 31, 2025 10:00
@danthe1st
Copy link

danthe1st commented Jan 31, 2025

However, utilizing annotations may reduce flexibility

I guess it would be possible to use folding annotations by default (if present, otherwise use indentation as done currently) and still allow overriding the sticky scrolling functionality?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants