|
Description
Noopur Gupta
Created attachment 276645 [details]
Bugs
Angelo, this should be fixed for 4.10. This could be an issue for CodeMining in general, not only JDT ones. It should be tried against the .target file editor which now uses code-mining too. Resetting assignee to clarify someone else can work on this at the moment. I can see the carret jump on clicking the mining in target editor. It goes to the line after the mining. I'm going to investigate and hopefully fix this one. Created attachment 276672 [details]
JDT Stack Trace when undo is done
@Mickael, as it's very hard to reproduce if you don't know the context, I give you some information. I suggest you that you set a breakpoint in the StyledText#setContent(StyledTextContent content) or better in DefaultDocumentAdapter#resumeForwardingDocumentChanges() which will call StyledText#setContent(StyledTextContent content).
This problem occurs:
* when you do a refactor of method name which have code mining header.
* when you do an undo after having importing package with completion (case of original issue). I mean import (after completion) a package. For instance if you type OutputStre and you apply it, it will add import 'java.io'. After an undo, you will have this problem too.
Both cases, call DefaultDocumentAdapter#resumeForwardingDocumentChanges(), which call StyledText#reset() which loose the lines height, etc and I think it's a reason why cursor location is lost.
Good luck @Mickael, it's a very hard task!
See attached demo to see stacktraces.
I am not able to reproduce it on Linux so far. I think it's a painting issue. The caret may actually be placed at the right line in the model, but one of the code-mining redraw or the StyledText.reset() fails are redrawing it at the right location. When you can reproduce this, can you please check how the caret behaves to the Left/Right arrow to see on which line it thinks it is? (In reply to Alexander Kurtakov from comment #5) > I can see the carret jump on clicking the mining in target editor. It goes > to the line after the mining. I don't think it's related to this one. I'm unassigning myself as I cannot reproduce it locally. Who want to give a try? > I'm unassigning myself as I cannot reproduce it locally.
Who want to give a try?
Have you set a breakpoint in DefaultDocumentAdapter#resumeForwardingDocumentChanges() to check you ar ein the usecase (try do a refactor on method which have a line header code mining below)?
(In reply to Alexander Kurtakov from comment #5) > I can see the carret jump on clicking the mining in target editor. It goes > to the line after the mining. I was trying out on target editor, but a strange behavior is visible, see Bug 541575. On a normal Java file, trying to undo does moves the caret here there, not at all predictable. Caret jump was reproducible on Windows and Mac but not on Linux. Scrolling up and down is reproducible on Linux also and that should be fixed. @Michael, You are not able to reproduce scroll up and down on Linux? Try it on TextViewer.java, it may not be reproducible on a new Java Project. (In reply to Sarika Sinha from comment #11) > @Michael, > You are not able to reproduce scroll up and down on Linux? > Try it on TextViewer.java, it may not be reproducible on a new Java Project. I'll try. Can we share some exact steps to reproduce to make sure we're testing the same thing and can refer to the same steps? Ideally putting those steps a JUnit test case? Ping! One day left for 4.10. (In reply to Dani Megert from comment #13) > Ping! One day left for 4.10. I personally didn't manage to reproduce the issue. I experienced it once in regular usage, kind of randomly, and tried to reproduce it and never managed to do it. I was able to reproduce the cursor hopping issue that Alex describes in comment 5. I filed a bug for this in SWT, see bug 542107. Mass change, please reset target if you still planning to fix this for 4.11. I could now reproduce, at least in the case of JDT with the `System.err.println("a")` line. Most other cases don't show an error.
When the issue happens on an Undo, then next Redo seems to also trigger the issue. What I'm looking at now is that StyledText.setStyleRange(...) receive a different result from getPartialTopIndex() when this issue happens from when it doesn't. A deeper cause seems to be that under some circumstances StyledText.handleTextChanged(...) isn't called -causing bug. TextViewerUndoManager.DocumentUndoListener.documentUndoNotification sometimes invoke `setRedraw(true)` which causes necessary StyledText.handleTextChanged() listener to never be invoked, so topIndex and others not computed as expected. https://git.eclipse.org/r/#/c/138192/ is an optimization/workaround: it makes the undo/redo process a simpler flow when there is only 1 change in the "Compound" one. This makes the StyledText go through a non-bugged state. I believe that while this change isn't an exhaustive fix, it's a good optimization that can have good impact on other parts of the IDE. New Gerrit change created: https://git.eclipse.org/r/138192 Gerrit change https://git.eclipse.org/r/138192 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=dd94c4b6d3b87eb57d89f2afa3ef4dc7b96e8488 New Gerrit change created: https://git.eclipse.org/r/138208 Gerrit change https://git.eclipse.org/r/138208 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=b0d63d9ce8037c9f304ee2e669621266df54820c New Gerrit change created: https://git.eclipse.org/r/138227 Gerrit change https://git.eclipse.org/r/138227 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=cc50b4dc1661f9bb249b77b57ff4a362304ff143 Merged patch seems enough to fix the erroneous case. Thanks Noopur for the report and review! |