Bug 311582 - [formatter] Master switch to enable/disable on/off tags
Summary: [formatter] Master switch to enable/disable on/off tags
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 RC1   Edit
Assignee: Frederic Fusier CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 197153 (view as bug list)
Depends on:
Blocks: 311617 311835
  Show dependency tree
 
Reported: 2010-05-04 12:14 EDT by Markus Keller CLA
Modified: 2020-10-12 16:12 EDT (History)
5 users (show)

See Also:
daniel_megert: pmc_approved+
Olivier_Thomann: review+

Attachments
Proposed patch (16.36 KB, patch)
2010-05-05 04:57 EDT, Frederic Fusier CLA 		no flags Details | Diff
New proposed patch (18.78 KB, text/plain)
2010-05-05 07:45 EDT, Frederic Fusier CLA 		no flags Details
New proposed patch + tests (20.48 KB, patch)
2010-05-05 13:05 EDT, Frederic Fusier CLA 		no flags Details | Diff
Last proposed patch + tests (21.23 KB, patch)
2010-05-06 05:25 EDT, Frederic Fusier CLA 		no flags Details | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2010-05-04 12:14:55 EDT 
HEAD, follow up to bug 27079 comment 39

We should add a master switch to enable/disable the tags in the FORMATTER_DISABLING_TAG and FORMATTER_ENABLING_TAG preferences (disabled by default).

Without that, we can never introduce defaults for the tags, since the defaults would become active immediately.

When we have the master switch, we can also set a default as per the arguments in bug 27079 comment 39. The hard thing is to come up with good defaults, since we have only one chance. Unless we have convincing arguments for a specific pair of defaults, we can also defer this decision to later versions of Eclipse.
Comment 1 Markus Keller CLA 2010-05-04 12:22:11 EDT 
When bug 311578 is fixed, my favorite defaults would be "//J-" and "//J+", since that seems to be "industry standard". But I'm open to other suggestions.
Comment 2 Olivier Thomann CLA 2010-05-04 14:57:47 EDT 
Frédéric,

Could you please add the master switch? If we don't do it for 3.6, it will be a nightmare to introduce it later without breaking existing clients.
Comment 3 Olivier Thomann CLA 2010-05-04 15:28:32 EDT 
A new code formatter option needs to be added to do this. PMC, please approve addition of a new option.
Frédéric will post a patch with the new option name.
Comment 4 Frederic Fusier CLA 2010-05-05 04:57:38 EDT 
Created attachment 167087 [details]
Proposed patch

The proposed name is FORMATTER_USE_ON_OFF_TAGS. I initially thought about FORMATTER_USE_TAGS but that sounded a little bit too much generic...
Comment 5 Frederic Fusier CLA 2010-05-05 04:58:34 EDT 
Markus, Olivier, please review
Comment 6 Frederic Fusier CLA 2010-05-05 07:45:47 EDT 
Created attachment 167117 [details]
New proposed patch

This patch slightly improves the formatter when the tags are not used...
Comment 7 Olivier Thomann CLA 2010-05-05 12:02:22 EDT 
Patch looks good.
Could you just add at least one test that is setting the value to false with some tags?
Comment 8 Frederic Fusier CLA 2010-05-05 13:05:36 EDT 
Created attachment 167178 [details]
New proposed patch + tests

Added two tests to verify that the code is formatted by default even if disabling and enabling tags are defined and used.
Comment 9 Markus Keller CLA 2010-05-06 04:46:27 EDT 
Looks good, but the FORMATTER_DISABLING_TAG and FORMATTER_ENABLING_TAG APIs need to tell that they are ignored if FORMATTER_USE_ON_OFF_TAGS is FALSE.
Comment 10 Frederic Fusier CLA 2010-05-06 05:25:14 EDT 
Created attachment 167276 [details]
Last proposed patch + tests

Last patch addressing the Markus' remarks made in comment 9.
Comment 11 Frederic Fusier CLA 2010-05-06 05:32:27 EDT 
(In reply to comment #10)
> Created an attachment (id=167276) [details]
> Last proposed patch + tests
> 
Released for 3.6RC1 in HEAD stream.
Comment 12 Srikanth Sankaran CLA 2010-05-17 01:46:04 EDT 
Verified for 3.6RC1 using Build id: I20100513-1500
Comment 13 Mateusz Matela CLA 2020-10-12 16:12:44 EDT 
*** Bug 197153 has been marked as a duplicate of this bug. ***