Community
Participate
Working Groups
Created attachment 252042 [details] Examples Attached zip contains three projects with modified formatter profiles. Each enum file in these projects contains different issues related to enum constant indentation, which are mentioned in comments in the files. This bugs lists the enum related issues that were reported in bug 458208. Dani, I didn't find any problem with formatter tabs/spaces using built-in formatter profile and hence didn't include that. Please let me know if I missed any example from the discussion. This is to make sure that everyone agrees on the expected formatter behavior in these cases, so that 'correct indentation' in JDT/Text (bug 400670) can be made to follow the same behavior. Dani/Manoj/Markus, please comment on what the expected behavior is in each of these cases.
Markus and I have looked at the examples and agree on the following: - The enum constants are like declarations and hence they cannot be considered a wrapping case and they must be formatted like other declarations, e.g. fields: class A { <tab>int i; <tab>int j; } Therefore, when using 'Tabs Only' the enum constants must be indented with tabs and not with spaces (if those are chosen to be used for wrapped lines). - If this behavior is a special case in the new formatter code, we can accept to deprecate the enum-constant setting and ignore it for now. UI would have to remove those options at some point.
Hi Mateusz. Any plans to fix this for M7 or during RC1?
We have run out of time for M7. Moving out to RC1. Also note that as part of this fix, we should also address bug 458208, comment #31
(In reply to Jay Arthanareeswaran from comment #3) > We have run out of time for M7. Moving out to RC1. > > Also note that as part of this fix, we should also address bug 458208, > comment #31 I understood it differently: the fix for this bug is almost done via bug 458208 but there's a remaining issue as mentioned in bug 458208 comment 31.
(In reply to Dani Megert from comment #4) > I understood it differently: the fix for this bug is almost done via bug > 458208 but there's a remaining issue as mentioned in bug 458208 comment 31. I see. In that case, are you okay with addressing the remaining issue in RC1? I hope it can wait for couple of days, can it?
(In reply to Jay Arthanareeswaran from comment #5) > (In reply to Dani Megert from comment #4) > > I understood it differently: the fix for this bug is almost done via bug > > 458208 but there's a remaining issue as mentioned in bug 458208 comment 31. > > I see. In that case, are you okay with addressing the remaining issue in > RC1? I hope it can wait for couple of days, can it? Yes.
With the patch, I can see that tabs are being used instead of spaces, which looks good. However, I can still see the extra indentation being added to enum constants, which should not be done as per comment #1. Also, the new issue mentioned in bug 458208 comment #31 needs to be addressed here.
(In reply to Noopur Gupta from comment #7) > However, I can still see the extra indentation being added to enum > constants, which should not be done as per comment #1. As I understood comment 1, it was only about wrapping with spaces and if that is problematic to implement, it's acceptable to deprecate the enum constant wrapping options. Do you actually want to deprecate the enum constant configuration altogether, or just "Indent on column"? If it's the former, I think this change would be very drastic. Up until now, the default was "Do not wrap", so making it unavailable and forcing only "Every element on a new line" will probably lead to a lot of complaints. What is the reason for this? Even removing only "Indent on column" looks strange to me. What's the problem with it? > Also, the new issue mentioned in bug 458208 comment #31 needs to be > addressed here. OK. Makes a bit of mess as it's not related to enum constants, but it's less hassle than creating a new bug.
(In reply to Mateusz Matela from comment #8) > (In reply to Noopur Gupta from comment #7) > > However, I can still see the extra indentation being added to enum > > constants, which should not be done as per comment #1. > > As I understood comment 1, it was only about wrapping with spaces and if > that is problematic to implement, it's acceptable to deprecate the enum > constant wrapping options. It was about whether to consider the enum constants as wrapping or separate elements like fields, and the conclusion was that wrapping doesn't apply to them, except of course if an enum constant definition itself exceeds the line length. Hence indentation rules apply, whether they are set to spaces, tabs or mixed. I'd prefer to not deprecate or remove any of the options if possible. Note that tomorrow evening is our last RC1 build.
(In reply to Dani Megert from comment #9) > It was about whether to consider the enum constants as wrapping or separate > elements like fields, and the conclusion was that wrapping doesn't apply to > them, except of course if an enum constant definition itself exceeds the > line length. Hence indentation rules apply, whether they are set to spaces, > tabs or mixed. > > I'd prefer to not deprecate or remove any of the options if possible. Now I'm confused, because "wrapping doesn't apply" and "not deprecate any of the options" are contradicting statements. Do you want the enum constants to respect the wrapping options or not? If they are "separate elements like fields", this effectively leads to the only available option being "Force split, every element on a new line, default indentation (+ ignore the Use spaces to indent...)". As a result, the enum constant wrapping option would have to be deprecated (replaced with "Blank lines before enum constants"?). I personally don't like this direction. Unless you're thinking about wrapping only in the context of the "Use spaces to indent wrapped lines", as I suggested in comment 8. This would mean there is currently nothing else for me to change regarding the enum constants formatting, AFAIK. (In reply to Noopur Gupta from comment #7) > However, I can still see the extra indentation being added to enum > constants, which should not be done as per comment #1. Noopur, what indentation did you mean? I don't see any extra indentation unless the "Indent on column" or "Indent by one" setting is selected. If we don't want to deprecate anything, then these settings should work for enum constants too, just as they do now?
(In reply to Mateusz Matela from comment #10) > (In reply to Noopur Gupta from comment #7) > > However, I can still see the extra indentation being added to enum > > constants, which should not be done as per comment #1. > Noopur, what indentation did you mean? I don't see any extra indentation > unless the "Indent on column" or "Indent by one" setting is selected. If we > don't want to deprecate anything, then these settings should work for enum > constants too, just as they do now? Yes, I was talking about extra indentation when "Indent on column" or "Indent by one" setting is selected, as that applies to wrapping.
Moving this out of 4.5
And what about bug 458208 comment #31 and #35? This is a smaller issue that I think could be addressed in RC2. Maybe it's actually better to add a separate bug for this.
(In reply to Mateusz Matela from comment #13) > And what about bug 458208 comment #31 and #35? > This is a smaller issue that I think could be addressed in RC2. Maybe it's > actually better to add a separate bug for this. It depends on the size of the fix. Tomorrow 20:00 EDT is our RC2 build. Regarding the enum indent vs. wrap: maybe I interpreted the wrong indentation that used spaces, as the wrap case being applied (and caused the confusion). All I tried to say is that the individual constants must be seen as individual elements that use the indentation options and should be formatted like in the attached examples.
(In reply to Dani Megert from comment #14) > (In reply to Mateusz Matela from comment #13) > > And what about bug 458208 comment #31 and #35? > It depends on the size of the fix. Tomorrow 20:00 EDT is our RC2 build. See bug 467618. > Regarding the enum indent vs. wrap: maybe I interpreted the wrong > indentation that used spaces, as the wrap case being applied (and caused the > confusion). All I tried to say is that the individual constants must be seen > as individual elements that use the indentation options and should be > formatted like in the attached examples. Somehow I still don't get what you mean. If there is still anything you'd like to be changed, just paste an example :) Note that the attached exapmles always use "force split + wrap all elements" for enum constants, so you don't see that the formatter treats them in a similar way to other wrappable elements - with other configuration, all enum constants may lay in the same line.
Verified for 4.6 RC1
