Re: [PATCH 4/7] Don't leave empty first line in C source files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 17, 2014 at 09:27:14AM -0600, Eric Blake wrote:
> On 03/17/2014 08:39 AM, Martin Kletzander wrote:
> > If there should be some sort of separator it is better to use comment
> > with the filename, copyright, description, license information and
> > authors.
> > 
> > Found by:
> > 
> > git grep -nH '^$' | grep '\.[ch]:1:'
> > 
> > Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> > ---
> > 
> > Notes:
> >     If any reviewer wants I can add a syntax-check for that.
> 
> Yes, that would be a nice followup.  We already have a check for
> trailing blank lines; can that check be extended to also cover leading
> blank lines?
> 

That's a gnulib test which might be extended, but in their codebase,
not ours.  There are few approaches that just emerged on my mind.
Since we want this to be enforced in .h files as well, we either have
to modify bracket-spacing.pl to skip .h files and feed those to it or
create another make target for this.  The most readable and
straight-forward one is probably:

empty-lines-check:
       $(AM_V_GEN)files=`$(VC_LIST) | grep '\.[hc]$$'`; \
       grep -nH -m1 '^$$' $$files | grep ':1:$$' && \
         { echo '$(ME): prohibited empty first line' 1>&2; \
           exit 1; }

the '-m1' is there to speed it up, unfortunately grep doesn't have a
parameter to scan only X lines of each file.  We can use way more
tools than this and make it better, I just don't have a preference.
Can you give me a hint what would be the best to use from e.g. awk,
perl, just shell?  Or whether we want to split this into
self-contained checking script like bracket-spacing.pl?

> 
> > +++ b/tests/xml2sexprtest.c
> > @@ -1,4 +1,3 @@
> > -
> >  #include <config.h>
> > 
> 
> ACK
> 

Pushed it without the follow up for now.  Thanks for the review.

Martin

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]