Re: checkstyle-5.5 support, updated rules

2012-03-05 Thread Vincent Hennebert
On 03/03/12 00:57, Glenn Adams wrote:
snip/
 2. for NoWhitespaceAfter, specified that line breaks are allowed after 
 DOT;
 if this isn't done, then one cannot break a long line before *or* after DOT,
 and consequently could force long lines when not desired; this because the
 default settings for the NoWhitespaceBefore rule disallows a line break
 before DOT;

DOT is not in NoWhitespaceBefore’s default list of tokens to check.
Therefore it /is/ allowed to break a line before a dot. This already
happens plenty of times in the codebase.

Fixing the violations reported by the NoWhitespaceAfter rule is just
a matter of moving the dot to the next line.

Can we revert this change?

 
 G.
 

Thanks,
Vincent


Re: checkstyle-5.5 support, updated rules

2012-03-05 Thread Glenn Adams
On Mon, Mar 5, 2012 at 12:38 PM, Vincent Hennebert vhenneb...@gmail.comwrote:

 On 03/03/12 00:57, Glenn Adams wrote:
 snip/
  2. for NoWhitespaceAfter, specified that line breaks are allowed after
  DOT;
  if this isn't done, then one cannot break a long line before *or* after
 DOT,
  and consequently could force long lines when not desired; this because
 the
  default settings for the NoWhitespaceBefore rule disallows a line break
  before DOT;

 DOT is not in NoWhitespaceBefore’s default list of tokens to check.
 Therefore it /is/ allowed to break a line before a dot. This already
 happens plenty of times in the codebase.


ok, i see I misread [1] and didn't notice that DOT was in the default tokens

[1]
http://checkstyle.sourceforge.net/config_whitespace.html#NoWhitespaceBefore



 Fixing the violations reported by the NoWhitespaceAfter rule is just
 a matter of moving the dot to the next line.

 Can we revert this change?


actually this (allowing line break after DOT) was allowed in the
checkstyle-5.1 rules that we have been using in FOP, so the new proposal to
disallow line break after DOT is a new change (which hasn't yet been
implemented); so it isn't really revering a change;

in any case, i see that only 12 new errors are produced by disallowing line
break after DOT, and, since i don't have a strong preference on this issue,
i will go ahead and add the new restriction along with fixes for these 12
violations


Re: checkstyle-5.5 support, updated rules

2012-03-05 Thread Glenn Adams
On Mon, Mar 5, 2012 at 1:14 PM, Glenn Adams gl...@skynav.com wrote:

 On Mon, Mar 5, 2012 at 12:38 PM, Vincent Hennebert 
 vhenneb...@gmail.comwrote:

 On 03/03/12 00:57, Glenn Adams wrote:
 snip/
  2. for NoWhitespaceAfter, specified that line breaks are allowed after
  DOT;
  if this isn't done, then one cannot break a long line before *or* after
 DOT,
  and consequently could force long lines when not desired; this because
 the
  default settings for the NoWhitespaceBefore rule disallows a line break
  before DOT;

 DOT is not in NoWhitespaceBefore’s default list of tokens to check.
 Therefore it /is/ allowed to break a line before a dot. This already
 happens plenty of times in the codebase.


 ok, i see I misread [1] and didn't notice that DOT was in the default
 tokens


s/was in/was not in/


 [1]
 http://checkstyle.sourceforge.net/config_whitespace.html#NoWhitespaceBefore



Re: checkstyle-5.5 support, updated rules

2012-03-02 Thread Vincent Hennebert
Please can we wait to reach an agreement before making any changes?
I realize the thread in @general hasn’t made any progress in 3 weeks and
I’ll try to revive it, but making changes now will just make things more
confusing.

In particular, I think we want to all use the same version of Checkstyle
in order to ensure consistency. ATM Checkstyle 5.1 is used but when the
new set of rules is in place I think we will all want to switch to
Checkstyle 5.5.

The checkstyle.location however is a good move, I like it.

Thanks,
Vincent


