Ability to augment/customize the outline view for Groovy scripts and other files

There is a method JavaEditor.setOutlinePageInput() that could be overridden to supply this information. We would essentially need to pass in a mock IJavaElement that has its children ordered in an appropriate way.

Here is my proposition
1. The groovy Outline page manage to fill the outline view and also to synchronize the tree nodes with the editor content. The groovy editor checks all extenders; the first one that returns an outline page will win... by default (of course) the standard outline page is used.
2. The extender receives the IProject and the GroovyCompilationUnit. It should inherit and implement the methods below :
a. OCompilationUnit.refreshChildren : this method will allow the extender to build and refresh the IOJavaElement tree
b. OCompilationUnit.getChildrenAt(int caretOffset) : this method is used to synchronize the Editor view with the outline view
c. OMethod.getElementNameNode, OMethod.getReturnTypeName, OType.getElementNameNode : those methods will determine the outline lines labels
(I have’nt yet implemented a OField classe because I did’nt need it for Jsoresso, but I will do it later)
Maxime

The first file is the patch for the org.codehaus.groovy.eclipse.ui project
The second file is the Jspresso DSL extender

This patch looks good, but there are a few things that need to be worked on before we can apply it.

• There is no OField class
• In IOutlineExtender.getGroovyOutlinePageForUnit change method signature to remove the third element. It is redundant since the GroovyCompilationUnit can be retrieved directly from the GroovyEditor.
• I saw a few e.printStackTrace(). They should be removed. Instead, GroovyCore.logException(...)

• I don't understand why you overrode getDeclaringType() in OMethod and OType. I think this method can be removed (or tell me why it should remain).

• In OCompilationUnit, override buildStructure() should be a noop and return true. This method will probably never get called, but if it ever is by accident, it could cause some big problems with the mode.

• Delete: OCompilationUnit.getGroovyNode(). Seems like this method shouldn't be there.

• Here is a big one...OutlineExtenderRegistry should not base which extender to use only on the nature. This can be problematic because let's say someone produces an extender for vanilla groovy projects so that scripts can have a custom outline view. We can safely assume that there will only be one extender per compilation unit, but I don't think we can assume there will be one per file.

Here is a suggestion:

1. add new method to IOutlineExtender:

boolean appliesTo(GroovyCompilationUnit);

Returns true if should be used for a particular file or false if not applicable.

2. this will allow someone to create a script outline enhancer (which I am planning to do) that applies to all projects with a groovy nature

3. Now, it might be that we want to lower the priority of the groovy script extender so that (for example) the jspresso plugin would have its extender run instead of the default groovy one. You may need to then force the ordering of the OutlineExtenderRegistry.natureToExtenderMap field so that extenders associated with groovy projects appear last.

• Why does OMethod have a getElementNameNode()? That feels very specific to your own implementation. Can you document why this is general purpose and how to use it, or else remove it? I see that the method is used in OMethodInfo.getNameSourceStart() and OMethodInfo.getNameSourceEnd(). Instead of getElementNameNode(), why not have getNameStart() and getNameEnd()?

• Last, of course, is discussing the testing, which I'll do in another comment.

From the mailing list:

About tests :
1) test the extension point stuff:

• if there is no extenders then the outline contains the classical methods (main, etc.)
• if there is one extenders that should create an outline O1 then the created outline is an O1 instance.
• if there is two declared extenders, the first one wins.
  2) test the outline content.
  Let's say we have a test extender that produces an outline that contains a tree of OTypes that should be synchronized with groovy methods and inline methods. Each node contains also OMethod and OField sub nodes linked to the groovy...
• test the outline content consistency
• change one method : test outline content consistency
• change one line method : test outline content consistency
• remove one method : test outline content consistency
• remove one inline method : test outline content consistency
• add one method : test outline content consistency
• add one inline method : test outline content consistency
• select one IOElement : test selected groovy method consistency
• select one groovy method : test selected IOElement node

Regards,
Maxime

This seems to me to be a fairly complete list of tests. One comment, though...your test:

• if there is two declared extenders, the first one wins.

The behavior will be undefined if two or more extenders apply to the same file. I think this is OK. But, you will also have to add a test for boolean IOutlineExtender.appliesTo(GroovyCompilationUnit).

Now, here are some suggestions and APIs that you can use to create the tests:

