Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 320754 - [formatter] formatter:off/on tags does not work correctly
Summary: [formatter] formatter:off/on tags does not work correctly
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.6   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.6.1   Edit
Assignee: Frederic Fusier CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-23 13:04 EDT by Vimil Saju CLA
Modified: 2011-10-31 03:22 EDT (History)
7 users (show)

See Also:
daniel_megert: pmc_approved+
srikanth_sankaran: review+
Olivier_Thomann: review+


Attachments
Proposed patch (14.52 KB, patch)
2010-08-12 03:15 EDT, Frederic Fusier CLA
no flags Details | Diff
Proposed patch for R3_6_maintenance branch (14.53 KB, patch)
2010-08-30 12:37 EDT, 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 Vimil Saju CLA 2010-07-23 13:04:04 EDT
Build Identifier: 20100617-1415

trying to disable code formatting for specific pieces of code using the formatter:of/on tags does not work

Reproducible: Always

Steps to Reproduce:
//@formatter:off
//@formatter:on
public class TestClass
{
    public static void main(String[] args)
    {
        int a;int b;

        System.out.println(a);

    }
}

---

The above code does not get formatted to put the declaration statements on a new line when the formatter:off and formatter:on tags are present. 

When the formatter:off and formatter:on tags are remove the above code gets formatted correctly and the declaration statements are put on a new line.
Comment 1 Remy Suen CLA 2010-07-23 17:51:12 EDT
Bugs with the Java tooling goes to JDT.
Comment 2 Vimil Saju CLA 2010-07-28 20:46:07 EDT
Code formatting works as expected for the below code snippet though.
Here the formatter:on tag is placed just below the class declaration.

//@formatter:off

public class TestClass
{
//@formatter:on
    public static void main(String[] args)
    {
        int a;int b;

        System.out.println(a);

    }
}

I have some class level annotations which i don't want to be formatted. which is where the code formatting tags do not work

like the below example

//@formatter:off
@TestAnnotation (
  testAttribute = {"test1", "test2", "test3"}
)
//@formatter:on
public class TestClass
{
    public static void main(String[] args)
    {
        int a;int b;

        System.out.println(a);

    }
}
Comment 3 Frederic Fusier CLA 2010-08-10 09:28:05 EDT
Even the following one works:
//@formatter:off

//@formatter:on
public class X04
{
    public static void main(String[] args)
    {
        int a=0;int b;

        System.out.println(a);

    }
}

I'll investigate...
Comment 4 Frederic Fusier CLA 2010-08-12 03:15:22 EDT
Created attachment 176439 [details]
Proposed patch

There were in fact two problems addressed by this patch:
1) When enabling/disabling tags were in two consecutive line comments, the second was not taken into account
2) enabling/disabling tags were not taken into account when formatting modifiers
Comment 5 Frederic Fusier CLA 2010-08-12 03:15:52 EDT
Released in HEAD for 3.7M2
Comment 6 Frederic Fusier CLA 2010-08-12 03:17:28 EDT
Srikanth, I think it could be a good candidate for 3.6.1. Do you agree?
Comment 7 Vimil Saju CLA 2010-08-12 13:00:20 EDT
Frederic,

Did you check if the patch fixes the issue where the formatter:on/off tags enclose 
class level annotations. Here's an example.

//@formatter:off
@TestAnnotation (
  testAttribute = {"test1", "test2", "test3"}
)
//@formatter:on
public class TestClass
{
    public static void main(String[] args)
    {
        int a;int b;

        System.out.println(a);

    }
}

In the above example the formatter:on tag does not turn formatting back on.

Neither does the below example where I have put an extra line before the formatter:on tag

//@formatter:off
@TestAnnotation (
  testAttribute = {"test1", "test2", "test3"}
)

//@formatter:on
public class TestClass
{
    public static void main(String[] args)
    {
        int a;int b;

        System.out.println(a);

    }
}
Comment 8 Frederic Fusier CLA 2010-08-12 14:01:23 EDT
(In reply to comment #7)
> Frederic,
> 
> Did you check if the patch fixes the issue where the formatter:on/off tags
> enclose 
> class level annotations. Here's an example.
> 
Yep :-) As this sample was already described in comment 2, I added it to the regression tests (see test testBug320754_02 in FormatterBugTests test suite)
> 
> In the above example the formatter:on tag does not turn formatting back on.
> 
> Neither does the below example where I have put an extra line before the
> formatter:on tag
> 
> //@formatter:off
> @TestAnnotation (
>   testAttribute = {"test1", "test2", "test3"}
> )
> 
> //@formatter:on
> public class TestClass
> {
>     public static void main(String[] args)
>     {
>         int a;int b;
> 
>         System.out.println(a);
> 
>     }
> }
I didn't have this peculiar one, but I have now added it to the regression tests...

So, there won't be any problem with your test cases if you use HEAD or the next integration build.

HTH
Comment 9 Srikanth Sankaran CLA 2010-08-13 00:24:30 EDT
OK for 3.6.1.
Patch looks good.
The block comment above the tests looks incorrect (copy + paste problem)
Comment 10 Frederic Fusier CLA 2010-08-30 12:25:53 EDT
Dani,

Unfortunately, I missed to backport the fix for this bug in R3_6_maintenance.
Do you think it still feasible to put it in 3.6.1 RC3?
Comment 11 Frederic Fusier CLA 2010-08-30 12:27:13 EDT
Olivier,

We now need an additional committer review to have a chance to put this fix into 3.6.1 RC3, could you please do it?

TIA
Comment 12 Olivier Thomann CLA 2010-08-30 12:29:52 EDT
+1. This should be part of 3.6.1 as this is a new feature introduced with 3.6 and it has to work properly.
Comment 13 Frederic Fusier CLA 2010-08-30 12:37:12 EDT
Created attachment 177749 [details]
Proposed patch for R3_6_maintenance branch
Comment 14 Dani Megert CLA 2010-08-31 07:03:03 EDT
+1 for 3.6.1 RC3.
Comment 15 Frederic Fusier CLA 2010-09-01 10:18:37 EDT
(In reply to comment #14)
> +1 for 3.6.1 RC3.

Released for 3.6.1 in R3_6_maintenance stream.
Comment 16 Satyam Kandula CLA 2010-09-02 02:54:01 EDT
Verified for 3.6.1 using build M20100901-1310.
Comment 17 Satyam Kandula CLA 2010-09-14 06:54:37 EDT
Verified for 3.7M2 using build I20100909-1700
Comment 18 Zornitsa Kostadinova CLA 2011-10-28 18:24:37 EDT
This is not fixed;

Eclipse SDK
Version: 3.7.1
+ android plugins

Does not work for me on Ubuntu 10.04.3 LTS
Comment 19 Dani Megert CLA 2011-10-31 03:22:22 EDT
(In reply to comment #18)
> This is not fixed;
> 
> Eclipse SDK
> Version: 3.7.1
> + android plugins
> 
> Does not work for me on Ubuntu 10.04.3 LTS

Please open a new bug report with steps to reproduce the problem you see.