On 02/03/12 00:30, Glenn Adams wrote:
 In order to proceed on Vincent's proposed new checkstyle rules [1], I've
 committed support for checkstyle-5.5 and the new rules [2].
 
 [1]
 http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3c4f2c1d25.8010...@gmail.com%3e
 [2] http://svn.apache.org/viewvc?view=revisionrevision=1296000
 
 More specifically, I parameterized build.xml to use either checkstyle-5.1
 or checkstyle-5.5, where the (current) default is checkstyle-5.1. Which
 version is used can be controlled by setting the following ant parameters:
 
- checkstyle.location (default: ${lib-tools}/checkstyle-all-5.1.jar)
- checkstyle.config (default: ${basedir}/checkstyle-5.1.xml)
 
 I'm specifying the following in my build-local.properties in order to
 select checkstyle-5.5:
 
 checkstyle.config = ${basedir}/checkstyle-5.5.xml
 checkstyle.location = ${basedir}/lib/build/checkstyle-5.5-all.jar
 
 I manually merged the new rules proposed by Vincent with the old set of
 checkstyle-5.1 rules to produce a new checkstyle-5.5.xml config file as
 follows:
 
- if adding a new rule did not produce new errors, i added; otherwise, i
added a commented out rule with a comment indicating the number of new
errors
- if changing an existing rule did not produce new errors, i changed it;
otherwise, i added a comment indicating the number of new errors produced
- for existing rules not included in [1], I removed some but retained a
few, about which see the additional notes below;
- i reordered the rules to be alphabetical, and added some divider
comments to make it a little easier to read the separate rules
 
 As of this commit, both checkstyle-5.1 and -5.5 do not produce any errors;
 however, in order to make full use of all the proposed new rules or changed
 rules some code will need to change or the proposed new rule or rule change
 dropped. An ordered list of the number of new errors that would be produced
 (by rule) is as follows:
 
 ParenPad1
 MethodParamPad  4316
 WhitespaceAfter 2203
 ExplicitInitialization  795
 ImportOrder 255
 NoWhitespaceAfter   183
 NewLineAtEndOfFile  111
 UnusedImports   80
 MultipleVariableDeclarations78
 RegexpSingleLine46
 OneStatementPerLine 43
 RedundantImport 22
 DefaultComesLast9
 RightCurly  5
 RedundantModifier   3
 GenericWhitespace   1
 NoWhitespaceBefore  1
 
 For those rules above with less than, say 300 errors, the errors can be
 addressed (in subsequent commits) and the rule enabled. For those rules
 above producing more than 500 new errors, I would suggest we not attempt to
 implement the rules, particularly since we don't have full consensus on
 their use.
 
 Here are my notes on the rule merge process:
 
 ConstantName
   1. current format ^([A-Z](_?[A-Z0-9]+)*)|(log)$
   2. proposed (default) format ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$
   3. problems with current code base
  (a) requires at least one '_'
  (b) requires length  1
  (b) doesn't admit 'log'
   4. conclusion: retain existing format
 
 DefaultComesLast
   1. produces 9 new errors
 
 DoubleCheckedLockinng
   1. left in, even though not present in proposed rules
 
 EmptyBlock
   1. left LITERAL_CATCH, even though removed in proposed rule
   2. changing block policy to default (stmt) instead of text produces 110
 new errors
 
 EmptyForIteratorPad
   1. removed (not present in proposed rules)
 
 EqualsHashCode
   1. left in, even though not present in proposed rules
 
 ExplicitInitialization
   1. produces 795 new errors
 
 FileLength
   1. removed (not present in proposed rules)
 
 GenericWhitespace
   1. produces 1 new error
 
 ImportOrder
   1. produces 255 new errors
 
 InnerAssignment
   1. left in, even though not present in proposed rules (I propose we
 remove this rule, but want to confirm with the group before doing so)
 
 Javadoc{Method,Type,Variable}
   1. removed (not present in proposed rules)
 
 LineLength
   1. removed, as suggested by cbowditch [3]
 
 [3]
 http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3cblu0-smtp420d413c8764cc374545b5bfb...@phx.gbl%3e
 
 MethodLength
   1. removed (not present in proposed rules)
 
 MethodParamPad
   1. 

Re: checkstyle-5.5 support, updated rules

2012-03-02 Thread Glenn Adams
I think we will need to come to an agreement (on the final set of 5.5
rules) incrementally rather than all at once. Furthermore, implementing the
changes to accommodate the new rules will also (most likely) occur
incrementally.

