Bug 430960 - [formatter] Provide examples of suitable Off/On tags for use
Summary: [formatter] Provide examples of suitable Off/On tags for use
Status: ASSIGNED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.4   Edit
Hardware: PC Windows 8
: P5 enhancement (vote)
Target Milestone: ---   Edit
Assignee: JDT-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-23 14:22 EDT by Timo Kinnunen CLA
Modified: 2014-04-23 04:29 EDT (History)
4 users (show)

See Also:


Attachments
Fix to FormatterMessages.properties (1.15 KB, patch)
2014-03-23 14:26 EDT, Timo Kinnunen CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timo Kinnunen CLA 2014-03-23 14:22:15 EDT
The default tags for turning Formatter Off and On in source code are quite verbose and it may not be obvious to some (like me) what other possibilities there are. Adding a few examples of tag pairs to use would make this clearer. I didn't realize until now that it's possible to change the tags so that /*Q*/ and /*E*/ comments can be used for quoting everything between them verbatim, for example.

My suggestion is to add these two pairs as examples:
- @formatter:off and @formatter:on
- /*Q* and /*E*
Comment 1 Timo Kinnunen CLA 2014-03-23 14:26:38 EDT
Created attachment 241147 [details]
Fix to FormatterMessages.properties

This contribution complies with http://www.eclipse.org/legal/CoO.php
Comment 2 Noopur Gupta CLA 2014-03-26 06:54:03 EDT
Once you enable the Off/On tags, you get the editable text boxes to edit the tags, which looks self-explanatory that you can change the tags.

In case we want to make it more obvious, how about mentioning "custom" tags?

Or, if we want to give examples of Off/On tags, why specifically /*Q* and /*E* ?
Comment 3 Timo Kinnunen CLA 2014-03-26 08:19:20 EDT
(In reply to comment #2)
> Once you enable the Off/On tags, you get the editable text boxes to edit the
> tags, which looks self-explanatory that you can change the tags.
> 
> In case we want to make it more obvious, how about mentioning "custom" tags?

It's those default values. They use a single word as a tag within a tag, they are long (15 characters or more to use), they contain the special character @ and they are uneven in length, which suggests that any user-chosen tags should also be all that as a starting point.

> Or, if we want to give examples of Off/On tags, why specifically /*Q* and /*E* ?

I chose those to demonstrate that:
- the tags can include characters that start or end the enclosing comment, i.e. that they can overlap the comment fully or partially
- the tags can be used equally well from block comments and end-of-line comments (for some reason it never occurred to me that /*@formatter:on*/ /*@formatter:off*/ could be useful or even legal)

In addition, they have these desirable properties:
- they can be activated in 5 characters from both end-of-line comments //*Q* and block comments /*Q*/
- they are short but still unlikely to be activated by mistake, so usable in real code

Why Q and E specifically? Those came from regular expressions where they are also used as quote, end-quote markers.
Comment 4 Martin Mathew CLA 2014-03-26 22:58:19 EDT
Off/On tags can be used in any comments, hence /*Q* and /*E* looks misleading as the user can give the tag inside a line comment or block comment or Javadoc comment.
May be we can remove @ from the current sample tag, but i think formatter:On and formatter:Off is much more readable and makes sense when another user encounter the tag in the source file. Also the current description looks detailed enough on the usage of the tags.
Comment 5 Timo Kinnunen CLA 2014-03-27 04:48:46 EDT
Note that although I consider the alternative tags better than the default (and have indeed started using them as default) this bug is not about changing the default values in any way. It is only about informing users of what is possible.

(In reply to comment #4)
> Off/On tags can be used in any comments, hence /*Q* and /*E* looks misleading as
> the user can give the tag inside a line comment or block comment or Javadoc
> comment.

How about these three examples:
- @formatter:off and @formatter:on
- //Quote/ and //End-quote/
- /*Q* and /*E*

These have a smoother progression from explicit to terse, show both line and block comments and introduce Q and E in a natural way. 

(In reply to comment #4)
> Also the current description looks detailed enough
> on the usage of the tags.

I think it's a bit misleading where it says that "tags can be used in any comments", which gives the impression that the comment has to exists before a tag can be used in it, which isn't true; a tag can be its own comment. Rather than changing the wording to capture this nuance I think it's better to follow the "show, don't tell" principle here and have examples which demonstrate this.

(In reply to comment #3)
> - they can be activated in 5 characters from both end-of-line comments //*Q* and block comments /*Q*/

Uh-oh, I clearly didn't test this well enough because this doesn't actually work. Specifically, with off/on tags set to /*Q* and /*E*, the following still gets formatted:

	//*Q*
	public static final String EXT_CLASS   = ".class"  ;
	public static final String EXT_JAVA    = ".java"   ;
	       static final String MARKER_MORE = "/*MORE*/";
	public static final String LINEFEED    = "\n"      ;
	//*E*

