Bugzilla – Full Text Bug Listing
| Summary: | formatter option "align field in columns" changed in Mars | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | zzzz <nzanaga> |
| Component: | Core | Assignee: | Mateusz Matela <mateusz.matela> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | ezanaga, jarthana, luke_bastendorff, manoj.palat, markus.kell.r, mateusz.matela, mohanaraosv, titou10.titou10, tobibobi, ymene7 |
| Version: | 4.5 | Flags: | manoj.palat:
review+
|
| Target Milestone: | 4.5.1 | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| See Also: |
https://git.eclipse.org/r/53108 https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=9db26c7bea8b081f74e04400ef73e415034114ae https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?h=R4_5_maintenance&id=17a84d4eab07595592188d8d0f5bdd5566f79b61 https://bugs.eclipse.org/bugs/show_bug.cgi?id=479109 |
||
| Whiteboard: | |||
note: previous version of Eclipse Mars 4.5 , for example M1, works as Eclipse 3.7.2 This was done on purpose to make aligning more flexible. You can now have several separated groups of fields and align them independently. If you want fields separated by an empty line to be considered the same group, you can increase setting for Blank lines -> Before field declarations to 1. I don't think the "Blank lines -> Before field declarations to " is the correct option. For example if you set to 5, you get:
before:
public class TestZ {
public int a = 10;
public int testAlignment = 10;
}
after:
public class TestZ {
public int a = 10;
public int testAlignment = 10;
}
I don't find any option about how many blank lines to use for considering field declarations in same group
This is what you wanted, right? You increased the "Before field declarations" and now all the fields are aligned as one group. There's no separate option for this. Field aligner will consider two neighboring fields to be in separate groups if the number of empty lines between them is greater than the "Before field declarations" setting. This is only possible if the "Number of empty lines to preserve" is greater too. It should also be noted that this doesnt work well together with comments as you would typically seperate commented lines with new lines (see a typical example below). I think it would be better to allow the old behavior (by configuration) so people relying on this behaviour in their code standards can still use this.
// intended look
public final class SW_ETools_Event_Types
{
/** Transmitted in the case that a timer tick is being fired. */
public static final String EVENT_TIME = "time";
/** Transmitted in the case that the state machine change. */
public static final String EVENT_STATEMACHINE_CHANGE = "state";
/** Transmitted in the case that the state machine change. */
public static final String EVENT_STATEWISH_CHANGE = "wish";
/** Transmitted in the case that the display state change. */
public static final String EVENT_TOOLDISP_CHANGE = "dispst";
/** Transmitted in the case that the group/safety state change. */
public static final String EVENT_TOOLGRP_CHANGE = "grsfst";
/** Transmitted in the case that the detector state change. */
public static final String EVENT_TOOLDET_CHANGE = "detst";
/** Transmitted in the case that the I/O state change. */
public static final String EVENT_TOOLIO_CHANGE = "iost";
/** Transmitted in the case that the ITC state change. */
public static final String EVENT_TOOLITC_CHANGE = "itcst";
}
----------------------
// formatted look on mars - looks ugly and is not clearly readable.
public final class SW_ETools_Event_Types
{
/** Transmitted in the case that a timer tick is being fired. */
public static final String EVENT_TIME = "time";
/** Transmitted in the case that the state machine change. */
public static final String EVENT_STATEMACHINE_CHANGE = "state";
/** Transmitted in the case that the state machine change. */
public static final String EVENT_STATEWISH_CHANGE = "wish";
/** Transmitted in the case that the display state change. */
public static final String EVENT_TOOLDISP_CHANGE = "dispst";
/** Transmitted in the case that the group/safety state change. */
public static final String EVENT_TOOLGRP_CHANGE = "grsfst";
/** Transmitted in the case that the detector state change. */
public static final String EVENT_TOOLDET_CHANGE = "detst";
/** Transmitted in the case that the I/O state change. */
public static final String EVENT_TOOLIO_CHANGE = "iost";
/** Transmitted in the case that the ITC state change. */
public static final String EVENT_TOOLITC_CHANGE = "itcst";
}
I try to explain with a better example.
Using "Before Field Declaration" = 0 and "Number of empty line to preserve" = 1
if you try to format this:
public class TestZ {
public int a = 10;
public int testAlignment = 10;
public int z = 10;
public int testAlignmentAdder = 10;
}
with Eclipse pre 4.5RC you get this:
public class TestZ {
public int a = 10;
public int testAlignment = 10;
public int z = 10;
public int testAlignmentAdder = 10;
}
I don't find any option to get the same thing with Eclipse 4.5RC3 because:
Using "Before Field Declaration" = 0 and "Number of empty line to preserve" = 1:
public class TestZ {
public int a = 10;
public int testAlignment = 10;
public int z = 10;
public int testAlignmentAdder = 10;
}
Using "Before Field Declaration" = 1 and "Number of empty line to preserve" >=1:
public class TestZ {
public int a = 10;
public int testAlignment = 10;
public int z = 10;
public int testAlignmentAdder = 10;
}
I mean, there is not way to keep all fields aligned and preserve blank lines as wanted.
Well, I was so happy with this behavior that I failed to consider that it may get in someone's way :( Markus, would you agree to add a setting for this feature? The "Align fields in column" option looks quite lonely in its group and there's room for a new setting like "Group fields by additional empty lines". Otherwise I guess I'll just restore the old behavior. (In reply to Mateusz Matela from comment #2) > This was done on purpose to make aligning more flexible. You can now have > several separated groups of fields and align them independently. > > If you want fields separated by an empty line to be considered the same > group, you can increase setting for Blank lines -> Before field declarations > to 1. IMHO this is clearly a regression from previous versions This "feature" is very annoying as every developer automatically performs a source "format" on save and all the variable declarations now appears as "changed" when we compare the source with previous version from SVN repo The priority of this bug should be raised as this is really really annoying for dayt-to-day development and there is no workaround This "feature" should have been implemented with a new formatting option, with the default setting keeping the result formatted source code as in previous versions New Gerrit change created: https://git.eclipse.org/r/53108 For now I removed the "feature". I tried to create a setting for it, but that would require changes in API, which would not be good to backport to 4.5.1. Even i'm facing the issue, could you please share when the fix will be available. (In reply to Mohana Rao S V from comment #11) > Even i'm facing the issue, could you please share when the fix will be > available. As of now, the bug is tagged for 4.6 M2, which means this will be available by end of September. Also note that this will not be back ported to 4.5.1. (In reply to Jay Arthanareeswaran from comment #12) > Also note that this will not be back ported to 4.5.1. May I ask why? This is a regression from the users' point of view. If we don't consider it a regression, we might as well leave it in (or add a configuration). Otherwise some users will get used to the new behavior and will be annoyed again when we switch back in 4.6. (In reply to Mateusz Matela from comment #13) > May I ask why? This is a regression from the users' point of view. If we > don't consider it a regression, we might as well leave it in (or add a > configuration). Otherwise some users will get used to the new behavior and > will be annoyed again when we switch back in 4.6. Okay, makes sense. First we need to push it to master as we don't have much time left for 4.5.1. +1 (In reply to Jay Arthanareeswaran from comment #14) > (In reply to Mateusz Matela from comment #13) > > May I ask why? This is a regression from the users' point of view. If we > > don't consider it a regression, we might as well leave it in (or add a > > configuration). Otherwise some users will get used to the new behavior and > > will be annoyed again when we switch back in 4.6. > > Okay, makes sense. First we need to push it to master as we don't have much > time left for 4.5.1. +1 for restoring the 4.4 behavior in 4.6 M2 and 4.5.1. For import groups, we have a separate setting blank_lines_between_import_groups. If there's a demand for field groups, then we'd need an enhancement request and a separate setting for that. To enable field grouping, the user would then have to set the new option to a number greater than blank_lines_before_field. Gerrit change https://git.eclipse.org/r/53108 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=9db26c7bea8b081f74e04400ef73e415034114ae +1 for 4.5.1 Also +1 for 4.5.1 Released to 4.5.1 with http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?h=R4_5_maintenance&id=17a84d4eab07595592188d8d0f5bdd5566f79b61 Verified for 4.6 M2 with build I20150818-0800 and Verified for 4.5.1 with build M20150819-1000 I would really appreciate, if in future there is a possibility to change between these two modes. I really loved the changes implemented from Mateusz Matela. Just updated to the latest eclipse version and already missing this feature (aka "bug" :). (In reply to Marcus Storre from comment #22) > I would really appreciate, if in future there is a possibility to change > between these two modes. See bug 479109 for this. |
working with Mars 4.5 RC3, formatting this class, the result is: public class TestZ { public int a = 10; public int testAlignment = 10; } formatting this class, for example, with Eclipse 3.7.2, the result is: public class TestZ { public int a = 10; public int testAlignment = 10; } with Mars 4.5 RC3, the variables will be aligned only without spaces : public class TestZ { public int a = 10; public int testAlignment = 10; }