[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] test-exclude.c: remove unmatched #endif
From: |
Bruno Haible |
Subject: |
Re: [PATCH] test-exclude.c: remove unmatched #endif |
Date: |
Sun, 20 Feb 2011 23:38:57 +0100 |
User-agent: |
KMail/1.9.9 |
Hi Jim,
> I was surprised to find that a test did not compile.
> * tests/test-exclude.c: Remove stray #endif, left over from
> the change of a week ago.
Thanks for the fix; my mistake.
> This one was interesting in that it arose (I suspect)
> due to the fact that code upon which the offending change
> was based had misleadingly-indented cpp directives.
Yes, this was part of the problem. The other part was that blank lines
normally help to find out the start and end of semantic blocks; in this
case, the absence of blank lines misled me.
> Easiest would be to convert
> all files, regardless of owner; would anyone prefer I *not* do that?
Yes, I am opposed to it. No way to have me agree to changing more than
50 files.
10 files would be ok. In other words, the proposed change is too far
away from current practice. I could agree to something that is closer
to current practice, yet avoids some of the biggest pitfalls.
The pitfall in this case was to have a block of lines, without blank lines
in them, which had an unbalanced #if[def]/#endif count _and_ more than
one #if[def]/#endif _and_ was not using cppi style.
This is OK for me (does not present a pitfall):
#if A
#if B
foo
#endif
#endif
This is OK too:
#if A
# if B
# endif
#endif
This is OK too:
#if A
#if B
#endif
#endif
This is OK too:
#if A
#if B
foo
#endif
#endif
But this is not OK:
#if A
#if B
foo
#endif
#endif
Can cppi be extended (through a command-line option) to flag only the last case
as
improperly indented? I haven't found a way to do so within 10 minutes, so I
wrote a
tool from scratch for doing this. It has some limitations / bugs (*), but on
the gnulib
source code before the 2011-02-13 change it produces these warnings:
$ for f in `find lib -name '*.[hc]' | sort` `find tests -name '*.[hc]' |
sort`; do ./findmisindent $f; done
misindented block at lib/argp-namefrob.h:98
misindented block at lib/getopt_int.h:108
misindented block at lib/progreloc.c:144
misindented block at lib/progreloc.c:256
misindented block at lib/regexec.c:3971
misindented block at lib/vasnprintf.c:879
misindented block at lib/vasnprintf.c:4568
misindented block at tests/test-argmatch.c:29
misindented block at tests/test-exclude.c:63
I.e. to fix this, we need to touch less than 10 files, and it catches to pitfall
in tests/test-exclude.c.
(*) It does not have --help. It does not do argument checking. It uses gets().
It does
not know about comment syntax. It does not know about preprocessor directive
continuation lines.
Bruno
--
In memoriam Juliusz Bursche <http://en.wikipedia.org/wiki/Juliusz_Bursche>
findmisindent.c
Description: Text Data
- [PATCH] test-exclude.c: remove unmatched #endif, Jim Meyering, 2011/02/20
- Re: [PATCH] test-exclude.c: remove unmatched #endif,
Bruno Haible <=
- Re: [PATCH] test-exclude.c: remove unmatched #endif, Jim Meyering, 2011/02/20
- Re: preprocessor directive indentation, cppi, Bruno Haible, 2011/02/20
- Re: preprocessor directive indentation, cppi, Bruce Korb, 2011/02/20
- Re: preprocessor directive indentation, cppi, Jim Meyering, 2011/02/21
- Re: preprocessor directive indentation, Bruno Haible, 2011/02/20