eclipse.platform.ui icon indicating copy to clipboard operation
eclipse.platform.ui copied to clipboard

[StickyScrolling] Introduce enhancement point

Open Christopher-Hermann opened this issue 11 months ago • 6 comments

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

See issues:

  • #2338
  • #2128
  • #1950

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 https://github.com/eclipse-jdt/eclipse.jdt.ui/pull/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.

Christopher-Hermann avatar Jan 24 '25 14:01 Christopher-Hermann

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 6bf66393. ± Comparison against base commit 0368ee56.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Jan 24 '25 14:01 github-actions[bot]

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) ?

mickaelistria avatar Jan 24 '25 17:01 mickaelistria

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 avatar Jan 29 '25 13:01 Christopher-Hermann

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?

danthe1st avatar Jan 31 '25 11:01 danthe1st

@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?

BeckerWdf avatar Feb 28 '25 09:02 BeckerWdf

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: https://github.com/eclipse-jdt/eclipse.jdt.ui/pull/1851

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

Christopher-Hermann avatar Apr 16 '25 07:04 Christopher-Hermann

testCreateAndRunWorkbench (RcpTestSuite PlatformUITest) is unrelated and documented in https://github.com/eclipse-platform/eclipse.platform.ui/issues/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?

BeckerWdf avatar May 12 '25 06:05 BeckerWdf

@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 avatar May 12 '25 06:05 Christopher-Hermann