Bugzilla – Full Text Bug Listing
| Summary: | [formatter] Formatter and Indenter are not consistent | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [WebTools] JSDT | Reporter: | Bashir Sadjad <bashir> | ||||||
| Component: | General | Assignee: | Project Inbox <jsdt.javascript-inbox> | ||||||
| Status: | NEW --- | QA Contact: | Chris Jaun <cmjaun> | ||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | bokowski, cmjaun, sam, simon_kaegi, tparker | ||||||
| Version: | unspecified | Flags: | cmjaun:
review?
(cmjaun) |
||||||
| Target Milestone: | Future | ||||||||
| Hardware: | PC | ||||||||
| OS: | Linux | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
Some other specific cases that indenter behavior is not consistent:
* Line breaks in array and object initializers do not honor all formatter settings.
* Line breaks while editing is different that running indenter afterwards, e.g., in
var a = {
a1: 's',
a2: 't'
};
hitting Enter after 't' put the cursor in wrong position.
* Line break after assignment in general, e.g.,
var a =
10;
Created attachment 203031 [details]
A patch to fix the problems listed on this bug report.
This patch addresses almost all of the issues that are listed on this bug report (plus some minor related other indent issues). I am not sure if there are any unit-tests for the indent functionality but if there is I would be happy to update them with regression tests for this patch.
Please note the patch is created from v201011172326 which is the plugin version that I want to eventually patch (internally in our org). However for the two files that have been modified they are exactly the same as HEAD so the patch can be applied both to branch and HEAD.
Nitin, will you have time to review this patch some time soon? Bashir, you could make it easier for Nitin to review your patch by getting rid of formatting changes. In the worst case, just go through the patch change by change and if it is a formatting or whitespace-only change, revert the change. Bonus points for contributing test cases as well :-) (In reply to comment #3) > Nitin, will you have time to review this patch some time soon? Hopefully in time for M3. I'm buried behind a lot of other demands for my attention. (In reply to comment #4) > Hopefully in time for M3. I'm buried behind a lot of other demands for my > attention. Thanks for responding. Are you the only one, or would it make sense to ask someone else familiar with the JSDT code base for a review? (In reply to comment #5) No, no one's done extensive work on the formatter since it was initially forked from JSDT. Will try for it in M4. Created attachment 212299 [details]
Same indenter fixes patch I have sent many months ago with just some white-space/unnecessary changes removed.
The new patch file is basically the same as the one I submitted long time ago with some white-space/unnecessary changes removed to reduce the size of the patch. It would be great if someone can review this patch and eventually commit it into JSDT code-base. It is a pain to maintain a JSDT feature patch internally for us and this patch really fixes several indenter problems, so everyone using JSDT can benefit from it.
I'm seeing another variation of this with array initializers. The formatter respects the value of the indentation policy (I use "Indent by one"), but the indenter always behaves as if it's set to "Indent on column". This is quite infuriating as the coding standards for my project specify a single-tab indent so I perpetually have to manually fix the indentation. I don't know whether Bashir's patch fixes this as I unfortunately don't have time to set up a custom build of JSDT. Bashir, you need to sign the Contributor License Agreement for us to accept patches from you. http://wiki.eclipse.org/Development_Resources/Contributing_via_Git#Eclipse_Foundation_Contributor_License_Agreement You also need to sign off the on the Certificate of Origin by adding this line to a bugzilla comment, "This contribution complies with http://www.eclipse.org/legal/CoO.php" More details here: http://wiki.eclipse.org/Development_Resources/Contributing_via_Git "This contribution complies with http://www.eclipse.org/legal/CoO.php" I have also signed the CLA. |
In many cases the behavior of indenter and formatter are not consistent. For example with some formatter preferences tweaks, I can get 4 spaces before '[' in: var t = [ 'a', 'b' ]; which is what I want. However if I select this segment and just run indenter, it changes the indent to 2 spaces. I cannot find any JSDT settings that changes this indenter behavior. Debugging the code a little bit it seems that org.eclipse.wst.jsdt.internal.ui.text.JavaIndenter has a CorePrefs internal class with a prefAssignmentIndent field that determines this indent. However this indent size is simply set to prefBlockIndent() which is (eventually) hard-coded 1. One consequence of this problem is that copy-pasting of a properly formatted block, corrupts the formatting. I got this in JSDT core ver. 1.1.4.v201011172326 and also at the current HEAD.