whitespace patches
Locked
 
|
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 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. > > Version: GnuPG v1.4.6 (MingW32) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFJups0rP3NbaWWqE4RAlF9AKC/sK86TloFTUbRa7UZ2G8z3PumCQCePHkq +wAxCW1VoR1pgDvfoPggZEo= =GF7S -----END PGP SIGNATURE----- |
|
|
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 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----- |
|
|
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 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----- |
Administrator
|
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" <[hidden email]> > 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. |
|
|
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" <[hidden email]> >> 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 [hidden email] H1:480-626-8022 H2:480-802-7173 Yahoo IM: stephen_rufle AOL IM: stephen1rufle |
|
Administrator
|
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" <[hidden email]> > 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" <[hidden email]> >>> 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 > [hidden email] > H1:480-626-8022 > H2:480-802-7173 > Yahoo IM: stephen_rufle > AOL IM: stephen1rufle > |
|
Administrator
|
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" <[hidden email]> > 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" <[hidden email]> >> 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" <[hidden email]> >>>> 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 >> [hidden email] >> H1:480-626-8022 >> H2:480-802-7173 >> Yahoo IM: stephen_rufle >> AOL IM: stephen1rufle >> > |
|
Administrator
|
Done, please comment if any problem http://docs.ofbiz.org/x/mg
Jacques From: "Jacques Le Roux" <[hidden email]> >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" <[hidden email]> >> 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" <[hidden email]> >>> 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" <[hidden email]> >>>>> 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 >>> [hidden email] >>> H1:480-626-8022 >>> H2:480-802-7173 >>> Yahoo IM: stephen_rufle >>> AOL IM: stephen1rufle >>> >> > > |
«
Return to OFBiz - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |