Extension for quick search dialog text viewers. Fixes #2010

[RedeemerSK]

What it does

Introduces new extension point textViewers of the org.eclipse.text.quicksearch plugin that makes it possible to plug-in different text viewers with different text styles/decorations to present quick search matches.

New extension point API is defined by interfaces but to make it easier for implementors to create extensions that provide common & expected text viewer behavior & visuals, easy-to-extend abstract classes that implement the common aspects are also provided.

API interfaces are modeled so that extensions are able to

• create new text viewers under parent widget provided by opened quicksearch dialog (supporting multiple dialogs)

• focus on the specific match (selected amongst quicksearch results) within content of its containing file, meaning:

  • limiting presented content of the input document (reachable by scrolling) *
  • presenting specific part of the input document to center the match (vertical scrolling) *
  • highlighting line of the match
  • making sure actual match is visible (horizontal scrolling)
  • positioning caret to the start of the match

• highlight all matches in the file *

Abstract classes implement common code that is responsible for:

• configuring created SourceViewer implementation to:
  • have vertical ruler column for displaying line numbers & highlighting selected match
  • use proper background color
  • use proper font & text colors
  • react to changes of preferences for used fonts & colors
• highlighting selected match line by:
  • highlighting line number in the line numbers vertical ruler column
  • highlighting whole line with current line highlighting color
• highlighting selected match by means of special text style background color *
  • also considering text styles applied from any presentation reconcilers
  • with workaround for losing styles from reconcilers when focusing on other match in same file

(* already done by current version of quicksearch plugin)

This PR also contains implementation of new extension provided by org.eclipse.ui.genericeditor plugin that is capable of presenting quicksearch matches inside same type of files that GenericEditor supports. Also fallback implementation of new extension capable of presenting any text file.

Quicksearch dialog also allows to manually change text viewer (extension) used to display matches inside specific file - selection is remembered until restart of workbench.

Design of the API + implementation of the abstract classes is the result of implementing new extension contributions in org.eclipse.ui.genericeditor plugin, org.eclipse.jdt.ui plugin (will be separate PR) and DBeaver project chosen for unusual/unreasonable approach to re-usable source viewers classes (actually misusing full IEditor implementation for them).

How to test

Open Quick Search dialog (default: CTRL + SHIFT + ALT + L) and confirm bottom area presenting parts of files where matches are located displays matches in a text (source) viewer that does text styling & decorations based on the file content + extension used. Focusing on match inside the file content should behave like it used to = displays match vertically centered, keeps it centered on resizing the dialog or area, content limited to same 'context' (lines before & after match within source file).

Known bugs

TBD

Author checklist

• I have thoroughly tested my changes
• The change is following the coding conventions
• I have signed the Eclipse Contributor Agreement (ECA)

[laeubi]

@RedeemerSK I think it would be good to include some screenshots / Video in the PR description that shows the new feature.

Also this is a rather huge contribution, do you think individual smaller parts can maybe merged independently?

[RedeemerSK]

It's not a small change, specifically in the impact on Eclipse platform (new extension point), so I'm creating the PR now even if eg. test are completely missing (will add in parallel), just to kick off the discussion.

[RedeemerSK]

I think it would be good to include some screenshots / Video in the PR description that shows the new feature.

Done

Also this is a rather huge contribution, do you think individual smaller parts can maybe merged independently?

Atm I can't see any reasonable way to split it.
In theory I could make 1st part with just the new extension API (interfaces) and then the other one with "helper" abstract classes. But merging 1st part like this would leave quicksearch non-working until 2nd part would be merged.

[RedeemerSK]

ABOVE: just the API (+ re-usable helper classes) of the extension point

BELOW: including classes for 2 implementations of new extension point included in PR : default/fallback viewer & generic editor viewer

[RedeemerSK]

I think it would be good to include some screenshots / Video in the PR description that shows the new feature.

Done

Also this is a rather huge contribution, do you think individual smaller parts can maybe merged independently?

Atm I can't see any reasonable way to split it. In theory I could make 1st part with just the new extension API (interfaces) and then the other one with "helper" abstract classes. But merging 1st part like this would leave quicksearch non-working until 2nd part would be merged.

Update on possibility of splitting:
This is doable, 1st PR just extension point API & essential helper classes + default implementation, 2nd PR contribution by genericeditor plugin with additional required common helper class.

[mickaelistria]

I doubt another extension point for that is likely to be well adopted. However what you're suggesting here is more or less a clone of the org.eclipse.compare.contentViewers extension-point which basically covers the exact same story and is already well adopted.
So I strongly recommend consuming the existing extension point (and related APIs) to simplify your code, and ease adoption.

[RedeemerSK]

I doubt another extension point for that is likely to be well adopted. However what you're suggesting here is more or less a clone of the org.eclipse.compare.contentViewers extension-point which basically covers the exact same story and is already well adopted. So I strongly recommend consuming the existing extension point (and related APIs) to simplify your code, and ease adoption.

At the beginning I was evaluation the possibility to re-use exactly that extension point (and some others as well) but realized it would do more harm than good. This is my reasoning:

• often the implementations of the compare extension do not provide access to actual viewer that displays the content, just dummy/facade viewer wrapping it + forwarding calls to just few selected methods (eg. setInput()). Therefore it would not be possible to focus on specific match or highlight matches in the content unless extension implementations are updated to support that. See just (subjectively) the most important contributions by org.eclipse.compare (I assume this is default/fallback), org.eclipse.jdt.ui or org.eclipse.ui.genericeditor +1. Not being able to focus on a match is I would say a no-go, that basically makes quicksearch unusable.
• contribution by org.eclipse.jdt.ui plugin does not perform full semantic highlighting, just the basic syntax coloring. I think this may have been conscious decision back then since the source of presented content is not expected to always come from real file on filesystem. Therefore such integration to quicksearch would look half-baked.
• from architecture point of view it IMHO is misusing unrelated stuff and feels like a not-so-pretty hack: piggybacking in quicksearch on something meant for compare functionality and having to accept the limitations it brings ... plus just praying that using existing extensions in new context for which they were not created would not produce strange behavior or exceptions.
• new extension point does not necessarily need huge adoption. Currently we have just plain text without any syntax highlighting. I have implementations of new extension point ready bringing not only that (as fallback), but also full java syntax (& semantic) coloring and full syntax coloring for everything generic editor (textmate) supports. Only the latter two is IMHO already a huge improvement over the current state.

Are the above points enough valid reasons to reconsider the rejection of new extension point idea @mickaelistria ?
Without new extension point I currently don't see a way to improve quicksearch to the level I would expect it to be - showing me the same content I would see having the file opened in the editor.

[mickaelistria]

often the implementations of the compare extension do not provide access to actual viewer that displays the content, just dummy/facade viewer wrapping it + forwarding calls to just few selected methods (eg. setInput()). Therefore it would not be possible to focus on specific match or highlight matches in the content unless extension implementations are updated to support that.

The returned viewer has "get/setSelection", which by API contract of the viewer is expected to work the viewer. If this doesn't work, then I suggest reporting it as an issue to those contributors and fixing it there.

does not perform full semantic highlighting, just the basic syntax coloring.

This is IMO not so related and can/should be fixed upstream too then (note that semantic hightlight can be an expensive operation that might not fit in a quick view anyway).

piggybacking in quicksearch on something meant for compare functionality and having to accept the limitations it brings ...

This comes from the "compare" bundle, but if you look at the description of that particular extension point and API, it seems to me like a perfect match for what you intend here. So yes, the extension might have been better located in another bundle, yet it still seems to match this case well.

new extension point does not necessarily need huge adoption.

I disagree about it in general. While it might be sometimes true, for user-facing extensions, such a new extension point with only 1 or 2 extension would give the impression that the feature only works partly. There would be disappointment for users because of the lack of completeness, and it's not likely that all language supports in the wild (CDT, PyDev, and all DSLs created in various RCP) do adopt this extension point.
On the other hand, the exisiting contentViewers extension point is long established, long adopted and probably already very well covered by all languages that have needed it. Basically, using it will give extra value to many plugins without adoption effort, while creating a new extension point would create a somewhat negative value ("why isn't language X working as expected in the quicksearch?") and resolving it would require unlikely development efforts from plugin editors.

While there are some limitations as you mentioned above about particular extensions not behaving at best; in Platform we have to think about how to unleash as much value as possible to adopters for the least possible development/maintenance effort (ideally, none at all) on their side. New extension points are welcome when there is nothing existing, but here we would create more value to more adopters by just using what's already existing.

[RedeemerSK]

@mickaelistria Would it be possible to start with my original #2644 so that at least that makes it into 4.36 release ?
For the re-work to use compare's contentViewers extensions I need to find both time and motivation and so it may miss 4.36 dev phase deadline.
Thanks

[RedeemerSK]

After looking at the contentViewers extension once again I arrived at the same conclusion : I don't see a way to use it for quicksearch use-case without either making necessary adjustments to all extensions or accepting that quicksearch would become broken when it uses extension that was not updated.

The returned viewer has "get/setSelection", which by API contract of the viewer is expected to work the viewer. If this doesn't work, then I suggest reporting it as an issue to those contributors and fixing it there.

Extension point defines that contributions return instance of Viewer. Viewer class declares 15 public methods,. Yet, out of the 4 plugins that I checked so far (bundled in Eclipse PDE distribution), none of them returns instances that fulfill this public API and even that I believe would be not enough to do necessary adjustments to content presentation. All of them return "wrapping" viewers.

None of them forwards mentioned setSelection() calls to underling/wrapped viewers. 3 of them, JavaTextViewer, GenericEditorViewer and GradleViewer even explicitly override the method to do nothing. Also with just setSelection() I doubt you can achieve centering match in the viewer's area with the same visual result as what we have today. Quicksearch currently uses TextViewer.revelaRange() to have the selected match centered in the viewer's widget.

Not only that, but 3 of them do not allow easy & reliable way to

• a) either get hold of TextWidget so that match highlighting (StyleRange[]) can be applied manually
• b) or deliver TextPresentation to updatePresentation() method of wrapped TextViewer (if it's of that Viewer sub-class)

one of which is necessary to highlight matches in the content. I assume we don't want to sacrifice matches highlighting.

So none of these common plugins can be used for quicksearch use-case without modifications, even if we would resort to some
hacks like traversing widget hierarchy (looks like all extensions implement Viewer.getControl(), but what they return also differs) and hoping to find a viewer we need. It's only understandable, the extensions were developed and tested for other use-case that actually requires narrower interface - just creating viewer under passed parent widget, returning it's control (used for just registering widget listeners it seems) and being able to set input.

I would say it's reasonable to assume other extensions would also be implemented in a way preventing their use without modifications.

@mickaelistria if you're convinced we could achieve re-use of contentViewers in quicksearch, please could you elaborate on how you think things should be updated so that we end up with at least what we have now ?

Thanks