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:
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.