Bug 224209 - Transition SSE Syntax Highlighting to Presentation Reconciler
Summary: Transition SSE Syntax Highlighting to Presentation Reconciler
Status: CLOSED FIXED
Alias: None
Product: WTP Source Editing
Classification: WebTools
Component: wst.sse (show other bugs)
Version: 3.0   Edit
Hardware: PC All
: P3 enhancement (vote)
Target Milestone: 3.0 M7   Edit
Assignee: Nick Sandonato CLA
QA Contact: Nitin Dahyabhai CLA
URL:
Whiteboard:
Keywords: contributed
: 254454 (view as bug list)
Depends on:
Blocks: 136923 201690
  Show dependency tree
 
Reported: 2008-03-26 17:15 EDT by Nick Sandonato CLA
Modified: 2010-02-22 17:21 EST (History)
8 users (show)

See Also:
thatnitind: review+


Attachments
patch (63.28 KB, patch)
2008-04-11 16:52 EDT, Nick Sandonato CLA
no flags Details | Diff
patch (81.85 KB, patch)
2008-04-15 16:31 EDT, Nick Sandonato CLA
bjorn.freeman-benson: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Sandonato CLA 2008-03-26 17:15:37 EDT
Transition SSE Syntax Highlighting to Presentation Reconciler from using Line Style Events
Comment 1 Nick Sandonato CLA 2008-04-11 16:52:26 EDT
Created attachment 95757 [details]
patch

This patch should transition the XML, HTML, CSS, and JSP editors to using a reconciler to handle syntax highlighting. Thanks, Nitin, for some help with one of the edge cases.
Comment 2 Nitin Dahyabhai CLA 2008-04-11 18:07:16 EDT
Thanks for keeping at it, Nick (I see that the JSP providers are once again running).  I'll have to dig into why the platform MultipleHyperlinkPresenter shows some odd labels for us...and how willing I am to use the platform's internal BrowserInformationControl, just like JDT does (see bug 218482 for its API request, it should be more capable than the current HTML printer).
Comment 3 Nick Sandonato CLA 2008-04-15 16:31:52 EDT
Created attachment 96163 [details]
patch

This patch contains modifications to the Line Style Providers for LineStyleProviderForDTDSubset as well as LineStyleProviderForJSDT so that they may now contribute to the presentation reconciler.

Additionally, highlighting should now only affect the text of a region and not any trailing whitespace.
Comment 4 Roy Ganor CLA 2008-04-16 01:31:33 EDT
Nick, thank you for the preview. 

Can you write a few words about the patch? since it is a new mechanism I am interested in a summary that tells how to adopt it for other projects (those who depends on SSE). Do I have to make any changes in my LineStyleProvider? 

Last question is it going to be delivered into M7?
 