While this is being done, it is useful to not disturb the checkstyle usage
of others, so it makes sense to parameterize the selected configuration.

While the changes are being made for support the new rules, it also makes
it easier to switch back and forth between the new and old rules, and
ensure that the code changes aren't breaking on either rule set.

Once all the changes are effected, then we can change the default config to
the new 5.5 rules.

In the future, we can use the same parameterization to help transition to
newer versions of checkstyle, 5.6, 5.7, etc.

So having a checkstyle.config parameter turns out to be very useful.

Now that I have put this mechanism in place, we can proceed with making the
code changes to accommodate the new rules, and we can divide up that task
between us and others can join as well if they wish.

As Chris mentioned previously, there are risks involved in making
auto-fixes, so I prefer to use manual edits for the changes.

If you don't mind, I'd like to start effecting the changes needed for
fixing the rules involving 300 or less new errors as shown in the report
below. I would address these in multiple batches (commits) to make it
easier to manage the changes, e.g., one commit per rule. Specifically, I
would start with:

 ImportOrder 255
 NoWhitespaceAfter   183
 NewLineAtEndOfFile  111
 UnusedImports   80
 MultipleVariableDeclarations78
 RegexpSingleLine46
 OneStatementPerLine 43
 RedundantImport 22
 DefaultComesLast9
 RightCurly  5
 RedundantModifier   3
 GenericWhitespace   1
 NoWhitespaceBefore  1

Are there any objections on my proceeding with this?

On Fri, Mar 2, 2012 at 4:30 AM, Vincent Hennebert vhenneb...@gmail.comwrote:

 Please can we wait to reach an agreement before making any changes?
 I realize the thread in @general hasn’t made any progress in 3 weeks and
 I’ll try to revive it, but making changes now will just make things more
 confusing.

 In particular, I think we want to all use the same version of Checkstyle
 in order to ensure consistency. ATM Checkstyle 5.1 is used but when the
 new set of rules is in place I think we will all want to switch to
 Checkstyle 5.5.

 The checkstyle.location however is a good move, I like it.

 Thanks,
 Vincent


 On 02/03/12 00:30, Glenn Adams wrote:
  In order to proceed on Vincent's proposed new checkstyle rules [1], I've
  committed support for checkstyle-5.5 and the new rules [2].
 
  [1]
 
 http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3c4f2c1d25.8010...@gmail.com%3e
  [2] http://svn.apache.org/viewvc?view=revisionrevision=1296000
 
  More specifically, I parameterized build.xml to use either checkstyle-5.1
  or checkstyle-5.5, where the (current) default is checkstyle-5.1. Which
  version is used can be controlled by setting the following ant
 parameters:
 
 - checkstyle.location (default: ${lib-tools}/checkstyle-all-5.1.jar)
 - checkstyle.config (default: ${basedir}/checkstyle-5.1.xml)
 
  I'm specifying the following in my build-local.properties in order to
  select checkstyle-5.5:
 
  checkstyle.config = ${basedir}/checkstyle-5.5.xml
  checkstyle.location = ${basedir}/lib/build/checkstyle-5.5-all.jar
 
  I manually merged the new rules proposed by Vincent with the old set of
  checkstyle-5.1 rules to produce a new checkstyle-5.5.xml config file as
  follows:
 
 - if adding a new rule did not produce new errors, i added;
 otherwise, i
 added a commented out rule with a comment indicating the number of new
 errors
 - if changing an existing rule did not produce new errors, i changed
 it;
 otherwise, i added a comment indicating the number of new errors
 produced
 - for existing rules not included in [1], I removed some but retained
 a
 few, about which see the additional notes below;
 - i reordered the rules to be alphabetical, and added some divider
 comments to make it a little easier to read the separate rules
 
  As of this commit, both checkstyle-5.1 and -5.5 do not produce any
 errors;
  however, in order to make full use of all the proposed new rules or
 changed
  rules some code will need to change or the proposed new rule or rule
 change
  dropped. An ordered list of the number of new errors that would be
 produced
  (by rule) is as follows:
 
  ParenPad1
  MethodParamPad  4316
  WhitespaceAfter 2203
  ExplicitInitialization  795
  ImportOrder 255
  NoWhitespaceAfter   183
  NewLineAtEndOfFile  111
  UnusedImports  

