Skip to content

[StickyScrolling] Introduce enhancement point #2756

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

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.

Copy link
Contributor

github-actions bot commented Jan 24, 2025

Test Results

 1 824 files  ± 0   1 824 suites  ±0   1h 34m 45s ⏱️ -58s
 7 925 tests + 7   7 697 ✅ + 7  228 💤 ±0  0 ❌ ±0 
23 862 runs  +21  23 114 ✅ +21  748 💤 ±0  0 ❌ ±0 

Results for commit 6bf6639. ± Comparison against base commit 0368ee5.

♻️ 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 2 times, most recently 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?

@BeckerWdf
Copy link
Contributor

@mickaelistria: We would like to finish this early when master opens up again.
Can you pls. again have a look and thought about @Christopher-Hermann's comments?

@Christopher-Hermann Christopher-Hermann force-pushed the stickyScrollingEnhancementPoint branch from b5eec35 to e5662e3 Compare April 16, 2025 07:18
@Christopher-Hermann
Copy link
Contributor Author

No updates or comments for a long time. So we will merge this change if there are no concerns in the next days/weeks as there is now also a JDT implementation: eclipse-jdt/eclipse.jdt.ui#1851

Need to check if a isEnabled() method needs to be added

@Christopher-Hermann Christopher-Hermann force-pushed the stickyScrollingEnhancementPoint branch 6 times, most recently from a35a6d8 to d293f16 Compare May 11, 2025 08:35
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 d293f16 to 2cb7045 Compare May 11, 2025 09:21
@BeckerWdf
Copy link
Contributor

testCreateAndRunWorkbench (RcpTestSuite PlatformUITest) is unrelated and documented in #1517

Regarding: testCancellingWhenRunning (org.eclipse.jface.text.tests.reconciler.FastAbstractReconcilerTest)
I could not find an issue for that. Assertion message is: "reconciler never ran in 5 seconds".
@Christopher-Hermann: Could this be related to your change?

@Christopher-Hermann
Copy link
Contributor Author

@Christopher-Hermann: Could this be related to your change?

I guess not, but not 100% sure.
The test was never failing before when I introduced the sticky scrolling. In this change, only the extension point was introduced. The sticky scrolling behavior it self was not changed.

I will try to start the build again...

@Christopher-Hermann Christopher-Hermann force-pushed the stickyScrollingEnhancementPoint branch from 2cb7045 to 6bf6639 Compare May 12, 2025 06:46
@Christopher-Hermann Christopher-Hermann merged commit bfeb6c6 into eclipse-platform:master May 13, 2025
18 checks passed
@Christopher-Hermann Christopher-Hermann deleted the stickyScrollingEnhancementPoint branch May 13, 2025 06:36
@BeckerWdf BeckerWdf added this to the 4.36 M3 milestone May 13, 2025
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