Comment 5 Nitin Dahyabhai CLA 2008-04-21 17:53:44 EDT
(In reply to comment #4)
> Nick, thank you for the preview. 
> 
> Can you write a few words about the patch? since it is a new mechanism I am
> interested in a summary that tells how to adopt it for other projects (those
> who depends on SSE). Do I have to make any changes in my LineStyleProvider? 
> 
> Last question is it going to be delivered into M7?

It's essentially getting SSE to use the presentation reconciler mechanism employed by the platform.  The StructuredDocumentEvents generated by our StructuredDocument are evaluated to decide how much damage has occurred from text changes, and the LineStyleProviders are used to fill in the appropriate repair for the presentation.  Under this new setup, a special Highlighter object will still be sent in the LineStyleProvider.init(IStructuredDocument, Highlighter) method that continues to provide access to the text viewer and implements the refreshDisplay(*) methods appropriately.  LineStyleProviderForPhp compiles cleanly, but I haven't had a chance to verify its function.  My guess is there's a good amount of presentation-related code that can be removed from it with this change, and yes, it's going into M7.
Comment 6 Nitin Dahyabhai CLA 2008-04-21 18:43:27 EDT
committed
Comment 7 David Williams CLA 2008-04-24 00:45:05 EDT
mass change to add 'contributed' keyword based on bugzilla query, please correct if that's not accurate (by marking patches as obsolete and removing the 'contributed' keyword. 
Comment 8 Nick Sandonato CLA 2008-04-29 16:20:20 EDT
Verified in I20080429053810. Closing.
Comment 9 Roy Ganor CLA 2008-05-18 03:39:09 EDT
Hi Nick, Nitin!

Any reason why StructuredTextViewerConfiguration#getPresentationReconciler() was finialized?

I do understand that we still use the Linestyle provider mechanism so we can limit the return value of this method to be of type StructuredPResentationReconciler.

it is very important for implemntors to have the ability to add functionality to the StructuredPresentationReconciler. 

Comment 10 Nitin Dahyabhai CLA 2008-05-18 14:40:51 EDT
No, no reason other than the method being "final" before.
Comment 11 Roy Ganor CLA 2008-05-18 15:24:48 EDT
Thank you for the quick reply, 

ok, I see your point but still I don't agree that we are now constraint to old conventions forever.

To be more specific, we (consider "we" as PDT + JSDT + JSP + XML) need to implement some new features (like SemanticHighlighting which is a super important feature for IDE of any language) that require to call methods of the presentation reconciler.

Another thing I can think of is to add the following method and variable to the StructuredPresentationReconciler so we can implement the :

	/** Last used document */
	private IDocument fLastDocument;

	/**
	 * Constructs a "repair description" for the given damage and returns
	 * this description as a text presentation.
	 * <p>
	 * NOTE: Should not be used if this reconciler is installed on a viewer.
	 * </p>
	 *
	 * @param damage the damage to be repaired
	 * @param document the document whose presentation must be repaired
	 * @return the presentation repair description as text presentation
	 */
	public TextPresentation createRepairDescription(IRegion damage, 
                                         IDocument document) {
		if (document != fLastDocument) {
			setDocumentToDamagers(document);
			setDocumentToRepairers(document);
			fLastDocument= document;
		}
		return createPresentation(damage, document);
	}

this will help us to implement the second coloring feature!
Comment 12 Nitin Dahyabhai CLA 2008-05-18 16:41:26 EDT
Semantic Highlighting is (almost) definitely something we want to do for the next release, but how would this method be used?  The presentation would still need to be applied to the text viewer, and the InternalListener (impelementing ITextInputListener) should already be ensuring that the correct document is set into the damagers and repairers.  The comment reading "hould not be used if this reconciler is installed on a viewer" is also rather curious.

The best thing to do would be to open a new feature request for semantic highlighting specifically so we can continue this discussion in a bug that's still open.  We're past the API cutoff point and even then there is some refactoring I'd like to see in the StructuredPresentationReconciler before we open it up some more.
Comment 13 Roy Ganor CLA 2008-05-18 17:10:07 EDT
Nitin,
IMHO this is a related topic, since we are working on the presentation reconciler we should not delay the work of semantic highlighing to next release.

As you guessed the presentation reconciler has opened a new world for us and we want to see it complete (and not a patched solution)

the provided code will help us call it in the same way JDT calls it as CompliationUnitEditor, you can refer to SemanticHighlightingPresenter for more details.

Comment 14 Nitin Dahyabhai CLA 2008-05-18 22:48:40 EDT
Unfortunately there's really no more time to work on it for this release.  The team's fully scheduled through the RC2 code cutoff to fix parser and encoding support issues, and we're hoping not to need to put anything into RC3.  Please open a RFE for semantic highlighting in next year's release.

And thanks for the pointers into the JDT solution.  Perhaps you can use an ITextPresentationListener?
Comment 15 Nitin Dahyabhai CLA 2010-02-22 17:21:14 EST
*** Bug 254454 has been marked as a duplicate of this bug. ***