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

Re: [Libvir] proposal: remove contradictory indentation directive



Jim Meyering <jim meyering net> wrote:
> "Daniel P. Berrange" <berrange redhat com> wrote:
>> On Wed, Apr 09, 2008 at 02:40:13PM +0100, Richard W.M. Jones wrote:
>>> On Wed, Apr 09, 2008 at 02:43:22PM +0200, Jim Meyering wrote:
>>> > Please don't add the "tab-width: 4" specifier.
>>> > Specifying a tab-width at all in a new file with "indent-tabs-mode: nil"
>>> > is a contradiction.  The latter says there should be no TABs, yet
>>> > the former says "when there are, give them width 4."  Coding style
>>> > guidelines are universal in their recommendations to stick with 8-byte
>>> > TAB stops, independent of whether you actually use TAB or spaces.
>>>
>>> Agreed, good idea.
>>
>> ACK.
>>
>> Could we get a make syntax-check test to look for these bogus tabs too
>
> I've done that.
> Abbreviated patches below.
>
> However, there are some "issues" to consider.
> Any global space-changing delta like these is going to cause
> trouble (conflicts) for people with pending changes and on branches.
> This one isn't too bad (as these things go), since the TAB-to-space
> change affects fewer than 1500 lines in 37 files.  However, the
> new rule enforces the coding standard only in files with an existing
> "indent-tabs-mode: nil" directive.  There are currently 42 .[ch] files
> that don't have such a directive (excluding gnulib/).
>
>     $ git ls-files|grep -E '\.[ch]$'|xargs grep -L indent-tabs-mode|grep -v \
>       gnulib|wc -l
>     42
>
> If I were to do the same for those remaining files,
> the TAB-to-space change would modify an additional 2817 lines
> in 28 files:
>
>     $ git ls-files|grep -E '\.[ch]$'|xargs grep -L indent-tabs-mode|grep -v \
>     gnulib|xargs grep -E '^ *	'|wc -l
>     2817
>     $ git ls-files|grep -E '\.[ch]$'|xargs grep -L indent-tabs-mode|grep -v \
>     gnulib|xargs grep -El '^ *	'|wc -l
>     28
>
> My opinion is that if it's worth doing the first, it's also worth
> finishing the job, ... but then I don't have any huge re-architecting
> changes in my queue.
>
> However, I don't want to overstate the case either.
> The cost of a few space-change-provoked merges is pretty low
> in the grand scheme of things.
>
> What say you?
> Convert 'em all?

Sounds like we all agree, so here goes:  four change-sets:
Here are their ChangeLog entries.
I'll post them separately.

    * HACKING: New file: begin to describe contributor/coding guidelines.

    Ensure that no C source file uses TABs for indentation.
    * Makefile.maint (sc_TAB_in_indentation): New rule.

    Convert TAB-based indentation in .[ch] files to use only spaces.
    Done using this command (also includes .c.in and .h.in files):
    for i in $(g ls-files|grep -E '\.[ch](\.in)?$'|grep -v gnulib); do
      expand -i $i > j && mv j $i;done

    Remove all vim: and emacs local variable settings in .c and .h files.
    Done with these commands:
    git grep -l Local.variab|xargs \
      perl -0x3b -pi -e 's,\n+/\*\n \* vim:(.|\n)*,\n,'
    git grep -l Local.variab|xargs \
      perl -0x3b -pi -e 's,\n+/\*\n \* Local variables:\n(.|\n)*,\n,'


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