• check out all of the plugins in https://svn.codehaus.org/groovy/eclipse/trunk/ide-test/ and from https://svn.codehaus.org/groovy/eclipse/trunk/test/
• You can add your tests to the org.codehaus.groovy.eclipse.tests plugin
• Your test class can subclass org.codehaus.groovy.eclipse.test.EclipseTestCase.
• The test plugin already defines two test natures that you can use in your tests. You can create a couple of test extensions for the outline extender in the plugin.
• The EclipseTestCase class has a field of type TestProject. You can use this field to perform many operations on java projects like add/remove natures, add/remove java and groovy classes, etc. This should be sufficient for what you need to do.
• We don't do SWT testing, so there is no need to test that clicking on an outline selection will take you to the correct part of the code. This is manual testing that we do just prior to releases.

Let me know if you have any questions.

Hi Andrew

Here is a new proposal. I think I succeed to take account the changes you asked me. I also added some Javadoc.

I attach a first draft code for tests... I think the first tests are fine... but tests about outline content aren't reliable : i would have base my tests onto Tree/TreeItems but i didn't find any solution...then I base my test on OType/OMethod/OFields... What do you think about that ?

Regards,
Maxime

This looks great. The test coverage is fine as it is. It is not easy to do tests based on JFace components so, we usually test the model that the JFace components are based on. This has worked fine for us in the past.

I see what you mean that the tests are unreliable. There is one test that is failing when run with the other tests, but it passes when it runs alone. I'm not exactly sure what the problem is, but it is probably something to do with the working copy or the indexes not being completely initialized.

I'll have a look at it tomorrow.

OK. I fixed the tests. They are now passing reliably for me. I had to make a few changes to the tests as well as one or two changes to the plugin code.

The biggest change that I made was to ensure that the groovy compilation unit was already a working copy and already reconciled before creating the outline view. This would ensure that GroocyCompilationUnit.getModuleNode() never returns null.

This makes the tests pass, but it does make me a little concerned that we will get into situations when opening and editor and the outline view will be empty because the outline page was initialized too early (and could not get a ModuleNode). I don't know if this will be a problem, but we will see.

Resolving this issue now and committing the code. Thanks for your contribution!

On second thought. Not quite ready to commit this.

The major problem now is the expectation that once you are using an extended Outline page now, then all contained JavaElements will be OJavaElements. This doesn't really work since, for example in a script, you may have a standard method declaration or standard class declarations. These need to appear in the outline view as well, and I don't think we should be using wrappers for them.

Hello Andrew

OK, the groovy outline view should also be able to receive normal IJavaElement or may be a mix of IJavaElement and IOJavaElement...

Here is a new proposition (incremental patches to apply after previous ones),

Thanks a lot... i'am happy to contribute to groovy eclipse code :o)

Thanks for the updated patch, but I have already made a bunch of local changes. Now, it is going to be very hard to apply your new changes. Times like this, I wish we were using git. Don't worry about this. I'll incorporate your changes.

Just for me to know... will you finalize this soon ?

Yes. I hope to do so today or tomorrow.

Here is the final patch that I will be committing. Take a look at it. The major changes are these:

• Use getSourceElementAt() instead of getSourceReferenceAt(). This makes the OJavaElements more aligned with IJavaElements
• Override buildStructure as a no-op in OCompilationUnit to make sure the method never gets called and causes some really bad things to happen
• Created the GroovyScriptOutlineExtender for all groovy scripts
• The OutlineExtenderRegistry will try the GroovyScriptOutlineExtender if no other outline extenders are used. It is not registered as a regular extension point
• Allow multiple extenders to be registered per nature
• Add some more tests for the groovy script extender.

Thanks for the initial work on this. I don't think this would ever had been implemented without your patch.

Work is committed. Will be available in next dev build. Two things I notice:

1. When using an extender, most of the outline buttons are disabled. They are explicitly removed from within GroovyOutlinePage.initializeViewer(). Was there a reason for this? (I never use them and I don't even know if they work in Groovy).
2. If you start off with your Groovy file as a regular file, but then turn it into a script, the outline view does not change to reflect that. You must close and re-open the editor before the extended outline view is used. This is not really a big problem, I think.

Initial work is complete. May open new issues for the bullet points in the previous comments.

Thank you Andrew !

About the "getSourceReferenceAt" : this method is used to select a part of code in the editor when some node is selected... the outline page expected a "ISourceReference" that can be à JavaElement but may be something else ? That's not important, i see that you kept the return type as ISourceReference...

About the button, yes i think they are currently not working... I was thinking someone will work on that later and add some specific Groovy buttons... may be i will do it for my Jspresso plugin.

It works fine in debug mode... i'm waiting the groovy eclipse site update to finalize my tests...

Dev build is now available. Let me know if it works for you.

Hello Andrew !
It works fine,
It is a new great feature for Jspresso DSL !
Thank you for your help
Maxime

Here is the Jspresso implementation for the outline feature
Hope this will help some other Groovy Eclipse developpers
Maxime
www.jspresso.org