Bug 545603 - [formatter] join_wrapped_lines is ignored when alignment_for_enum_constants is "0"
Summary: [formatter] join_wrapped_lines is ignored when alignment_for_enum_constants i...
Status: NEW
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 4.10   Edit
Hardware: PC Linux
: P3 normal with 1 vote (vote)
Target Milestone: ---   Edit
Assignee: JDT-Core-Inbox CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords:
Depends on:
Blocks:
 
Reported: 2019-03-20 16:44 EDT by Christopher Tubbs CLA
Modified: 2023-05-30 01:45 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Tubbs CLA 2019-03-20 16:44:34 EDT
When running the formatter code directly (for example, in a tool such as: https://github.com/revelc/formatter-maven-plugin/) with a config file that sets the following:

org.eclipse.jdt.core.formatter.join_wrapped_lines = false

Enums like:

public enum Number {
       ONE,
       TWO,
       THREE;
}

Are wrapped to:

public enum Number {
       ONE, TWO, THREE;
}

However, when importing that config file into the Eclipse platform JDT GUI, and then exporting the config file and using that file does not wrap the enums.

I was able to isolate it to the following conflicting entry:

org.eclipse.jdt.core.formatter.alignment_for_enum_constants = 16

"16" is the default value that gets injected into the config by the GUI when it is not already set in the config being imported. However, "0" is the default value assumed when running the formatter code directly without adding this entry.

I was able to reproduce this by setting a config file with only the following two entries:

org.eclipse.jdt.core.formatter.alignment_for_enum_constants = 0
org.eclipse.jdt.core.formatter.join_wrapped_lines = false

When I format the code with this config file, the wrapped lines incorrectly get wrapped. When the alignment value is set to 16, they correctly don't get wrapped.

See the following bug for user experience:

https://github.com/revelc/formatter-maven-plugin/issues/289



There are essentially two bugs here:

1. The wrapped lines should *not* be joined when the alignment is set to 0, and
2. There is a user experience bug that the "default" value is unintuitively auto-filled to the arbitrary value of "16" for the alignment, when importing a config into Eclipse that doesn't have this entry.

Furthermore, I was unable to identify the GUI element that corresponds to this configuration entry, because the formatter editor search bar does not match anything when I try to search property name "org.eclipse.jdt.core.formatter.alignment_for_enum_constants" or just "alignment_for_enum_constants". There is room for improvement there also, although I wouldn't consider that a bug.
Comment 1 Christopher Tubbs CLA 2019-03-20 16:53:10 EDT
It turns out other non-zero values cause wrapped lines to join also... but not 16. I don't know the significance of the value of 16 here.
Comment 2 Mateusz Matela CLA 2019-04-28 08:24:34 EDT
(In reply to Christopher Tubbs from comment #0)
> 1. The wrapped lines should *not* be joined when the alignment is set to 0,
> and

The alignment set to 0 means "do not wrap" (value of 16 means "wrap where necessary"). With "do not wrap", any line breaks between enum constants are not considered valid wraps, so they are removed even when join_wrapped_lines is false. That's by design since Eclipse 4.5.

> 2. There is a user experience bug that the "default" value is unintuitively
> auto-filled to the arbitrary value of "16" for the alignment, when importing
> a config into Eclipse that doesn't have this entry.

When importing a profile from an incomplete xml file into Eclipse UI, the missing values are filled with values from the "Eclipse [built-in]" profile. When the formatter is called through API with an incomplete settings map, it defaults to values from the "Eclipse 2.1 [built-in]" profile. I agree it can be confusing and I'm not sure why we keep this old profile at all. Maybe it's because these default values are a part of public API and we don't want to change them without a very important reason.

I'm curious though, why would someone want to use the formatter with incomplete settings? Is it a problem to copy all the values form a profile that works as you want?
 
> Furthermore, I was unable to identify the GUI element that corresponds to
> this configuration entry, because the formatter editor search bar does not
> match anything when I try to search property name
> "org.eclipse.jdt.core.formatter.alignment_for_enum_constants" or just
> "alignment_for_enum_constants". There is room for improvement there also,
> although I wouldn't consider that a bug.

It's an interesting idea to recognize API keys in the filter field, although I'm not sure how useful it would be. IMO it's easy enough to find the relevant setting by filtering with word "enum" or "constant".
Comment 3 Christopher Tubbs CLA 2019-05-06 18:54:10 EDT
(In reply to Mateusz Matela from comment #2)
> (In reply to Christopher Tubbs from comment #0)
> > 1. The wrapped lines should *not* be joined when the alignment is set to 0,
> > and
> 
> The alignment set to 0 means "do not wrap" (value of 16 means "wrap where
> necessary"). With "do not wrap", any line breaks between enum constants are
> not considered valid wraps, so they are removed even when join_wrapped_lines
> is false. That's by design since Eclipse 4.5.
> 

Thank you for clarification. It's unfortunate, but understandable, that these values aren't easily understood when editing the config directly.

> > 2. There is a user experience bug that the "default" value is unintuitively
> > auto-filled to the arbitrary value of "16" for the alignment, when importing
> > a config into Eclipse that doesn't have this entry.
> 
> When importing a profile from an incomplete xml file into Eclipse UI, the
> missing values are filled with values from the "Eclipse [built-in]" profile.
> When the formatter is called through API with an incomplete settings map, it
> defaults to values from the "Eclipse 2.1 [built-in]" profile. I agree it can
> be confusing and I'm not sure why we keep this old profile at all. Maybe
> it's because these default values are a part of public API and we don't want
> to change them without a very important reason.
> 

That is excellent clarification! Thank you for that. I would definitely advocate for changing the API settings to behave the same as the GUI default behavior. Default is default. It's nice to have a consistent user experience.

> I'm curious though, why would someone want to use the formatter with
> incomplete settings? Is it a problem to copy all the values form a profile
> that works as you want?

It's not necessarily intentional. First, an "incomplete" settings can be simply the result of using an older configuration file with the newer library. Second, it's not at all obvious that a complete set is expected or required, so if a user only cares about a few settings, and prefers defaults for the rest, it's not at all obvious they should keep the ones they don't care about around.

The bug in this case, was that the experience was inconsistent using an older config file. I narrowed it down to a difference in behavior for joining wrapped lines, depending on the setting for the enum alignment. With no alignment, they are joined... even though specifying not to join should override the alignment setting. I narrowed down to a minimal config to identify the bug... but the original scenario was merely an older config file... not an incomplete config file.


>  
> > Furthermore, I was unable to identify the GUI element that corresponds to
> > this configuration entry, because the formatter editor search bar does not
> > match anything when I try to search property name
> > "org.eclipse.jdt.core.formatter.alignment_for_enum_constants" or just
> > "alignment_for_enum_constants". There is room for improvement there also,
> > although I wouldn't consider that a bug.
> 
> It's an interesting idea to recognize API keys in the filter field, although
> I'm not sure how useful it would be. IMO it's easy enough to find the
> relevant setting by filtering with word "enum" or "constant".

It would be extremely useful. If you look at the configuration property names, there are many that are very similarly named, but affect formatting in very subtle ways. I wasn't able to find the key by simple filtering on keywords. This would have helped.
Comment 4 Eclipse Genie CLA 2021-04-26 14:58:27 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.
Comment 5 Eclipse Genie CLA 2023-05-30 01:45:01 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.