Community
Participate
Working Groups
Follow-up to bug 27079 comment 40 When bug 311582 is fixed, we can supply default tags to enable/disable the formatter. Proposals so far: - @formatter:off / @formatter:on - //J- / //J+
I much like // @formatter:on and // @formatter:off. But the small issue i can see is: if developers code it manually, there will be different forms (mostly because of spaces): 1. // @formatter: on 2. // @formatter:on 3. //@formatter:on 4. //@formatter: on And I read from the context of bug 27079 comment 40, shows this may be problem, because it only detects the exact form matching. So spaces after // may not be problem, but space between on and : may be trouble. So, if these two or similar are picked up, the related feature (request) will be: 1. Highlighting lines with these flags in the indicator bar (similar as TODO: but put on the right side only) 2. Have Context Assist to help input. But this will be a little bit tricky, since user may define any flags, and not necessarily has a common prefix for both: like <prefix>on/off or <prefix>+/-. If 1st can be done, I vote +1 for // @formatter: on/ off
sorry, should be bug 27079 comment 30, not 40.
I just don't see why spaces in the activating or deactivating tag should be a problem. If the tag is known as "@formatter:off" and someone writes "@formatter: off", adding a space where there should be no space, then the tag is not there. I see no problem there. Especially when (as I said in this comment https://bugs.eclipse.org/bugs/show_bug.cgi?id=27079#c25) the user chooses how the tag is spelled. Unless that choice is no longer available, having to care for user inconsistency looks like too much niceness. I'm on board for "@formatter:on/@formatter:off", which is very close to what was initially proposed (https://bugs.eclipse.org/bugs/show_bug.cgi?id=27079#c0). That would be a sort of credit/acknowledgement to Piotr.
@formatter:off / @formatter:on also looks fine to me. The spacing is not a problem, since // will not be part of the tags (and to support //, bug 311578 would first have to be fixed). For content assist, it's too late for 3.6, but we could consider this for 3.7.
+1 for @formatter:off / @formatter:on The only minor issue I can see with these tags is that the Javadoc view currently highlights the word after the '@' character, even if it's not a block nor an inline javadoc tag (I guess it's a known issue), hence using this tag in a Javadoc will emphasize the 'formatter' but not the trailing ':on'/':off'...
> Javadoc view currently highlights the word after the '@' character See bug 39699 and its dependencies. But this is not a problem in practice, since it doesn't make sense to put these tags in Javadoc comments anyway (why would you want to have source code formatter controls in generated documentation?).
(In reply to comment #4) > @formatter:off / @formatter:on > also looks fine to me. The spacing is not a problem, since // will not be part > of the tags (and to support //, bug 311578 would first have to be fixed). > > For content assist, it's too late for 3.6, but we could consider this for 3.7. When I say space is a "small" problem, consider: @formatter:off @formatter: off If my understanding of bug 27079 comment 30 correctly, it means that the second one will not be recognized. And as of now, there are no indicator in the left or right indicator bar (like what TODO comment tag has been done), and no highlight with for/back color in the code, so, if there is a space in between, and it may be mistakenly ignored. Of cause, it can be easily discovered and fixed once the developer tries to format the code. I'm happy to agree with all votes so far, +@formatter:off and @formatter:on (which does not have space, for this I feel a little bit strange is, usually, a space is expected after colon).
Marking RC1 since this will go in along with bug 311582. +1 for RC1.
(In reply to comment #7) > And as of now, there are no indicator in the left or right indicator bar (like > what TODO comment tag has been done), and no highlight with for/back color in > the code, so, if there is a space in between, and it may be mistakenly ignored. > Of cause, it can be easily discovered and fixed once the developer tries to > format the code. If the tag doesn't exactly match the given tags, then it won't disable the formatter. The user has to specify exact tags. > I'm happy to agree with all votes so far, +@formatter:off and @formatter:on > (which does not have space, for this I feel a little bit strange is, usually, a > space is expected after colon). Doesn't bother me not to have a space after a colon. We provide a default. Nothing prevents users from choosing a different one. Code assist will be added for 3.7.
Frédéric, Please add @formatter:off / @formatter:on as disabling and enabling default tags. Thanks.
Created attachment 167278 [details] Proposed patch
Srikanth, can you please review? Thanks
Patch looks good.
(In reply to comment #11) > Created an attachment (id=167278) [details] > Proposed patch Released for 3.6RC1 in HEAD stream.
Verified for 3.6RC1 using Build id: I20100513-1500
A quick one: spell check takes @format:off and @format:on as warning, and currently, all 3 options available are not good enough to handle it. (don't know which component to take care the spell check, so not reporting it)
(In reply to comment #16) > A quick one: spell check takes @format:off and @format:on as warning, and > currently, all 3 options available are not good enough to handle it. > > (don't know which component to take care the spell check, so not reporting it) Good point - I've fixed this in HEAD.
