"Daniel P. Berrange" <berrange@xxxxxxxxxx> 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? ----------------------------------------------------------------- Here are the interesting parts of the two changes. First, remove the 'tab-width: 4' directives and make sure they stay gone: remove all 'tab-width: 4' directives Makefile.maint (sc_prohibit_tab_width_4): New rule. Remove all of them, using this command: git grep -l 'tab-width: 4' \ | xargs perl -ni -e '/^\s*\*\s*tab-width:\s*4\s*$/ or print' Signed-off-by: Jim Meyering <meyering@xxxxxxxxxx> --- Makefile.maint | 9 +++++++++ proxy/libvirt_proxy.c | 1 - ... tests/xmconfigtest.c | 1 - 78 files changed, 9 insertions(+), 77 deletions(-) diff --git a/Makefile.maint b/Makefile.maint index a357d74..fc21bf4 100644 --- a/Makefile.maint +++ b/Makefile.maint @@ -287,6 +287,15 @@ sc_trailing_blank: { echo '$(ME): found trailing blank(s)' \ 1>&2; exit 1; } || : +sp=[ ] +# Some files used to containe a line like this: " * tab-width: 4", +# contradicting an adjacent "indent-tabs-mode: nil" directive. +# Make sure no more are introduced. +sc_prohibit_tab_width_4: + @grep -E '^$(sp)*\*$(sp)*tab-width: 4$$' $$($(VC_LIST_EXCEPT)) && \ + { echo '$(ME): found bogus "tab-width:" directive(s)' \ + 1>&2; exit 1; } || : + # Match lines like the following, but where there is only one space # between the options and the description: # -D, --all-repeated[=delimit-method] print all duplicate lines\n diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c index bed3ae8..b28a3bc 100644 --- a/proxy/libvirt_proxy.c +++ b/proxy/libvirt_proxy.c @@ -857,6 +857,5 @@ int main(void) { * indent-tabs-mode: nil * c-indent-level: 4 * c-basic-offset: 4 - * tab-width: 4 * End: */ diff --git a/python/libvir.c b/python/libvir.c index fe650e8..8ad1122 100644 --- a/python/libvir.c +++ b/python/libvir.c @@ -1464,6 +1464,5 @@ initcygvirtmod * indent-tabs-mode: nil * c-indent-level: 4 * c-basic-offset: 4 - * tab-width: 4 * End: */ ... --------------------------------------------------------------------- Then remove the offending in-indentation-only TABs from files with "indent-tabs-mode: nil": In a file with "indent-tabs-mode: nil", no TABs for indentation. Convert offenders via "expand --initial". * Makefile.maint (sc_TAB_in_indentation): New rule to prevent regression. * proxy/libvirt_proxy.c: Convert indentation TABs to spaces. * python/libvir.c, src/buf.c: src/conf.c, src/conf.h: Likewise. * src/driver.h, src/internal.h, src/libvirt.c, src/lxc_conf.c: Likewise. * src/openvz_driver.h, src/proxy_internal.c: Likewise. * src/proxy_internal.h, src/qemu_conf.c, src/qemu_driver.c: Likewise. * src/qparams.h, src/remote_internal.c, src/stats_linux.h: Likewise. * src/util.c, src/virsh.c, src/virterror.c, src/xen_internal.c: Likewise. * src/xen_unified.c, src/xen_unified.h, src/xend_internal.c: Likewise. * src/xm_internal.c, src/xml.c, src/xs_internal.c: Likewise. * tests/nodeinfotest.c, tests/sexpr2xmltest.c: Likewise. Signed-off-by: Jim Meyering <meyering@xxxxxxxxxx> --- Makefile.maint | 9 + proxy/libvirt_proxy.c | 652 ++++++++++++++++++++++++------------------------ python/libvir.c | 34 ++-- src/buf.c | 56 ++-- src/conf.c | 324 ++++++++++++------------ src/conf.h | 16 +- src/driver.h | 260 ++++++++++---------- src/internal.h | 18 +- src/libvirt.c | 174 +++++++------- src/lxc_conf.c | 34 ++-- src/openvz_driver.h | 12 +- src/proxy_internal.c | 244 +++++++++--------- src/proxy_internal.h | 26 +- src/qemu_conf.c | 2 +- src/qemu_driver.c | 10 +- src/qparams.h | 2 +- src/remote_internal.c | 8 +- src/stats_linux.h | 6 +- src/util.c | 8 +- src/virsh.c | 66 +++--- src/virterror.c | 356 ++++++++++++++-------------- src/xen_internal.c | 212 ++++++++-------- src/xen_unified.c | 52 ++-- src/xen_unified.h | 74 +++--- src/xend_internal.c | 88 ++++---- src/xm_internal.c | 50 ++-- src/xml.c | 12 +- src/xs_internal.c | 90 ++++---- tests/nodeinfotest.c | 2 +- tests/sexpr2xmltest.c | 166 +++++++------- 30 files changed, 1536 insertions(+), 1527 deletions(-) diff --git a/Makefile.maint b/Makefile.maint index fc21bf4..a22c0f5 100644 --- a/Makefile.maint +++ b/Makefile.maint @@ -296,6 +296,15 @@ sc_prohibit_tab_width_4: { echo '$(ME): found bogus "tab-width:" directive(s)' \ 1>&2; exit 1; } || : +# Ensure that each file containing an "indent-tabs-mode: nil" +# directive uses no TABs for indentation. +sc_TAB_in_indentation: + @grep -lE '^ * ' /dev/null \ + $$(grep -lE '^$(sp)*\*$(sp)*indent-tabs-mode: nil$$' \ + $$($(VC_LIST_EXCEPT))) && \ + { echo '$(ME): found TAB(s) used for indentation' \ + 1>&2; exit 1; } || : + # Match lines like the following, but where there is only one space # between the options and the description: # -D, --all-repeated[=delimit-method] print all duplicate lines\n diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c index b28a3bc..6be3ad8 100644 --- a/proxy/libvirt_proxy.c +++ b/proxy/libvirt_proxy.c @@ -93,10 +93,10 @@ proxyInitXen(void) { return(-1); } else { ret = xenHypervisorGetVersion(conn, &xenVersion); - if (ret != 0) { - fprintf(stderr, "Failed to get Xen hypervisor version\n"); - return(-1); - } + if (ret != 0) { + fprintf(stderr, "Failed to get Xen hypervisor version\n"); + return(-1); + } } ret = xenDaemonOpen_unix(conn, "/var/lib/xend/xend-socket"); if (ret < 0) { @@ -110,12 +110,12 @@ proxyInitXen(void) { } ret = xenDaemonGetVersion(conn, &xenVersion2); if (ret != 0) { - fprintf(stderr, "Failed to get Xen daemon version\n"); - return(-1); + fprintf(stderr, "Failed to get Xen daemon version\n"); + return(-1); } if (debug) fprintf(stderr, "Connected to hypervisor %lu and daemon %lu\n", - xenVersion, xenVersion2); + xenVersion, xenVersion2); if (xenVersion2 > xenVersion) xenVersion = xenVersion2; return(0); ... -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list