Community
Participate
Working Groups
User-Agent: Mozilla/5.0 (X11; U; Linux i686; da; rv:1.9.0.14) Gecko/2009110300 Gentoo Firefox/3.0.14 Build Identifier: 20090930-1017 Eclipse for RCP/Plug-in Developers For my editor I was experimenting with dynamic update of my outlinePage. To my surprise the outlinePage did not get updated as I would expect. After some debugging and reading of many lines of codes within the org.eclipse.jface.text plugin, I finally discovered that the PresentationReconciler class might be at fault. I noticed that my partitioning scanner functioned as expected and also saw the correct document range that needed to be repaired in the getCoverage method of the DocumentPartitioningChangedEvent. At the start of the PresentationReconciler getDamage method I added the following lines of code, and after that my dynamic update of the outline page worked as I would expect. In package org.eclipse.jface.text.presentation. private IRegion getDamage(DocumentEvent e, boolean optimize) { if ( optimize && fDocumentPartitioningChanged && fChangedDocumentPartitions!=null) return new Region(fChangedDocumentPartitions.getOffset() ,fChangedDocumentPartitions.getLength()); Can someone with more experience with the PresentationReconciler class comment on this? What is the procedure to have this change accepted by the JFace project ? thanks for your help Henrik Reproducible: Always
The presentation reconciler has nothing to do with updating the model and hence updating the outline. If you wired it to do this then you are abusing that class. The org.eclipse.jface.text.reconciler.IReconciler (which is not implemented by the presentation reconciler) is responsible to update the model. If you still think the PresentationReconciler has a bug then please provide a test case that shows the problem and reopen the bug again.
(In reply to comment #1) > The presentation reconciler has nothing to do with updating the model and hence > updating the outline. If you wired it to do this then you are abusing that > class. The org.eclipse.jface.text.reconciler.IReconciler (which is not > implemented by the presentation reconciler) is responsible to update the model. agreed, and I dont do that. Not intentionally anyway. But I want to prevent rescanning of the source, and have the scanners prepare the data necessary for the outline contents. The content is needed for correct parsing of the source code anyway. > > If you still think the PresentationReconciler has a bug then please provide a > test case that shows the problem and reopen the bug again. I am using the FastPartitioningScanner and the DefaultDamagerRepairer. Here is why I think its a bug. In method doFireDocumentChanged2 (AbstractDocument.Java:731), for the fireDocumentPartitioningChanged is invoked. Adding a PartitioningChangedListner, and printing the getCoverage of the documentpartitionchanged event gives me the full region that needs the be rescanned. As can be seen below "getCoverage=offset: 62, length: 25", and can be seen my statement for the document content "pl1_statmnt" scanner gets invoked for each of the of the relevant regions for the range of 62 to 87. This is the trace output for my scanners setRange method: = PLIStmtScanner setRange foffs=62, flen=20 = PLIStmtScanner setRange foffs=83, flen=4 Now what happens next in doFireDocumentChanged2 (AbstractDocument.Java:739), is the TextViewer documentChanged invokes the updateTextListners (TextViewer.java:2806), this will invoke the necessary scanners for the whole region. The region is calculated by the PresentationReconciler getDamage method, which led me to the fix. Now what happens in getDamage without my patch, partition range is still correct, but the Region passed to the updateTextListners methos is wrong. The region will be from the document event (';') and until the end of the partition This is the trace output for my scanners setRange method: = PLIStmtScanner setRange foffs=84, flen=3 As can be seen the statement scanner only gets invoked for the last partition, but but for the whole damaged region. I have added some traces. Text before: %include alstxY09; Text after: %include alst;xY09; As can be seen I insert a ';' in the text, and since the includes are on my outline page, this change needs to be reflected as well. Trace output with my fix: = PLIPartitionScanner setPartialRange offset=67, length=45, partitionOffset=62, contentType=__pli_statmnt = PLIPartitionScanner setPartialRange , document length=112, scanOffset=62, scanLength=50, text > %include alst;xY09; end mypgm; < = PLIPartitionScanner nextToken, isOther, getData=__pli_statmnt offset=62, end=82, length=20 = PLIPartitionScanner nextToken leaving ... = PLIPartitionScanner nextToken, isOther, getData=__pli_eofstmt offset=82, end=83, length=1 = PLIPartitionScanner nextToken leaving ... = PLIPartitionScanner nextToken, isOther, getData=__pli_statmnt offset=83, end=87, length=4 = PLIPartitionScanner nextToken leaving ... = PLIPartitionScanner nextToken, isOther, getData=__pli_eofstmt offset=87, end=88, length=1 = PLIPartitionScanner nextToken leaving ... <b> = PLIDocumentProvider documentPartitioningChanged event getCoverage=offset: 62, length: 25 = __dftl_partitioning, getChangedRegionoffset: 62, length: 25 </b> = PLIStmtScanner setRange foffs=62, flen=20 = PLIStmtScanner setRange token 33, >Whitespace< offset=62, end=69, length=7 = PLIStmtScanner setRange token 34, >%< offset=69, end=70, length=1 = PLIStmtScanner setRange token 37, >%INCLUDE< offset=70, end=77, length=7 = PLIStmtScanner setRange token 33, >Whitespace< offset=77, end=78, length=1 = PLIStmtScanner setRange token 32, >Name< offset=78, end=82, length=4, alst = PLIDocumentProvider addOutlineInfo info=Include, partitionOffset=62, token=Token=32 (Name), pos=16, end=20>alst< updating segment = PLIStmtScanner setRange foffs=83, flen=4 = PLIStmtScanner setRange token 32, >Name< offset=83, end=87, length=4, xY09 Trace outout without my fix: = PLIPartitionScanner setPartialRange offset=67, length=45, partitionOffset=62, contentType=__pli_statmnt = PLIPartitionScanner setPartialRange , document length=112, scanOffset=62, scanLength=50, text > %include alstx;Y09; end mypgm; < = PLIPartitionScanner nextToken, isOther, getData=__pli_statmnt offset=62, end=83, length=21 = PLIPartitionScanner nextToken leaving ... = PLIPartitionScanner nextToken, isOther, getData=__pli_eofstmt offset=83, end=84, length=1 = PLIPartitionScanner nextToken leaving ... = PLIPartitionScanner nextToken, isOther, getData=__pli_statmnt offset=84, end=87, length=3 = PLIPartitionScanner nextToken leaving ... = PLIPartitionScanner nextToken, isOther, getData=__pli_eofstmt offset=87, end=88, length=1 = PLIPartitionScanner nextToken leaving ... = PLIDocumentProvider documentPartitioningChanged event getCoverage=offset: 62, length: 25 = __dftl_partitioning, getChangedRegionoffset: 62, length: 25 + NonRuleBasedDamager getDamageRegion partition=__pli_eofstmt - offset: 83, length: 1, documentPartitionChanged=true, event=offset: 83, length: 0, timestamp: 2 text:>;< + NonRuleBasedDamager getDamageRegion partition=__pli_eofstmt - offset: 87, length: 1, documentPartitionChanged=true, event=offset: 83, length: 0, timestamp: 2 text:>;< = PLIStmtScanner setRange foffs=84, flen=3 = PLIStmtScanner setRange token 32, >Name< offset=84, end=87, length=3, Y09
Henrik, I'm sorry but we simply don't have time to investigate a problem that's not surfacing somewhere. If you want us too look at it then please provide a JUnit test case or demo plug-in. If don't want to do this then please close the bug again.
(In reply to comment #3) > Henrik, I'm sorry but we simply don't have time to investigate a problem that's > not surfacing somewhere. If you want us too look at it then please provide a > JUnit test case or demo plug-in. If don't want to do this then please close the > bug again. Dani, btw thanks for your prompt replies. Here is a summary why it is a bug: The partitioning changed event reports a region offset: 62, length 25. The range has 4 partitions: __pli_statmnt offset=62, length=20 __pli_eofstmt offset=82, length=1 __pli_statmnt offset=83, length=4 __pli_eofstmt offset=87, length=1 With my fix, the setRange method for the scanner for associated with "__pli_statmnt" gets invoked twice: = PLIStmtScanner setRange foffs=62, flen=20 = PLIStmtScanner setRange foffs=83, flen=4 this is the expected behaviour. Without my fix, the setRange method for the scanner for associated with "__pli_statmnt" gets invoked only once: = PLIStmtScanner setRange foffs=83, flen=4 I will add my plugin and fix as attachments in a minute ...
Created attachment 152153 [details] Fixed PresentationReconciler getDamage method In the package Added the following lines to the Class PresentationReconciler method getDamage private IRegion getDamage(DocumentEvent e, boolean optimize) { if ( optimize && fDocumentPartitioningChanged && fChangedDocumentPartitions!=null) return new Region(fChangedDocumentPartitions.getOffset() ,fChangedDocumentPartitions.getLength());
Created attachment 152154 [details] Editor plugin The editor works on files ending with *.pl1.
Step to reproduce the error: Copy the EditorPlugin to the eclipse/dropin directory. Create a file test.pl1, with the following content:
Step to reproduce the error: Copy the EditorPlugin to the eclipse/dropins directory. Create a file test.pl1, with the following content: mypgm: proc(a) ; dcl a var char(*); put skip list(a); %include alstxY09; end mypgm; now open the outline view. You should now see the name of the include "alstxY09". Next add a ';' in the middle of the include name %include alst;xY09; The outline view is no longer reflecting the include name. The fix the error. Add my fixed version of the PresentationReconciler class in the org.eclipse.jface.text plugin to the eclipse/dropins and retry the change to the %include as described above. I will attach the changed getDamage method as well.
Created attachment 152158 [details] replacement for getDamage methon in PresentationReconciler the code probably got whitespace damaged. This should be the only difference if ( optimize && fDocumentPartitioningChanged && fChangedDocumentPartitions!=null) return new Region(fChangedDocumentPartitions.getOffset() ,fChangedDocumentPartitions.getLength());
(In reply to comment #3) > If you want us too look at it then please provide a > JUnit test case or demo plug-in. Do you have a sample JUnit test for an editor plugin ?
>Do you have a sample JUnit test for an editor plugin ? There are no simple tests but in this case I guess it would be enough to write a JUnit test case that involves a text view, a document and the presentation reconciler. Can you please attached the editor plug-in as source so that I can verify that the APIs are used as intended. If you want to suggest a change then please attach the change as patch (Team > Create Patch...).
Created attachment 152427 [details] Editor Plugin with Source The editor works in files ending with *.pl1 Features: Outline page for %include Preference page for debugging options.
Created attachment 152432 [details] diff -rupw old.java new.java Patch generated with diff command
Henrik, the attached source does not compile (e.g. 'Debug' code is missing.) and I don't have time to fix that. FYI, to make a patch you need to 1. checkout the source from CVS 2. make the changes directly in the code 3. Team > Create Patch...
(In reply to comment #14) > Henrik, the attached source does not compile (e.g. 'Debug' code is missing.) > and I don't have time to fix that. just realised the export was incomplete. I will upload new version asap.
Created attachment 152502 [details] Editor Plugin with Source Editor works on *.pl1 files
Created attachment 152508 [details] patch of method getDamage Inserts the following lines at the very beginning of getDamage if ( optimize && fDocumentPartitioningChanged && fChangedDocumentPartitions!=null) return new Region(fChangedDocumentPartitions.getOffset() ,fChangedDocumentPartitions.getLength());
PLIPartitionScannerFlex is still missing. I give you one last try then I will stop looking at this bug.
(In reply to comment #18) > PLIPartitionScannerFlex is still missing. I give you one last try then I will > stop looking at this bug. sorry for this, but the export function of my plugin for some reason (a bug?) did not take source folders with linked content. The reason the files are linked is they are generated by the JFlex utility in another project. I added the two missing files manually to the src-jar file, and will upload a new version shortly. thanks for your patience.
Created attachment 152648 [details] Editor Plugin with Source
A few implementation notes: The editor is still in the very early stages and is based on the XML sample editor with some significant changes. For example the tokens are scanned by a JFlex scanner, hence the missing files in the first plugin. Also I am writing a PL/I frontend for gcc, so I will reuse much of my experiences from scanning and parsing PL/I source code in that project. (http://pl1gcc.sourceforge.net) Currently the editor just supports very limited coloring and an outline page of the %includes in the program. I got a bit taken by having the outline page being dynamically updated as I type or change the %include name. To achieve this I decided to let the document provider (PLIDocumentProvider) have a datastructure with the data needed for the view, and let the view-content provider access the information from the document provider. This further has the advantage that the document is only scanned once. There are obviously other solutions as well.
(In reply to comment #3) > JUnit test case ... I am trying to create a JUnit case, but have some troubles setting up the workspace. I have checked out version R3_5_1 of
> JUnit test case ... I am trying to create a JUnit case, but have some troubles setting up the workspace. I have checked out version R3_5_1 of org.eclipse.jface.text org.eclipse.jface.text.tests org.eclipse.test org.eclipse.text.tests but when launcing the JFaceTextTestSuite.launch, I receive this error: Failed to invoke suite(): java.lang.SecurityException: class "org.eclipse.jface.text.ITextViewer"'s signer information does not match signer information of other classes in the same package java.lang.reflect.InvocationTargetException what does that mean ? thanks for your help
>Failed to invoke suite(): java.lang.SecurityException: class >"org.eclipse.jface.text.ITextViewer"'s signer information does not match signer I've never seen this. Very old VMs had this issue and it can happen if you have (wrongly) signed JARs. You shouldn't get it if you 1. download latest build (e.g. 3.6 M3) 2. start fresh workspace 3. checkout the test bundles from CVS HEAD
I've looked at your example: there is no bug: it returns the correct damage for your example in comment 8, namely the full partition where the change happens (offset: 71, length: 14): this is the new partition that got created by typing a ';'. The presentation reconciler calls the damager for the affected partition and the partitioning is always done as the first thing when a change in the document happens. You have two choices: - (preferred) don't create a new partition with same type like the previous one at this point (example) instead make sure the partitioner creates a partition with "inc;lude...." - provide your own damager that damages multiple partitions when needed
(In reply to comment #25) > I've looked at your example: there is no bug: it returns the correct damage Thanks taking the time to looking at the example. Maybe I am misunderstanding the relation between the coverage reported by the documentpartitionchanged event. Is it not true that the whole range in the coverage needs to be handled by the repairer/damager/reconciler ? >- provide your own damager that damages multiple partitions when needed but the createPresentation method in PresentationReconciler already supports this: ITypedRegion[] partitioning= TextUtilities.computePartitioning(document, getDocumentPartitioning(), damage.getOffset(), damage.getLength(), false); for (int i= 0; i < partitioning.length; i++) { ITypedRegion r= partitioning[i]; IPresentationRepairer repairer=getRepairer(r.getType()); if (repairer != null) repairer.createPresentation(presentation, r); } I have prepared a skeleton for a JUnit test that I will attach. Anyway, I think I am hearing you, and I will provide my own reconciler.
Created attachment 153129 [details] patch of method getDamage This patch adds an interface to enable/disable the suggested bug fix. public void enableBugfix295059 public void disableBugfix295059 public boolean isBugfix291152 The default is to enable the fix.
Created attachment 153131 [details] Java Main program to test the PresentationReconciler The main program will produce invoke the PresentationReconciler with the bugfix enabled and disabled, and will collect the scanned ranges and compare the ranges to the partition ranges of the document. It will show that the partition starting at offset 15 with length 15 is only scanned with the bug fix enabled. PresentationReconciler Bugfix 295059 DISABLED = MyDocumentPartitionListner documentPartitioningChanged event.getCoverage=offset: 15, length: 29 PresentationReconciler Bugfix 295059 ENABLED = MyDocumentPartitionListner documentPartitioningChanged event.getCoverage=offset: 15, length: 29 [0] __pli_comment - offset: 0, length: 6 status OK: Partition not in range [6] __pli_statmnt - offset: 6, length: 8 status OK: Partition not in range [14] __pli_eofstmt - offset: 14, length: 1 status OK: Partition not in range [15] __pli_statmnt - offset: 15, length: 15status BUG: Partition not scanned. PresentationReconciler buggy. [30] __pli_eofstmt - offset: 30, length: 1 status OK: Partition matched [31] __pli_comment - offset: 31, length: 9 status OK: Partition matched [40] __pli_statmnt - offset: 40, length: 4 status OK: Partition matched [44] __pli_eofstmt - offset: 44, length: 1 status OK: Partition not in range [45] __pli_statmnt - offset: 45, length: 8 status OK: Partition not in range [53] __pli_eofstmt - offset: 53, length: 1 status OK: Partition not in range
>Is it not true that the whole range in the coverage needs to be handled by the >repairer/damager/reconciler ? Yes, it is and it does unless you are reporting two identical partitions after each other instead of one. We don't plan to change this, so please make the required changes on your side.
(In reply to comment #29) > >Is it not true that the whole range in the coverage needs to be handled by the > >repairer/damager/reconciler ? > Yes, it is and it does unless you are reporting two identical partitions after > each other instead of one. Your statement leaves me a bit confused ... since I clearly dont have two identical partitions after each other: PartitionChangedEvent offset: 15, length: 29 [15] __pli_statmnt - offset: 15, length: 15 status BUG - not repaired [30] __pli_eofstmt - offset: 30, length: 1 status OK - match [31] __pli_comment - offset: 31, length: 9 status OK - match [40] __pli_statmnt - offset: 40, length: 4 status OK - match the first partition offset 15 does not get repaired by the current version of PresentationReconciler. Still seems like a bug, but I will not prolong this discussion, since you have clearly stated your opinion, and I am unable to convince you otherwise. >We don't plan to change this, it would be helpful if you at least would document this. > so please make the required changes on your side. ok will do: welcome to PLIPresentationReconciler.java :-) thanks for your time Henrik
Sorry Henrik, I assumed that naturally, nc;lude would end up in the same partition (at least I would have done it that way). Since those are two different partitions I agree it's a bug.
Fixed in HEAD without using the patch. Available in builds > N20091125-2000.
(In reply to comment #32) > Fixed in HEAD without using the patch. > Available in builds > N20091125-2000. Sorry the fix does not work for me. Why not just do something simple like this: private IRegion getDamage(DocumentEvent e, boolean optimize) { IRegion damage; if ( fDocumentPartitioningChanged && fChangedDocumentPartitions!=null) { damage=new Region(fChangedDocumentPartitions.getOffset() ,fChangedDocumentPartitions.getLength()); } else { ITypedRegion partition= null; int length= e.getText() == null ? 0 : e.getText().length(); int deletionOffset = (length == 0?-1:0); try { partition= getPartition(e.getDocument(), e.getOffset()+deletionOffset); } catch (BadLocationException e1) { } if (partition!=null) damage=new Region(partition.getOffset(),partition.getLength()); else damage=new Region(0,e.getDocument().getLength()); } return damage; }
>Sorry the fix does not work for me. It works with the example you gave. If you have another test case feel free to attach it.
