Bugzilla – Full Text Bug Listing
| Summary: | [Xtend] fold import section | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Modeling] TMF | Reporter: | Sven Efftinge <sven.efftinge> | ||||||
| Component: | Xtext | Assignee: | Holger Schill <Holger.Schill> | ||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||
| Severity: | enhancement | ||||||||
| Priority: | P3 | CC: | Holger.Schill, sebastian.zarnekow | ||||||
| Version: | 2.0.0 | Flags: | sven.efftinge:
indigo+
|
||||||
| Target Milestone: | SR2 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Mac OS X - Carbon (unsup.) | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Sven Efftinge
Pushed first version and tests to master. Please review. Wanted to move the method org.eclipse.xtext.xtend2.ui.editor.Xtend2FoldingRegionProvider.getFullTextRegionForFeature(EObject, EStructuralFeature) to org.eclipse.xtext.resource.DefaultLocationInFileProvider but this would change the API. I think there could be a profit for other clients. As nobody said something negative I mark this bug as resolved. :-) The import section should be folded initially. (In reply to comment #3) > The import section should be folded initially. I'm not sure that we have an API for that, have we? I have taken a look but I cannot find something similar. Any thoughts? A possible implementation would look like: Create a subtype of org.eclipse.xtext.ui.editor.folding.DefaultFoldedPosition that has a flag #isInitiallyCollapsed which is not part of the equals contract Instantiate that on in org.eclipse.xtext.ui.editor.folding.DefaultFoldingRegionAcceptor.newFoldedPosition(IRegion, ITextRegion) Therefore override org.eclipse.xtext.ui.editor.folding.DefaultFoldingRegionProvider.createAcceptor(IXtextDocument, Collection<FoldedPosition>) and finally process #isInitiallyCollapsed in override org.eclipse.xtext.ui.editor.folding.DefaultFoldingStructureProvider.createProjectionAnnotation(boolean, Position) where the first boolean parameter looks bogus to me and should still be ignored. Careful testing is necessary since the editor should not flicker on edit in any case (content assist that inserts imports, organize import, normal typing, manually adding imports with w/out content assist). Giving this another thought, the listed steps are not sufficient. We need a customized IFoldingRegionAcceptorExtension that takes a boolean argument #initiallyCollapsed. I think this API would benefit from a clean-up, e.g. remove the type parameter and add the boolean flag. However, this would not be backwards compatible. Created attachment 205372 [details]
Possible solution with flicker problems ins some cases
Possible solution as patch because in some cases it does flicker when adding or removing imports manually. Automatic inserted imports are no problem. Typing somewhere else as the import section is no problem.
The Flag in org.eclipse.xtext.ui.editor.folding.DefaultFoldingStructureProvider.calculateProjectionAnnotationModel(boolean) is always false. It looks bogus to me too. Mybe we should find another solution.
Any thoughts?
Thanks for the night-shift, Holger. Some feedback: I don't think that we need the package editor.folding in Xtend. Folding should not use xtextResource.getContents() but xtextResource.getParseResult()... since the former will trigger the lazy content computation which may be expensive. We should at least test it with #getParseResult for Xtend. We have to take care of that in #computeEObjectFolding, too. Is the described flicker that same that happens in the Java editor when you manually add an import below a folded import region? Or does it happen with expanded import regions, too? I could only see a that the import region gets discollapsed on organize imports. This is also true for Java. Yesterday night I had the feeling that it sometimes flickers in other situations but now I am not able to reproduce that. May be that has something to do with org.eclipse.xtext.ui.editor.folding.DefaultFoldingStructureProvider.modelChanged(XtextResource). Why do you thing that that we should use the ParserRsult instead of the resource's content? Figuring out where to fold could be even more expensive than get the content and the corresponding nodes for that stuff. I will have a further look this night. Do you really thing that we should introduce IFoldingRegionAcceptorExtension now? (In reply to comment #11) > I could only see a that the import region gets discollapsed on organize > imports. This is also true for Java. > Yesterday night I had the feeling that it sometimes flickers in other > situations but now I am not able to reproduce that. May be that has something > to do with > org.eclipse.xtext.ui.editor.folding.DefaultFoldingStructureProvider.modelChanged(XtextResource). > > Why do you thing that that we should use the ParserRsult instead of the > resource's content? The XtendResource derives the JvmModel on the first #getContent > > I will have a further look this night. > Do you really thing that we should introduce IFoldingRegionAcceptorExtension > now? no. Created attachment 205452 [details]
New Version wit respect to the comments
Used ParserResult. Hope that this is not invoking ModelInferrer on Xtend2. Uncollapsing of import retion does only take place if organize imports is invoked and this is also the case in JDT.
Please let me know if this is sufficient.
After a discussion with Sebastian I pushed a different version to the patch to master. Using the getASTElement intstead of using getRootNode.getSemanticElement() is the right choise. Mark this as resolve. When we have the possibility we should refactor the API for the folding in Xtext with respect to initiallyCollapsed. For the moment and 2.1.0 this should be sufficient. Thank you, Holger! Closing all bugs that were set to RESOLVED before Neon.0 Closing all bugs that were set to RESOLVED before Neon.0 |