Re: checkstyle-5.5 support, updated rules

2012-03-02 Thread Vincent Hennebert
OK, I understand your approach now. Makes sense. The discussion on
general@ can continue in parallel then.

I’m OK with progressively fixing the warnings in different sets of
commits and I agree with your proposed list. Will you also fix the
tests? Eventually we want to check them as well.

I’d be happy to fix RegexpSingleLine and NewLineAtEndOfFile if that
helps.

Thanks for looking into this!

Vincent


On 02/03/12 17:27, Glenn Adams wrote:
 I think we will need to come to an agreement (on the final set of 5.5
 rules) incrementally rather than all at once. Furthermore, implementing the
 changes to accommodate the new rules will also (most likely) occur
 incrementally.
 
 While this is being done, it is useful to not disturb the checkstyle usage
 of others, so it makes sense to parameterize the selected configuration.
 
 While the changes are being made for support the new rules, it also makes
 it easier to switch back and forth between the new and old rules, and
 ensure that the code changes aren't breaking on either rule set.
 
 Once all the changes are effected, then we can change the default config to
 the new 5.5 rules.
 
 In the future, we can use the same parameterization to help transition to
 newer versions of checkstyle, 5.6, 5.7, etc.
 
 So having a checkstyle.config parameter turns out to be very useful.
 
 Now that I have put this mechanism in place, we can proceed with making the
 code changes to accommodate the new rules, and we can divide up that task
 between us and others can join as well if they wish.
 
 As Chris mentioned previously, there are risks involved in making
 auto-fixes, so I prefer to use manual edits for the changes.
 
 If you don't mind, I'd like to start effecting the changes needed for
 fixing the rules involving 300 or less new errors as shown in the report
 below. I would address these in multiple batches (commits) to make it
 easier to manage the changes, e.g., one commit per rule. Specifically, I
 would start with:
 
 ImportOrder 255
 NoWhitespaceAfter   183
 NewLineAtEndOfFile  111
 UnusedImports   80
 MultipleVariableDeclarations78
 RegexpSingleLine46
 OneStatementPerLine 43
 RedundantImport 22
 DefaultComesLast9
 RightCurly  5
 RedundantModifier   3
 GenericWhitespace   1
 NoWhitespaceBefore  1
 
 Are there any objections on my proceeding with this?
 
 On Fri, Mar 2, 2012 at 4:30 AM, Vincent Hennebert vhenneb...@gmail.comwrote:
 
 Please can we wait to reach an agreement before making any changes?
 I realize the thread in @general hasn’t made any progress in 3 weeks and
 I’ll try to revive it, but making changes now will just make things more
 confusing.

 In particular, I think we want to all use the same version of Checkstyle
 in order to ensure consistency. ATM Checkstyle 5.1 is used but when the
 new set of rules is in place I think we will all want to switch to
 Checkstyle 5.5.

 The checkstyle.location however is a good move, I like it.

 Thanks,
 Vincent


 On 02/03/12 00:30, Glenn Adams wrote:
 In order to proceed on Vincent's proposed new checkstyle rules [1], I've
 committed support for checkstyle-5.5 and the new rules [2].

 [1]

 http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3c4f2c1d25.8010...@gmail.com%3e
 [2] http://svn.apache.org/viewvc?view=revisionrevision=1296000

 More specifically, I parameterized build.xml to use either checkstyle-5.1
 or checkstyle-5.5, where the (current) default is checkstyle-5.1. Which
 version is used can be controlled by setting the following ant
 parameters:

- checkstyle.location (default: ${lib-tools}/checkstyle-all-5.1.jar)
- checkstyle.config (default: ${basedir}/checkstyle-5.1.xml)

 I'm specifying the following in my build-local.properties in order to
 select checkstyle-5.5:

 checkstyle.config = ${basedir}/checkstyle-5.5.xml
 checkstyle.location = ${basedir}/lib/build/checkstyle-5.5-all.jar

 I manually merged the new rules proposed by Vincent with the old set of
 checkstyle-5.1 rules to produce a new checkstyle-5.5.xml config file as
 follows:

- if adding a new rule did not produce new errors, i added;
 otherwise, i
added a commented out rule with a comment indicating the number of new
errors
- if changing an existing rule did not produce new errors, i changed
 it;
otherwise, i added a comment indicating the number of new errors
 produced
- for existing rules not included in [1], I removed some but retained
 a
few, about which see the additional notes below;
- i reordered the rules to be alphabetical, and added some divider
comments to make it a little easier to read the separate rules

 As of this commit, both checkstyle-5.1 and -5.5 do not produce any
 errors;
 however, in order to make full use of all the proposed new rules or
 changed
 rules some code will 

Re: checkstyle-5.5 support, updated rules

2012-03-02 Thread Glenn Adams
On Fri, Mar 2, 2012 at 11:18 AM, Vincent Hennebert vhenneb...@gmail.comwrote:

 I’d be happy to fix RegexpSingleLine and NewLineAtEndOfFile if that
 helps.


Sure, that will help divide the effort. One way you might do this (which I
will use), is:

(1) changing the new checkstyle.* params in your local properties file to
5.5 config as I showed in my earlier email;
(2) run checkstyle target to verify things are working (and zero errors);
(3) uncommenting the specific rule you are making fixes for in the 5.5
config file;
(4) rerunning checkstyle and making fixes until zero errors for that new
rule;
(5) committing those change;

I'm sure I don't need to explain this to you, but it doesn't hurt to keep
others informed.


Re: checkstyle-5.5 support, updated rules

2012-03-02 Thread Glenn Adams
On Fri, Mar 2, 2012 at 11:18 AM, Vincent Hennebert vhenneb...@gmail.comwrote:

 Will you also fix the
 tests? Eventually we want to check them as well.


I agree that we will want to do so, but I'd prefer to handle the currently
checkstyle tested code first, and then subsequently do a separate fix round
to bring test code into compliance.


Re: checkstyle-5.5 support, updated rules

2012-03-02 Thread Glenn Adams
On Thu, Mar 1, 2012 at 5:30 PM, Glenn Adams gl...@skynav.com wrote:

 ParenPad1
 MethodParamPad  4316
 WhitespaceAfter 2203
 ExplicitInitialization  795


Not implemented. I oppose enabling ParenPad or MethodParamPad for two
reasons: (1) the large number of changes that would be required to fix
violations; and (2) my preferred setting for styles differ from what was
proposed. For similar reasons, I oppose adding TYPECAST to WhitespaceAfter.

That leaves ExplicitInitialization, which I don't oppose, but will take
some time to implement due to the large number of existing violations.


 NewLineAtEndOfFile  111
 RegexpSingleLine46


Will wait for Vincent to implement these two.


 ImportOrder 255
 NoWhitespaceAfter   183
 NewLineAtEndOfFile  111
 UnusedImports   80
 MultipleVariableDeclarations78
 RegexpSingleLine46
 OneStatementPerLine 43
 RedundantImport 22
 DefaultComesLast9
 RightCurly  5
 RedundantModifier   3
 GenericWhitespace   1
 NoWhitespaceBefore  1


These are now implemented (with the checkstyle-5.5 config), but with the
following minor changes from what Vincent had proposed:

1. for ImportOrder, specified that static imports are under, not top; this
corresponds to the practice that already existed in FOP;

2. for NoWhitespaceAfter, specified that line breaks are allowed after DOT;
if this isn't done, then one cannot break a long line before *or* after DOT,
and consequently could force long lines when not desired; this because the
default settings for the NoWhitespaceBefore rule disallows a line break
before DOT;

G.


checkstyle-5.5 support, updated rules

2012-03-01 Thread Glenn Adams
In order to proceed on Vincent's proposed new checkstyle rules [1], I've
committed support for checkstyle-5.5 and the new rules [2].

[1]
http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3c4f2c1d25.8010...@gmail.com%3e
[2] http://svn.apache.org/viewvc?view=revisionrevision=1296000

More specifically, I parameterized build.xml to use either checkstyle-5.1
or checkstyle-5.5, where the (current) default is checkstyle-5.1. Which
version is used can be controlled by setting the following ant parameters:

   - checkstyle.location (default: ${lib-tools}/checkstyle-all-5.1.jar)
   - checkstyle.config (default: ${basedir}/checkstyle-5.1.xml)

