Bugzilla – Full Text Bug Listing
| Summary: | Selection and IndexedRegion | ||
|---|---|---|---|
| Product: | [WebTools] WTP Source Editing | Reporter: | Dawid Pakula <zulus> |
| Component: | wst.sse | Assignee: | Nitin Dahyabhai <thatnitind> |
| Status: | RESOLVED FIXED | QA Contact: | Nitin Dahyabhai <thatnitind> |
| Severity: | enhancement | ||
| Priority: | P3 | ||
| Version: | 3.10 (Photon) | ||
| Target Milestone: | 3.11 | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: |
https://git.eclipse.org/r/125534 https://git.eclipse.org/r/125806 https://git.eclipse.org/c/sourceediting/webtools.sourceediting.git/commit/?id=536a273268def453bcff250c59f38f44616b34b9 https://bugs.eclipse.org/bugs/show_bug.cgi?id=537221 |
||
| Whiteboard: | |||
| Bug Depends on: | |||
| Bug Blocks: | 536109 | ||
|
Description
Dawid Pakula
Rather than run adapter framework, protected overridable "updateRangeIndication(ISelection selection)" method should be enough. What you think? The model already supports having an adapter factory for org.eclipse.wst.sse.ui.internal.editor.SelectionConvertor.class to solve this problem, but it's possible there's a use case communicating selection between parts that I'm overlooking. Let me know if so. Where it plugs in: http://git.eclipse.org/c/sourceediting/webtools.sourceediting.git/tree/core/bundles/org.eclipse.wst.sse.ui/src/org/eclipse/wst/sse/ui/StructuredTextEditor.java#n637 What plugs it in: http://git.eclipse.org/c/sourceediting/webtools.sourceediting.git/tree/xml/bundles/org.eclipse.wst.xml.ui/plugin.xml#n469 The version used for XML: http://git.eclipse.org/c/sourceediting/webtools.sourceediting.git/tree/xml/bundles/org.eclipse.wst.xml.ui/src/org/eclipse/wst/xml/ui/internal/editor/DOMSelectionConvertorFactory.java I already used SelectionFactory adapter to adapt ITextSelection into my model. After that "Show In" works fine with Project Explorer and Outliner. Also eclipse status line looks great. Unfortunately my model can't implement IndexedRegion (DLTK model) so highlighted range have always one line. See implementation of StructuredTextEditor#updateRangeIndication. If this method become protected, I'll be able to override this behavior. I mean SelectionConvertor. New Gerrit change created: https://git.eclipse.org/r/125534 Dawid, I'd rather expand on what the SelectionConvertor is responsible for, moving it to a public package for 3.11, rather than create another reason to have a subclass of StructuredTextEditor, which is something we generally want to avoid. There hasn't been a great history of communication between our projects, and we need to do better. If PDT creates a selection convertor that returns its desired IModelElements as part of the structured selection 1) what errors happen and 2) what functionality is missing that needs to be routed through a selection convertor? (In reply to Nitin Dahyabhai from comment #6) > Dawid, I'd rather expand on what the SelectionConvertor is responsible for, > moving it to a public package for 3.11, rather than create another reason to > have a subclass of StructuredTextEditor, which is something we generally > want to avoid. There hasn't been a great history of communication between > our projects, and we need to do better. This is what I'm trying to change. Rather than create another proxy/workaround/fork I prefer fix/improve source, too many code to maintain later ;) See for example bug 438661. > If PDT creates a selection convertor that returns its desired IModelElements > as part of the structured selection 1) what errors happen and 2) what > functionality is missing that needs to be routed through a selection > convertor? If I transform ITextSelection to IStructuredSelection with IType/IMethod/IField SSE is not able to correctly highlight source range (ISourceViewer#setRangeIndication) because my IStructuredSelection elements aren't IndexedRegion. If StructuredTextEditor#updateRangeIndication logic will be somehow part of SelectionConvertor my problem will be resolved in clean way. Currently PDT after each SSE post selection have to perform re-select and reset range indication. This introduces lot of problems (flashy range indication, double event processing, problems with "show in", mylyn context reinjection etc...). Would a new method that accepted the ISelection and returned the range calculated to be indicated with an org.eclipse.jface.text.Region suffice? It would be called both after using the SelectionConvertor to change the PostSelection event content into a StructuredTextSelection as well as regular selection from the Outline page? On double-click in the outline, it would do the same but then also call selectAndReveal as StructuredTextEditor.OutlinePageListener.doubleClick(DoubleClickEvent) already does. Perfect! Should I prepare a patch or you do you have an idea? (In reply to Dawid Pakula from comment #10) I expect to have refactoring, and the accompanying version and code changes in our other bundles, in the master branch within the next few days. At the latest, it'll be in time for the M1 candidate build, this Thursday. Committed at http://git.eclipse.org/c/sourceediting/webtools.sourceediting.git/commit/?id=4f827704b84af58048729f4a4de7fb313111f0d8 . New Gerrit change created: https://git.eclipse.org/r/125806 Gerrit change https://git.eclipse.org/r/125806 was merged to [master]. Commit: http://git.eclipse.org/c/sourceediting/webtools.sourceediting.git/commit/?id=536a273268def453bcff250c59f38f44616b34b9 Works perfectly! I'm waiting for I-Build now ;) Our integration: https://git.eclipse.org/r/#/c/124877/ I'm marking as resolved. If you are interested I'll be happy if PDT could switch to base StructuredTextEditor or even GenericTextEditor with SSE flavors. It have now 4200 lines :/ One more thing. SSE be design run selectAndReveal with same start/length as highlighting. Could be this also pluggable? We currently have own outline selection listener, and like JDT set range indicator for IMember but select for IMember.name. Should I open another bug? (In reply to Dawid Pakula from comment #17) > One more thing. SSE be design run selectAndReveal with same start/length as > highlighting. Could be this also pluggable? > > We currently have own outline selection listener, and like JDT set range > indicator for IMember but select for IMember.name. Should I open another bug? Yes, please use a different bug for M2. I'd like to keep the behavior--the selecting itself--separate from the determination of what is to be selected. That determination could be a new method on the converter, once it's determined that the outline's selection is of a single object. |