Community
Participate
Working Groups
Created attachment 245056 [details] Test source file When in project "Save Actions" enabled are: - Format Source code, Format all lines - Additional actions, Correct identation then saving file from attachment, adding a space to end file and saving again. Formatting of the source is changing.
The issue is not reproducible in Version: 4.3.2 Build id: M20140205-0800 but is reproducible from Version: Luna (4.4)Build id: I20131212-1600. It looks like a regression introduced in Luna.
This is surfaced by a bug fix which now also correctly honors the correct indentation save action. Unfortunately
*** Bug 442689 has been marked as a duplicate of this bug. ***
The code Indentation option of the save actions (CleanUpConstants.FORMAT_CORRECT_INDENTATION in the jdt.ui code) is consumed by jdt.ui and is not percolated down to the jdt.core as can be inferred from CodeFormatFix.createCleanup(). jdt.core api returns the same "edits" for both the versions of the source; this can be confirmed by switching off the code indentation option and observing that the "toggle format" behavior is not seen. The issue lies in the handling of this option at the call TextEdit edit= IndentAction.indent(document, cu.getJavaProject()); : line 142 when edit is null - ie when the current source is formatted with correct indentation. The probable fix may want to take this fact into consideration. The relevant portion of the stack trace is given below: CodeFormatFix.createCleanUp(ICompilationUnit, IRegion[], boolean, boolean,boolean, boolean) line: 142 CodeFormatCleanUp.createFix(CleanUpContext) line: 67 CleanUpRefactoring.calculateChange(CleanUpContext, ICleanUp[], List<ICleanUp>, HashSet<ICleanUp>) line: 809 CleanUpPostSaveListener.saved(ICompilationUnit, IRegion[], IProgressMonitor) line: 396 CompilationUnitDocumentProvider$5.run() line: 1598 SafeRunner.run(ISafeRunnable) line: 42 CompilationUnitDocumentProvider.notifyPostSaveListeners(CompilationUnitDocumentProvider$CompilationUnitInfo, IRegion[], IPostSaveListener[], IProgressMonitor) line: 1593 CompilationUnitDocumentProvider.commitWorkingCopy(IProgressMonitor, Object, CompilationUnitDocumentProvider$CompilationUnitInfo, boolean) line: 1380 CompilationUnitDocumentProvider$4.execute(IProgressMonitor) line: 1458 CompilationUnitDocumentProvider$4(TextFileDocumentProvider$DocumentProviderOperation).run(IProgressMonitor) line: 132 WorkspaceModifyDelegatingOperation.execute(IProgressMonitor) line: 70 WorkspaceModifyOperation$1.run(IProgressMonitor) line: 108 Workspace.run(IWorkspaceRunnable, ISchedulingRule, int, IProgressMonitor) line: 2313 WorkspaceModifyDelegatingOperation(WorkspaceModifyOperation).run(IProgressMonitor) line: 130 WorkspaceOperationRunner.run(IRunnableWithProgress, ISchedulingRule) line: 75 WorkspaceOperationRunner.run(boolean, boolean, IRunnableWithProgress) line: 65 CompilationUnitDocumentProvider(TextFileDocumentProvider).executeOperation(TextFileDocumentProvider$DocumentProviderOperation, IProgressMonitor) line: 456 CompilationUnitDocumentProvider(TextFileDocumentProvider).saveDocument(IProgressMonitor, Object, IDocument, boolean) line: 772 CompilationUnitEditor(AbstractTextEditor).performSave(boolean, IProgressMonitor) line: 5076 CompilationUnitEditor.performSave(boolean, IProgressMonitor) line: 1247 CompilationUnitEditor.doSave(IProgressMonitor) line: 1301 . . . Moving this to jdt.ui
*** Bug 434962 has been marked as a duplicate of this bug. ***
(In reply to Manoj Palat from comment #5) > *** Bug 434962 has been marked as a duplicate of this bug. *** Noopur: Please add the test case of bug 434962 as well when this bug is fixed.
*** Bug 459341 has been marked as a duplicate of this bug. ***
> line 142 when edit is null - ie when the current source is formatted with correct indentation. What I think is happening is two different things (interacting with one another to cause the 'toggling format' behavior. (1) a bug in the 'correct indentiation' action If I take the attached sample code, select it in the editor and do 'Correct Indentation' from the menu, it indents improperly (an extra tab added in front of second "+String.format". It does this incorrect indentation irrespective of how the selected code is indented beforehand (correctly or incorrectly). 2) Save action doing both 'format' and 'correct indentation' seems to 'toggle' between leaving the end result of the 'format' or the end-result of the 'correct indentation' action behind. (Which one is left behind, I guess has something to do with 'edit being null' or not, as Manoj explained). However, the problem is really that 'format' and 'correct indentation' actions do not seem to 'agree' on how the code should be indented. This can be seen by selecting the test code in open editor and peforming either a 'Format' or 'Correct Indentiation' action on the selected code. Correct indentation adds an extra tab where format does not. I am guessing that if 'format' and 'correct indentation' would both agree on how the code ought to be indented, then both problems would go away. So the problem that really needs to be fixed is (1) and this has nothing to do with save actions.
(In reply to Kris De Volder from comment #8) If we take all four examples from this bug and its duplicates, we can see that 'formatter' and 'correct indentation' work correctly when applied individually as per the corresponding state of the document (probably except for the indentation bug in example of (1)). The toggle on save bug here occurs when both are applied in the save action. The formatter works on the original document and produces certain text edits, which could influence (need not overlap) the indentation calculation. But again the original document is passed to #indent and it returns text edits based on that, which are then applied along with formatter's text edits if they don't overlap. So, the problem is that 'correct indentation' does not take formatter changes into account while calculating the indentation. If Format source code is already being applied in the save action, we should either apply formatEdits first to the document and use that to calculate indentation or, we should ignore/disable Additional Save Actions in Formatter section for the lines being formatted via Format source code (which will anyway take care of these actions also).
This bug is really bugging some of our developers, so I want to take a crack at trying to fix (+ pull request). But it looks like I'm not yet clear exactly what the problem is (at least not the whole picture). The original example attached on this bug shows a 'toggle on save'. But it seems to me that with this particular example is just because the individual 'format' or 'correct indentation' are producing inconsistent end results. I.e, if I do the actions individually they produce a different end result. I take it that this is not 'correct' (I'd expect both of these to agree on how the code should be indented. So at least one of them must be 'wrong') So I'll start by trying to fix that bug in the "correct indentation" action first. Then I'll look at all the examples and see if the 'toggle on save' problem remains in any of the examples.
Update. Sorry guys I'm going to back out of trying to fix this. I had a chat with one of the developer affected by the bug and suggested they simply disable the 'Correct indentation' save action which is redundant anyway (since Format action is already doing indentation) This seems to be a suitable workaround and makes the bug a lot less annoying/serious from our point of view.
(In reply to Kris De Volder from comment #10) > But it > seems to me that with this particular example is just because the individual > 'format' or 'correct indentation' are producing inconsistent end results. Correct.(In reply to Kris De Volder from comment #11) > Update. Sorry guys I'm going to back out of trying to fix this. > > I had a chat with one of the developer affected by the bug and suggested > they simply disable the 'Correct indentation' save action which is redundant > anyway (since Format action is already doing indentation) > > This seems to be a suitable workaround and makes the bug a lot less > annoying/serious from our point of view. Exactly.
Created attachment 250814 [details] Patch All examples from this bug and its duplicates show 5 differences between formatter and correct indentation. Attached a patch that fixes these issues in correct indentation. Verified that toggle on save does not happen in the examples after these fixes. Added new tests and all existing tests are green. Dani, please review.
(In reply to Noopur Gupta from comment #13) > Created attachment 250814 [details] [diff] > Patch > > All examples from this bug and its duplicates show 5 differences between > formatter and correct indentation. Attached a patch that fixes these issues > in correct indentation. Verified that toggle on save does not happen in the > examples after these fixes. Added new tests and all existing tests are > green. Dani, please review. Committed patch with http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=2d7e1be4fd729c6996dc3ae6518b65b35b92509f
