You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Adam Heath <do...@brainfood.com> on 2009/03/13 18:14:41 UTC
whitespace patches
How do we feel about whitespace patches to java code? I've added "let
java_space_errors=1" to my $HOME/.vimrc, so when I use vim, I see bad
spaces highlighted in red. The things that get highlighed are
trailing space or tab, lines that *only* contain space, and mixed
space/tab at the beginning of a line.
Fix this is semi-automatic. Trailing space and only space are fixed with:
==
find -name '*.java' | xargs sed -i -e 's/\([^\s]\)\s\+$/\1/;s/^[\s]\+$//'
==
Finding files with mixed space/tab is done with:
==
(tab=$(printf '\t'); find -name '*.java' | xargs egrep
"^( +$tab|$tab+ )" -l)
==
If given the ok, I'll start fixing this minor issue. However, it
could cause conflicts with anyone else who has uncommitted changes.
ps: The patch against framework/base to fix these issues is 260k in
size, 981 insertions/deletions, and only deals with java files.
Re: whitespace patches
Posted by Jacques Le Roux <ja...@les7arts.com>.
Done, please comment if any problem http://docs.ofbiz.org/x/mg
Jacques
From: "Jacques Le Roux" <ja...@les7arts.com>
>I was ready to change/refactor this page, notably, in the 1st section, I will suggest to use Anyedit plugin in Eclipse for
>trailing
> spaces.
> But I'd like to make a proposition before. Like Adrian (http://markmail.org/thread/sesxygbc6kj5r2yc) I really prefer to use 2
> spaces
> indentation in FTL, could we change our conventions to use 2 spaces in FTL/HTML files ?
>
> Also I'd like to know if we should use 2 spaces for XML. There are plenty of files where 2 spaces is used (notably in *model*.xml
> files) and I found this often annoying when dealing whith those files (parching, merging, commiting changes, etc.).
> IMO, 4 space should be olny used in java files and groovy, and 2 spaces everywhere else (but I may miss some types of files to be
> put in the 4 spaces group, if you see one please react)
>
> Jacques
>
> From: "Jacques Le Roux" <ja...@les7arts.com>
>> Yes, good point. I will wait to see Adam's progress... If nobody see a problem with the changes he proposed (seems OK nobody
>> growled)
>>
>> Jacques
>>
>> From: "Stephen Rufle" <sr...@salmonllc.com>
>>> If this the removal of trailing spaces is done, would it be good to also
>>> update your comment on
>>> http://docs.ofbiz.org/display/OFBADMIN/Coding+Conventions
>>>
>>> Change the AnyEdit plug in section to have "Remove trailing whitespace"
>>> to true
>>>
>>>
>>> Jacques Le Roux wrote:
>>>> I did something like that sometimes ago, but only to transform tabs
>>>> into spaces. I'm not against cleaning this aspect of the code. We
>>>> could do it now, I mean before freezing 9.3, since there has been
>>>> already a lot of changes recently
>>>> So +1
>>>>
>>>> Jacques
>>>>
>>>> From: "BJ Freeman" <bj...@free-man.net>
>>>>> sorry missed that part
>>>>>
>>>>> Adam Heath sent the following on 3/13/2009 11:19 AM:
>>>>>> BJ Freeman wrote:
>>>>>>> My thought was the it could be run before patches were made.
>>>>>>
>>>>>> Sure, but we have tons of existing code that isn't compliant, that
>>>>>> needs to be fixed. I was asking whether I should do that now, or wait
>>>>>> for a bit for others to speak up.
>>>>
>>>>
>>>>
>>>>
>>>
>>> --
>>> Stephen P Rufle
>>> srufle@salmonllc.com
>>> H1:480-626-8022
>>> H2:480-802-7173
>>> Yahoo IM: stephen_rufle
>>> AOL IM: stephen1rufle
>>>
>>
>
>
Re: whitespace patches
Posted by Jacques Le Roux <ja...@les7arts.com>.
I was ready to change/refactor this page, notably, in the 1st section, I will suggest to use Anyedit plugin in Eclipse for trailing
spaces.
But I'd like to make a proposition before. Like Adrian (http://markmail.org/thread/sesxygbc6kj5r2yc) I really prefer to use 2 spaces
indentation in FTL, could we change our conventions to use 2 spaces in FTL/HTML files ?
Also I'd like to know if we should use 2 spaces for XML. There are plenty of files where 2 spaces is used (notably in *model*.xml
files) and I found this often annoying when dealing whith those files (parching, merging, commiting changes, etc.).
IMO, 4 space should be olny used in java files and groovy, and 2 spaces everywhere else (but I may miss some types of files to be
put in the 4 spaces group, if you see one please react)
Jacques
From: "Jacques Le Roux" <ja...@les7arts.com>
> Yes, good point. I will wait to see Adam's progress... If nobody see a problem with the changes he proposed (seems OK nobody
> growled)
>
> Jacques
>
> From: "Stephen Rufle" <sr...@salmonllc.com>
>> If this the removal of trailing spaces is done, would it be good to also
>> update your comment on
>> http://docs.ofbiz.org/display/OFBADMIN/Coding+Conventions
>>
>> Change the AnyEdit plug in section to have "Remove trailing whitespace"
>> to true
>>
>>
>> Jacques Le Roux wrote:
>>> I did something like that sometimes ago, but only to transform tabs
>>> into spaces. I'm not against cleaning this aspect of the code. We
>>> could do it now, I mean before freezing 9.3, since there has been
>>> already a lot of changes recently
>>> So +1
>>>
>>> Jacques
>>>
>>> From: "BJ Freeman" <bj...@free-man.net>
>>>> sorry missed that part
>>>>
>>>> Adam Heath sent the following on 3/13/2009 11:19 AM:
>>>>> BJ Freeman wrote:
>>>>>> My thought was the it could be run before patches were made.
>>>>>
>>>>> Sure, but we have tons of existing code that isn't compliant, that
>>>>> needs to be fixed. I was asking whether I should do that now, or wait
>>>>> for a bit for others to speak up.
>>>
>>>
>>>
>>>
>>
>> --
>> Stephen P Rufle
>> srufle@salmonllc.com
>> H1:480-626-8022
>> H2:480-802-7173
>> Yahoo IM: stephen_rufle
>> AOL IM: stephen1rufle
>>
>
Re: whitespace patches
Posted by Jacques Le Roux <ja...@les7arts.com>.
Yes, good point. I will wait to see Adam's progress...
If nobody see a problem with the changes he proposed (seems OK nobody growled)
Jacques
From: "Stephen Rufle" <sr...@salmonllc.com>
> If this the removal of trailing spaces is done, would it be good to also
> update your comment on
> http://docs.ofbiz.org/display/OFBADMIN/Coding+Conventions
>
> Change the AnyEdit plug in section to have "Remove trailing whitespace"
> to true
>
>
> Jacques Le Roux wrote:
>> I did something like that sometimes ago, but only to transform tabs
>> into spaces. I'm not against cleaning this aspect of the code. We
>> could do it now, I mean before freezing 9.3, since there has been
>> already a lot of changes recently
>> So +1
>>
>> Jacques
>>
>> From: "BJ Freeman" <bj...@free-man.net>
>>> sorry missed that part
>>>
>>> Adam Heath sent the following on 3/13/2009 11:19 AM:
>>>> BJ Freeman wrote:
>>>>> My thought was the it could be run before patches were made.
>>>>
>>>> Sure, but we have tons of existing code that isn't compliant, that
>>>> needs to be fixed. I was asking whether I should do that now, or wait
>>>> for a bit for others to speak up.
>>
>>
>>
>>
>
> --
> Stephen P Rufle
> srufle@salmonllc.com
> H1:480-626-8022
> H2:480-802-7173
> Yahoo IM: stephen_rufle
> AOL IM: stephen1rufle
>
Re: whitespace patches
Posted by Stephen Rufle <sr...@salmonllc.com>.
If this the removal of trailing spaces is done, would it be good to also
update your comment on
http://docs.ofbiz.org/display/OFBADMIN/Coding+Conventions
Change the AnyEdit plug in section to have "Remove trailing whitespace"
to true
Jacques Le Roux wrote:
> I did something like that sometimes ago, but only to transform tabs
> into spaces. I'm not against cleaning this aspect of the code. We
> could do it now, I mean before freezing 9.3, since there has been
> already a lot of changes recently
> So +1
>
> Jacques
>
> From: "BJ Freeman" <bj...@free-man.net>
>> sorry missed that part
>>
>> Adam Heath sent the following on 3/13/2009 11:19 AM:
>>> BJ Freeman wrote:
>>>> My thought was the it could be run before patches were made.
>>>
>>> Sure, but we have tons of existing code that isn't compliant, that
>>> needs to be fixed. I was asking whether I should do that now, or wait
>>> for a bit for others to speak up.
>
>
>
>
--
Stephen P Rufle
srufle@salmonllc.com
H1:480-626-8022
H2:480-802-7173
Yahoo IM: stephen_rufle
AOL IM: stephen1rufle
Re: whitespace patches
Posted by Jacques Le Roux <ja...@les7arts.com>.
I did something like that sometimes ago, but only to transform tabs into spaces. I'm not against cleaning this aspect of the code.
We could do it now, I mean before freezing 9.3, since there has been already a lot of changes recently
So +1
Jacques
From: "BJ Freeman" <bj...@free-man.net>
> sorry missed that part
>
> Adam Heath sent the following on 3/13/2009 11:19 AM:
>> BJ Freeman wrote:
>>> My thought was the it could be run before patches were made.
>>
>> Sure, but we have tons of existing code that isn't compliant, that
>> needs to be fixed. I was asking whether I should do that now, or wait
>> for a bit for others to speak up.
Re: whitespace patches
Posted by BJ Freeman <bj...@free-man.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
sorry missed that part
Adam Heath sent the following on 3/13/2009 11:19 AM:
> BJ Freeman wrote:
>> My thought was the it could be run before patches were made.
>
> Sure, but we have tons of existing code that isn't compliant, that
> needs to be fixed. I was asking whether I should do that now, or wait
> for a bit for others to speak up.
>
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFJuqShrP3NbaWWqE4RAg6bAKC23oiAuTst/lipADUaPaDcannM9gCfYID8
OJRQ6OiUQdYI1KUu6AyBvTo=
=gUWe
-----END PGP SIGNATURE-----
Re: whitespace patches
Posted by Adam Heath <do...@brainfood.com>.
BJ Freeman wrote:
> My thought was the it could be run before patches were made.
Sure, but we have tons of existing code that isn't compliant, that
needs to be fixed. I was asking whether I should do that now, or wait
for a bit for others to speak up.
Re: whitespace patches
Posted by BJ Freeman <bj...@free-man.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
My thought was the it could be run before patches were made.
Adam Heath sent the following on 3/13/2009 10:52 AM:
> BJ Freeman wrote:
>> could we add something like that to build.xml -> <target name="scriptfix">
>
> Hmm, good point, adding it somewhere to allow it to be redone in the
> future. I'll keep that in mind.
>
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFJuqMkrP3NbaWWqE4RAsooAJ9ccWXlB0KmQpqYsGskzbS+lnXfYQCgiuKX
NZfeNP+cePSes66vAayJx1k=
=OUKC
-----END PGP SIGNATURE-----
Re: whitespace patches
Posted by Adam Heath <do...@brainfood.com>.
BJ Freeman wrote:
> could we add something like that to build.xml -> <target name="scriptfix">
Hmm, good point, adding it somewhere to allow it to be redone in the
future. I'll keep that in mind.
Re: whitespace patches
Posted by BJ Freeman <bj...@free-man.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
could we add something like that to build.xml -> <target name="scriptfix">
Adam Heath sent the following on 3/13/2009 10:14 AM:
> How do we feel about whitespace patches to java code? I've added "let
> java_space_errors=1" to my $HOME/.vimrc, so when I use vim, I see bad
> spaces highlighted in red. The things that get highlighed are
> trailing space or tab, lines that *only* contain space, and mixed
> space/tab at the beginning of a line.
>
> Fix this is semi-automatic. Trailing space and only space are fixed with:
>
> ==
> find -name '*.java' | xargs sed -i -e 's/\([^\s]\)\s\+$/\1/;s/^[\s]\+$//'
> ==
>
> Finding files with mixed space/tab is done with:
>
> ==
> (tab=$(printf '\t'); find -name '*.java' | xargs egrep
> "^( +$tab|$tab+ )" -l)
> ==
>
> If given the ok, I'll start fixing this minor issue. However, it
> could cause conflicts with anyone else who has uncommitted changes.
>
> ps: The patch against framework/base to fix these issues is 260k in
> size, 981 insertions/deletions, and only deals with java files.
>
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFJups0rP3NbaWWqE4RAlF9AKC/sK86TloFTUbRa7UZ2G8z3PumCQCePHkq
+wAxCW1VoR1pgDvfoPggZEo=
=GF7S
-----END PGP SIGNATURE-----