On Jul 14, 2009, at 8:11 AM, Marius Dumitru Florea wrote:
> Vincent Massol wrote:
>> Hi Thomas,
>>
>> On Jul 13, 2009, at 3:20 PM, tmortagne (SVN) wrote:
>>
>>> Author: tmortagne
>>> Date: 2009-07-13 15:20:03 +0200 (Mon, 13 Jul 2009)
>>> New Revision: 21910
>
> [snip]
>
>>> public void testIncludeMacroWithNewContext() throws Exception
>>> {
>>> - String expected = "beginDocument\n"
>>> - + "beginMacroMarkerStandalone [velocity] [] [$myvar]\n"
>>> - + "beginParagraph\n"
>>> - + "onWord [hello]\n"
>>> - + "endParagraph\n"
>>> - + "endMacroMarkerStandalone [velocity] [] [$myvar]\n"
>>> - + "endDocument";
>>> + String expected =
>>> + "beginDocument\n" + "beginMacroMarkerStandalone
>>> [velocity] [] [$myvar]\n" + "beginParagraph\n"
>>> + + "onWord [hello]\n" + "endParagraph\n" +
>>> "endMacroMarkerStandalone [velocity] [] [$myvar]\n"
>>> + + "endDocument";
>>
>> I much prefer my version. Can you please rollback? Again an example
>> why applying code style automatically sucks :)
>
> Why don't you use a StringBuffer?
It's more clear this way (and btw performance wise it's better - see
my other mail in response to you).
[snip]
>>> IncludeMacro macro = (IncludeMacro)
>>> getComponentManager().lookup(Macro.class, "include");
>>>
>>> mockDocumentAccessBridge
>>> .expects
>>> (once()).method("isDocumentViewable").will(returnValue(true));
>>> @@ -119,7 +120,7 @@
>>>
>>> assertBlocks(expected, blocks);
>>> }
>>> -
>>> +
>>> public void testIncludeMacroWithNoDocumentSpecified() throws
>>> Exception
>>> {
>>> IncludeMacro macro = (IncludeMacro)
>>> getComponentManager().lookup(Macro.class, "include");
>>> @@ -129,8 +130,8 @@
>>> macro.execute(parameters, null, new
>>> MacroTransformationContext());
>>> fail("An exception should have been thrown");
>>> } catch (MacroExecutionException expected) {
>>> - assertEquals("You must specify a 'document' parameter
>>> pointing to the document to include.",
>>> - expected.getMessage());
>>> + assertEquals("You must specify a 'document' parameter
>>> pointing to the document to include.", expected
>>> + .getMessage());
>>
>> again a brilliant example why it sucks... :(
>
> Can you store the long string in a separate variable (maybe a
> StringBuffer if it's really long)?
Marius, I see where you want to go but this will never work. You can't
modify code so that automatic formatting will always be perfect. If we
want to automate formatting (like with a SVN hook to automatically
apply code style - in a first commit for ex) then we'll have to accept
suboptimal result. The balance of course is vs wrongly formatted code
which is probably more the norm in our code in general.
I'm just whinning because I'm very strict with my code style and I
directly format it in a certain way that I find the best and the IDE
formatting removes my carefully crafted styles... ;)
If we could improve the IDE styles it could be ok. But I don't think
Eclipse for ex support things like:
- put braces on next line for if/for/etc only if the line is longer
than 120 chars. We could also decide to remove this rule we had voted
and (which I personally like - my preference has always been braces on
next line everywhere for increased readability)
- don't split on "." (as in the example above)
- don't split for javadoc @see (to be verified this is invalid - I
seem to recall it is)
Thanks
-Vincent
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs
