Bug 332877 - [formatter] line comment wrongly put on a new line
Summary: [formatter] line comment wrongly put on a new line
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.7   Edit
Hardware: All Mac OS X - Carbon (unsup.)
: P3 normal with 1 vote (vote)
Target Milestone: 3.7 M5   Edit
Assignee: Frederic Fusier CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-17 19:13 EST by Ray V. CLA
Modified: 2011-01-25 07:54 EST (History)
3 users (show)

See Also:


Attachments
code formatting settings (28.90 KB, text/xml)
2010-12-20 12:46 EST, Ray V. CLA
no flags Details
Proposed patch (2.09 KB, patch)
2010-12-22 11:37 EST, Frederic Fusier CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ray V. CLA 2010-12-17 19:13:12 EST
Build Identifier: 20100917-0705

// @formatter:off Eclipse formatter messes up comment indentation here so turn it off for this block
public static enum Environment {
    PROD,       // Production level environments
    STAGING    // Staging
}
// @formatter:on

would like to not have to turn off formatter for this kind of code.  with formatter on it does this (which makes the comments not lineup in the editor):

public static enum Environment {
    PROD, // Production level environments
    STAGING // Staging
}


Reproducible: Always
Comment 1 Ayushman Jain CLA 2010-12-20 01:09:52 EST
(In reply to comment #0)
> [..] with
> formatter on it does this (which makes the comments not lineup in the editor):
> 
> public static enum Environment {
>     PROD, // Production level environments
>     STAGING // Staging
> }

I actually get this with default formatter settings:

public enum Environment {
    PROD, // Production level environments
    STAGING
    // Staging
}

which is also wrong.

Can you please attach your formatter settings? Thanks.
Comment 2 Ray V. CLA 2010-12-20 12:46:42 EST
Created attachment 185570 [details]
code formatting settings
Comment 3 Ray V. CLA 2010-12-20 12:52:16 EST
(In reply to comment #1)
> (In reply to comment #0)
> > [..] with
> > formatter on it does this (which makes the comments not lineup in the editor):
> > 
> > public static enum Environment {
> >     PROD, // Production level environments
> >     STAGING // Staging
> > }
> 
> I actually get this with default formatter settings:
> 
> public enum Environment {
>     PROD, // Production level environments
>     STAGING
>     // Staging
> }
> 
> which is also wrong.
> 
> Can you please attach your formatter settings? Thanks.

The comment on the last line goes on a new line for me too.  I hadn't noticed that before.  I was more concerned with the horizontal alignment.  I had a longer list of enum values that I cut down for clarity, but that turned out to be wrong... anyhow I've attached my formatter settings and here is a better representation of the bug:

    public static enum Environment {
        PROD,       // Production level environments
        STAGING,    // Staging
        S1,         // S1 (NO OUTGOING ACTION HANDLERS!)
        QA,         // QA
        DEMO,       // W3 Sales machine
        DEMO2,      // W4 Sales machine
        DEV,        // Development
        READONLY,   // When we want to swap over to readonly instance, for example.
        FAILOVER,   // SPECIAL Env that can be included to bring in failover configuration for any env.
        QUARANTINE  // Configuraton to place specific listeners on a different messaging queue!  Isolates servers.
    }

formats to this:

    public static enum Environment {
        PROD, // Production level environments
        STAGING, // Staging
        S1, // S1 (NO OUTGOING ACTION HANDLERS!)
        QA, // QA
        DEMO, // W3 Sales machine
        DEMO2, // W4 Sales machine
        DEV, // Development
        READONLY, // When we want to swap over to readonly instance, for example.
        FAILOVER, // SPECIAL Env that can be included to bring in failover configuration for any env.
        QUARANTINE
        // Configuraton to place specific listeners on a different messaging queue! Isolates servers.
    }

The two problems are:
1. The horizontal alignment isn't preserved (most important)
2. The last comment gets put on a new line (also wrong, but maybe a new bug)
Comment 4 Ayushman Jain CLA 2010-12-20 12:54:40 EST
Ah! Makes sense now. I can reproduce the same.
Comment 5 Frederic Fusier CLA 2010-12-21 11:53:50 EST
(In reply to comment #3)
> 
> The two problems are:
> 1. The horizontal alignment isn't preserved (most important)

This is not a bug, the formatter never intends to keep alignment of unrelated line comments. Please fill a new bug for this enhancement request, thanks

> 2. The last comment gets put on a new line (also wrong, but maybe a new bug)

I agree this one is an unexpected formatter output, we'll keep this bug to track work around this issue...
Comment 6 Ray V. CLA 2010-12-21 15:56:06 EST
(In reply to comment #5)
> (In reply to comment #3)
> > 
> > The two problems are:
> > 1. The horizontal alignment isn't preserved (most important)
> 
> This is not a bug, the formatter never intends to keep alignment of unrelated
> line comments. Please fill a new bug for this enhancement request, thanks
> 
> > 2. The last comment gets put on a new line (also wrong, but maybe a new bug)
> 
> I agree this one is an unexpected formatter output, we'll keep this bug to
> track work around this issue...

First off, number 1 above is a bug.  The code formatter needs to be safe; it should only apply changes you ask it to.  By default it should leave your code unchanged and any code formatting options you choose to turn on should be applied without making unwanted changes.  There is no way to achieve that with the eclipse formatter because of this bug and a couple others around comment formatting.

Secondly, if I can't convince you it's a bug, do you really need me to resubmit this as an enhancement?  I mean it'll be the exact same issue with the same title and description, just an "enhancement" rather than a "bug".  I mean I can certainly do copy and paste, but it does seem rather pointless.
Comment 7 Frederic Fusier CLA 2010-12-22 04:12:18 EST
(In reply to comment #6)
> 
> First off, number 1 above is a bug.  The code formatter needs to be safe; it
> should only apply changes you ask it to.  By default it should leave your code
> unchanged and any code formatting options you choose to turn on should be
> applied without making unwanted changes.  There is no way to achieve that with
> the eclipse formatter because of this bug and a couple others around comment
> formatting.
> 
We consider that the formatter is safe in this case. Formatting line comments at end of statement line put them one space after the statement. This how the formatter works since the beginning and that didn't hurt so many people for a long time...

I understand that you disagree with the current behavior and you want us to make it change, but then this is typically a requirement not a bug. My first thought about this change is that we would need to introduce an additional preferences to make the formatter taking into account such comments. My second thought is that it wouldn't be so easy, hence I doubt we could work on it rapidly...

Note also that version 3.6 introduced tags to disable the formatter as you show it in comment 0. So, there's a way to workaround your problem which makes this requirement not so critical.

Of course, if you think it absolutely vital for you, then you can provide a patch to implement your requirement, I will be really happy to review it.

> Secondly, if I can't convince you it's a bug, do you really need me to resubmit
> this as an enhancement?  I mean it'll be the exact same issue with the same
> title and description, just an "enhancement" rather than a "bug".  I mean I can
> certainly do copy and paste, but it does seem rather pointless.

No, I want you to open a fresh enhancement bug as this one will be used to fix the second problem: the last comment is put on a new line. Sorry, I forgot to update the summary, hence it was a little bit confusing...
Comment 8 Frederic Fusier CLA 2010-12-22 04:19:00 EST
In fact looking at existing formatter bugs, I found that somebody else already open a bug for your requirement, see bug 282988...
Comment 9 Frederic Fusier CLA 2010-12-22 11:37:27 EST
Created attachment 185723 [details]
Proposed patch

This patch just prints last comment before the closing brace when there are constants declared in the enumeration.
Comment 10 Frederic Fusier CLA 2010-12-22 11:37:50 EST
Released for 3.7M5 in HEAD stream.
Comment 11 Ray V. CLA 2010-12-22 13:47:37 EST
Cool, glad you fixed that so fast, even if it wasn't the bug I originally reported.  Also, I suppose it makes sense that you changed the title of the bug since the thing that got fixed is a totally separate issue then the one I reported.

The main reason I push for the line comment indentation thing being a bug is that in the past I've seen enhancements get pushed from release to release and never get done.  I do still consider it a bug, but who really cares what we call it as long as it gets fixed?  In the end what matters is what the committers think, and it looks like we have a philosophical difference there.  So let me explain my point of view.

Here's why I consider correct code formatting vital.  The code formatting is an awesome feature, why not have it always turned on?  The problem is that the default code formatting options mangle your code and  I never used the feature in the past because of that.  But recently I spent the time to tweak the settings into something that did mostly the right thing.  It got so close to being good enough that I could turn it on as a save action and never have to think about it again, except for the end of line line comments removing indentation.  This may be "safe" in terms of it not causing compile errors, but it is not "safe" in terms of "doing only the transformations you ask it to".  So I can't make it a save action and have perfectly formatted code every time because of some silly issue of comments on different lines not lining up horizontally.

So I just have one other question.  There is another bug that causes Javadoc Formatting to not respect the "Never Join Lines" setting.  I have that setting turned on with columns set to 9999, and in block comments, the text that spans multiple lines stays where it is, but in javadocs it gets put on the same line.  So I have to turn off javadoc formatting entirely.  Do I file this as a bug or an enhancement?
Comment 12 Frederic Fusier CLA 2010-12-23 04:04:51 EST
(In reply to comment #11)
> Cool, glad you fixed that so fast, even if it wasn't the bug I originally
> reported.  Also, I suppose it makes sense that you changed the title of the 
> bug since the thing that got fixed is a totally separate issue then the one
> I reported.
> 
As I said in comment 8, the bug you originally opened was in fact a duplicate of bug 282988 (which was rightly opened as an enhancement), hence I have used bug 332877 to fix the second problem you noticed... That's the reason why I changed the summary.

> The main reason I push for the line comment indentation thing being a bug is
> that in the past I've seen enhancements get pushed from release to release and
> never get done.  I do still consider it a bug, but who really cares what we
> call it as long as it gets fixed?  In the end what matters is what the
> committers think, and it looks like we have a philosophical difference there. 
> So let me explain my point of view.
> 
I can ensure you that developers really care about this. This is definitely important for us to know whether a reported issue was in a functionality which is expected to work properly but does not or it was a functionality which is missing to our product... This gives us important information without having to look at the content of the reported issue:
1) could it be a regression
2) could it be fixed in a maintenance stream or not
3) could it have backward compatibility issue when fixed

In case of enhancement, the answer for 2 first questions is "no" and that makes it something which does not need an urgent matter. The answer to last question is "yes" and that means we cannot change the current behavior but instead propose an alternative to implement the new behavior (typically adding a preference for the formatter).

The side effect of this is that it could be pushed from release to release, but as you may know, in all software development, we need to prioritize the enhancements as we always lack of resources to implement them all. And changing the nature of the issue to a bug instead of an enhancement is not the right thing to do to have a chance to modify this fact...

> Here's why I consider correct code formatting vital.  The code formatting is 
> an awesome feature, why not have it always turned on?  The problem is that 
> the default code formatting options mangle your code and  I never used the 
> feature in the past because of that.  But recently I spent the time to tweak 
> the settings into something that did mostly the right thing.  It got so close 
> to being good enough that I could turn it on as a save action and never have 
> to think about it again, except for the end of line line comments removing
> indentation.  This may be "safe" in terms of it not causing compile errors, 
> but it is not "safe" in terms of "doing only the transformations you ask it  
> to". So I can't make it a save action and have perfectly formatted code every 
> time because of some silly issue of comments on different lines not lining up
> horizontally.
> 
This your perception in this peculiar case that the formatter mangle your code, but for an other user, it could be the opposite feeling. So fixing it for you would introduce a regression for another person...

As I said in comment 7, I understand that you have a real need. But to have a chance to see this requirement implemented you need either convinced us of the gain to spend time on this new functionality. Usually, 'performance' is the magic word, but it should not apply in this case. The second magic word is 'votes'... The most of votes you get on your requirement the best chance you have to see it implemented. But honestly, the real magic word is 'patch'! As I already said, I will be really happy to review any patch you'll be willing to attach to bug 282988.

As I also said in comment 7, you already have a workaround to avoid to see you code mangled, hence you can use it while waiting for this functionality to appear in a future release.

> So I just have one other question.  There is another bug that causes Javadoc
> Formatting to not respect the "Never Join Lines" setting.  I have that setting
> turned on with columns set to 9999, and in block comments, the text that spans
> multiple lines stays where it is, but in javadocs it gets put on the same 
>  line. So I have to turn off javadoc formatting entirely.  Do I file this as 
> a bug or an enhancement?

There's a preference to 'Never Join Lines' in comments, then first verify that it's also checked in the 'Comments' preference tab. If this preference is activated, then, as it does not seem to work properly, you can open a bug with all necessary information to help us to reproduce it easily.
Comment 13 Ray V. CLA 2010-12-23 17:58:18 EST
Thanks for the explanation.  I somehow missed your comment about bug 282988 before.  Also missed it in the search I did before filing the original bug.  We are actually in agreement on how to deal with this, I apologize for the confusion.  Unfortunately bug 282988 is almost 1.5 years old with no movement, but I've voted for it now.  Not sure I can commit to working on a patch, but I may have a look if I have some time.
Comment 14 Satyam Kandula CLA 2011-01-25 07:54:30 EST
Verified for 3.7M5 using build I20110124-1800