I'm specifying the following in my build-local.properties in order to
select checkstyle-5.5:

checkstyle.config = ${basedir}/checkstyle-5.5.xml
checkstyle.location = ${basedir}/lib/build/checkstyle-5.5-all.jar

I manually merged the new rules proposed by Vincent with the old set of
checkstyle-5.1 rules to produce a new checkstyle-5.5.xml config file as
follows:

   - if adding a new rule did not produce new errors, i added; otherwise, i
   added a commented out rule with a comment indicating the number of new
   errors
   - if changing an existing rule did not produce new errors, i changed it;
   otherwise, i added a comment indicating the number of new errors produced
   - for existing rules not included in [1], I removed some but retained a
   few, about which see the additional notes below;
   - i reordered the rules to be alphabetical, and added some divider
   comments to make it a little easier to read the separate rules

As of this commit, both checkstyle-5.1 and -5.5 do not produce any errors;
however, in order to make full use of all the proposed new rules or changed
rules some code will need to change or the proposed new rule or rule change
dropped. An ordered list of the number of new errors that would be produced
(by rule) is as follows:

ParenPad1
MethodParamPad  4316
WhitespaceAfter 2203
ExplicitInitialization  795
ImportOrder 255
NoWhitespaceAfter   183
NewLineAtEndOfFile  111
UnusedImports   80
MultipleVariableDeclarations78
RegexpSingleLine46
OneStatementPerLine 43
RedundantImport 22
DefaultComesLast9
RightCurly  5
RedundantModifier   3
GenericWhitespace   1
NoWhitespaceBefore  1

For those rules above with less than, say 300 errors, the errors can be
addressed (in subsequent commits) and the rule enabled. For those rules
above producing more than 500 new errors, I would suggest we not attempt to
implement the rules, particularly since we don't have full consensus on
their use.

Here are my notes on the rule merge process:

ConstantName
  1. current format ^([A-Z](_?[A-Z0-9]+)*)|(log)$
  2. proposed (default) format ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$
  3. problems with current code base
 (a) requires at least one '_'
 (b) requires length  1
 (b) doesn't admit 'log'
  4. conclusion: retain existing format

DefaultComesLast
  1. produces 9 new errors

DoubleCheckedLockinng
  1. left in, even though not present in proposed rules

EmptyBlock
  1. left LITERAL_CATCH, even though removed in proposed rule
  2. changing block policy to default (stmt) instead of text produces 110
new errors

EmptyForIteratorPad
  1. removed (not present in proposed rules)

EqualsHashCode
  1. left in, even though not present in proposed rules

ExplicitInitialization
  1. produces 795 new errors

FileLength
  1. removed (not present in proposed rules)

GenericWhitespace
  1. produces 1 new error

ImportOrder
  1. produces 255 new errors

InnerAssignment
  1. left in, even though not present in proposed rules (I propose we
remove this rule, but want to confirm with the group before doing so)

Javadoc{Method,Type,Variable}
  1. removed (not present in proposed rules)

LineLength
  1. removed, as suggested by cbowditch [3]

[3]
http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3cblu0-smtp420d413c8764cc374545b5bfb...@phx.gbl%3e

MethodLength
  1. removed (not present in proposed rules)

MethodParamPad
  1. produces 4316 new errors

MultipleVariableDeclarations
  1. produces 78 new errors

NewLineAtEndOfFile
  1. produces 111 new errors

NoWhitespaceAfter
  1. changing allowLineBreaks to false produces 183 new errors

NoWhitespaceBefore
  1. changing allowLineBreaks to false produces 1 new error

OneStatementPerLine
  1. produces 43 new errors

ParameterNumber
  1. removed (not present in proposed rules)

ParenPad
  1. produces 1 new errors

RedundantImport
  1. produces 22 new errors

RedundantModifier
  1. adding INTERFACE_DEF (via default tokens) produces 3 new errors

RegexpSingleLine
  1. produces 46 new errors

RightCurly
  1. adding LITERAL_IF (via default tokens) produces 5 new