| Message ID | 20130616105050.GB4043@domone.kolej.mff.cuni.cz |
|---|---|
| State | New |
| Headers | show |
2013/6/16 Ondřej Bílka <neleai@seznam.cz>: > On Sat, Jun 15, 2013 at 05:13:31PM +0800, Chung-Ju Wu wrote: >> 2013/6/14 Joseph S. Myers <joseph@codesourcery.com>: >> > On Thu, 13 Jun 2013, Richard Biener wrote: >> > >> >> Btw, rather than these kind of patches I'd appreciate if someone would look >> >> at a simple pre(post?)-commit hook that enforces those whitespace rules. >> > >> > In the cpp testsuite we definitely want tests with bad whitespace (e.g. >> > gcc.dg/cpp/backslash*.c) and generally I think it's wrong to be changing >> > whitespace in the testsuite without an understanding of what the test is >> > testing and how the whitespace is irrelevant to that (more generally, >> > cleanups of compiler tests are suspect without such an understanding of >> > what is or is not significant in a particular test). And so you need to >> > allow addition of otherwise bad whitespace there. >> > >> > It's not obvious whether there might be other cases needing such >> > whitespace as well. >> > >> >> Either by adjusting the committed content or by rejecting the commit(?) >> > >> > I don't think hooks adjusting committed content are likely to work at all. >> > >> > -- >> > Joseph S. Myers >> > joseph@codesourcery.com >> >> By having a look at Ondřej's patch, it is nice to fix existing >> codes with proper whitespace/tab rules. >> >> But it covers too much under whole gcc source tree. >> Like Joseph said, we may accidentally change the cases >> that need bad whitespace. >> >> Perhaps we should gradually fix them? i.e. We can only fix >> leading spaces for some obvious cases (gcc/*.c gcc/c-family/*.c ...), >> leaving other source (gcc/testsuite/* gcc/config/* ...) untouched. >> > I uploaded new version that touches only gcc/*.[ch] gcc/c-family/*.[ch] > to http://kam.mff.cuni.cz/~ondra/stylepp.tar.bz2 > patches are also updated. > IMHO, the preliminary whitelist could be: gcc/*.[ch] gcc/c/*.[ch] gcc/c-family/*.[ch] gcc/common/*.[ch] gcc/cp/*.[ch] > Anyway what in gcc/config/*.c depends on leading/trailing spaces? > In general, I agree with you that all stuff under gcc/config/ and gcc/common/config/ are supposed to follow whitespace rules. But I think it would be better to have corresponding port maintainers make the decision. Your tool and patches look great to me. It helps fixup the existing codes with strong whitespace convention. But I am sorry that I don't have right to approve it. You will need some reviewers to review the patch and give the OK. If you do not receive any response to the patches within two weeks or so, you can 'ping' it with a follow-up mail to remind reviewers. :) Best regards, jasonwucj
Ping On Tue, Jun 18, 2013 at 02:56:52PM +0800, Chung-Ju Wu wrote: > 2013/6/16 Ondřej Bílka <neleai@seznam.cz>: > > On Sat, Jun 15, 2013 at 05:13:31PM +0800, Chung-Ju Wu wrote: > >> 2013/6/14 Joseph S. Myers <joseph@codesourcery.com>: > >> > On Thu, 13 Jun 2013, Richard Biener wrote: > >> > > >> >> Btw, rather than these kind of patches I'd appreciate if someone would look > >> >> at a simple pre(post?)-commit hook that enforces those whitespace rules. > >> > > >> > In the cpp testsuite we definitely want tests with bad whitespace (e.g. > >> > gcc.dg/cpp/backslash*.c) and generally I think it's wrong to be changing > >> > whitespace in the testsuite without an understanding of what the test is > >> > testing and how the whitespace is irrelevant to that (more generally, > >> > cleanups of compiler tests are suspect without such an understanding of > >> > what is or is not significant in a particular test). And so you need to > >> > allow addition of otherwise bad whitespace there. > >> > > >> > It's not obvious whether there might be other cases needing such > >> > whitespace as well. > >> > > >> >> Either by adjusting the committed content or by rejecting the commit(?) > >> > > >> > I don't think hooks adjusting committed content are likely to work at all. > >> > > >> > -- > >> > Joseph S. Myers > >> > joseph@codesourcery.com > >> > >> By having a look at Ondřej's patch, it is nice to fix existing > >> codes with proper whitespace/tab rules. > >> > >> But it covers too much under whole gcc source tree. > >> Like Joseph said, we may accidentally change the cases > >> that need bad whitespace. > >> > >> Perhaps we should gradually fix them? i.e. We can only fix > >> leading spaces for some obvious cases (gcc/*.c gcc/c-family/*.c ...), > >> leaving other source (gcc/testsuite/* gcc/config/* ...) untouched. > >> > > I uploaded new version that touches only gcc/*.[ch] gcc/c-family/*.[ch] > > to http://kam.mff.cuni.cz/~ondra/stylepp.tar.bz2 > > patches are also updated. > > > > IMHO, the preliminary whitelist could be: > gcc/*.[ch] > gcc/c/*.[ch] > gcc/c-family/*.[ch] > gcc/common/*.[ch] > gcc/cp/*.[ch] > > > Anyway what in gcc/config/*.c depends on leading/trailing spaces? > > > > In general, I agree with you that all stuff under gcc/config/ and > gcc/common/config/ are supposed to follow whitespace rules. > But I think it would be better to have corresponding port maintainers > make the decision. > > Your tool and patches look great to me. It helps fixup the existing > codes with strong whitespace convention. > But I am sorry that I don't have right to approve it. > You will need some reviewers to review the patch and give the OK. > > If you do not receive any response to the patches within two weeks or so, > you can 'ping' it with a follow-up mail to remind reviewers. :) > > > Best regards, > jasonwucj
diff --git a/gcc/.indent.on b/gcc/.indent.on new file mode 100644 index 0000000..48cdce8 --- /dev/null +++ b/gcc/.indent.on @@ -0,0 +1 @@ +placeholder diff --git a/gcc/c-family/.indent.on b/gcc/c-family/.indent.on new file mode 100644 index 0000000..48cdce8 --- /dev/null +++ b/gcc/c-family/.indent.on @@ -0,0 +1 @@ +placeholder