This might actually be a bug?
Comment 6 Martin Mathew CLA 2014-03-27 21:17:43 EDT
(In reply to Timo Kinnunen from comment #5)

> I think it's a bit misleading where it says that "tags can be used in any
> comments", which gives the impression that the comment has to exists before
> a tag can be used in it, which isn't true; a tag can be its own comment.
TODO, FIXME etc are commonly used tags and formatter Off/On is also a tag which should be used in a similar fashion.
 
> (In reply to comment #3)
> This might actually be a bug?
I tested and it works as expected. You did not use the tags in the right way in the comment.
If the tag that you defined is /*Q* and /*E* then it should be used as:
// /*Q* and // /*E* => line comment
or
/* /*Q*  */ and /* /*E*  */ => block comment

Coming back to the original topic, if you feel that it will be beneficial to show example to the user, then the best would be to follow how the other tabs handle example. We can have a preview section with some code snippet showing the effect of using the formatter tags. If user change the tag, then the code snippet should be updated to reflect the change.
Comment 7 Timo Kinnunen CLA 2014-03-28 07:28:00 EDT
(In reply to comment #6)
> (In reply to Timo Kinnunen from comment #5)
> 
> > I think it's a bit misleading where it says that "tags can be used in any
> > comments", which gives the impression that the comment has to exists before
> > a tag can be used in it, which isn't true; a tag can be its own comment.
> TODO, FIXME etc are commonly used tags and formatter Off/On is also a tag which
> should be used in a similar fashion.

You know, that almost sounds like a challenge :) But let's keep a narrower scope here and save those for another time, shall we?

> > (In reply to comment #3)
> > This might actually be a bug?
> I tested and it works as expected. You did not use the tags in the right way in
> the comment.

Oh, don't give me that. The tag is made of the characters '/', '*', 'Q', '*'. That some of those characters can be used for starting and ending comments is incidental and the way I used the tags is in line with the description: 

Off/On tags can be used in any comments to turn the formatter off and on in a source file.
- At the beginning of each file, the formatter is enabled.
- Each time the formatter sees an off tag, it disables formatting for that comment and the source after it.
- Each time the formatter sees an on tag, it enables formatting for the source after that comment.

My previous mental model was that for the comment "/*comment-text-goes-here*/" the formatter sees a tag when "comment-text-goes-here".indexOf(tag)!=-1 is true. That wasn't the whole picture, so my current mental model is that the formatter sees a tag when "/*comment-text-goes-here*/".indexOf(tag)!=-1 is true. But this isn't true either?! Apparently there is some restriction on what the formatter can see and I think if this restriction was spelled out it would contradict the description above.

Which means either the description is incorrect or the implementation is.

> If the tag that you defined is /*Q* and /*E* then it should be used as:
> // /*Q* and // /*E* => line comment
> or
> /* /*Q*  */ and /* /*E*  */ => block comment
or
///*Q* and ///*E* => line comment
or
/*Q*/ and /*E*/ => block comment

work too and are useful, so don't think about "fixing" those.

> Coming back to the original topic, if you feel that it will be beneficial to
> show example to the user, then the best would be to follow how the other tabs
> handle example. We can have a preview section with some code snippet showing the
> effect of using the formatter tags. If user change the tag, then the code
> snippet should be updated to reflect the change.

This UI is problematic. The effect of not using the formatter in an already formatted content is no effect at all, for starters.
Comment 8 Timo Kinnunen CLA 2014-03-28 08:04:12 EDT
(In reply to comment #7)
> Apparently there is some restriction on what the formatter
> can see and I think if this restriction was spelled out it would contradict the
> description above.
> 
> Which means either the description is incorrect or the implementation is.

/*Q*/
System.out.println();
/*E*/
//*Q*/
 System.out.println();
//*E*/
///*Q*/
  System.out.println();
///*E*/
////*Q*/
   System.out.println();
////*E*/
/////*Q*/
    System.out.println();
/////*E*/
//////*Q*/
     System.out.println();
//////*E*/

The formatter sees the tags in the first and third and subsequent comment pairs, but not in the second pair. Which can't be reconciled with the description without redefining what the word "in" means :)
Comment 9 Timo Kinnunen CLA 2014-03-28 14:06:07 EDT
Found more tag pairs that don't work as advertised, filed bug 431523 for them and the ones already mentioned here.
Comment 10 Markus Keller CLA 2014-03-28 14:36:17 EDT
The Javadocs of org.eclipse.jdt.core.formatter.DefaultCodeFormatterConstants#FORMATTER_DISABLING_TAG and #FORMATTER_ENABLING_TAG have the full story -- not something

(In reply to Timo Kinnunen from comment #8)
> Which can't be reconciled with the
> description without redefining what the word "in" means :)

I think the problem is not the word "in", but when the formatter "sees" a tag. Since the visual experience is very subjective, this is actually very hard to answer without deeper studies of the formatter's visual apparatus ;-)

IIRC, the implementation tries to match the behavior of other Java code formatters.


Back to the subject of this bug: If we add examples, then we better use the //J-
and //J+ from the spec. Or we just add a link to the Javadocs.
Comment 11 Timo Kinnunen CLA 2014-03-28 15:08:58 EDT
I looked at some tests about this which use //F++ and //F-- and those (and others like those) could be problematic in real use, just think of having code like:
	double F, m, A;
	//...
	F++;
	A=F/m;
	
and then using Toggle Comment on that... This is why I went for tags which include as much of the enclosing comment delimiters as possible without being limiting in the first place. Whichever tags are shown to the user should be the best that could be suggested, because they are the ones most likely to be chosen and used.