[StickyScrolling] Introduce enhancement point
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.
- We from SAP will provide a extension for the SAP language ABAP.
- 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.
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.
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) ?
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:
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?
@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?
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
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?
@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...