[Libvir] removing trailing blanks (and keeping them away)
Any objection to a mass removal of trailing blanks? [ If you haven't yet been adversely affected by e.g., pointless conflicts due solely to differences in white space, count yourself lucky, and chalk it up to not having to deal with many branches. The sooner we do this the better, IME. ] I'm proposing two things: - remove all trailing blanks - add a "make syntax-check" check to ensure that no new ones are added There aren't that many, but the change would affect many files. IMHO, such a change should wait until after big patches like Dan's StorageAPI changes are in. Obviously, one would not change .png and .gif files, and they would be exempted from the mechanical check. Here's the full list: tests/xmlrpctest.c:44 src/openvz_driver.c:41 src/openvz_conf.c:38 NEWS:33 src/libvirt.c:30 docs/virsh.pod:29 docs/libvir.html:24 virsh.1:21 src/xend_internal.c:19 docs/news.html:19 docs/index.py:19 docs/apibuild.py:18 src/xmlrpc.c:16 src/remote_internal.c:14 docs/devhelp/style.css:11 src/xen_internal.c:10 src/proxy_internal.c:9 src/xen_unified.c:8 proxy/libvirt_proxy.c:8 src/xs_internal.c:6 src/util.c:6 src/iptables.c:6 src/qemu_driver.c:5 src/qemu_conf.c:5 include/libvirt/libvirt.h:5 include/libvirt/libvirt.h.in:5 docs/library.xen:5 src/conf.c:4 python/generator.py:4 tests/virshtest.c:3 src/xml.c:3 src/xend_internal.h:3 src/openvz_conf.h:3 src/bridge.c:3 python/libvir.c:3 po/sv.po:3 libvirt.spec.in:3 docs/search.php:3 docs/libvirt.rng:3 docs/libvirLogo.png:3 docs/format.html:3 docs/examples/Makefile.am:3 docs/ChangeLog.xsl:3 docs/ChangeLog.awk:3 src/virterror.c:2 src/qemu.conf:2 src/driver.h:2 qemud/mdns.c:2 docs/site.xsl:2 docs/libvirHeader.png:2 docs/examples/suspend.c:2 docs/examples/python/README:2 docs/examples/examples.xsl:2 docs/Libxml2-Logo-90x34.gif:2 autogen.sh:2 TODO:2 tests/xml2sexprtest.c:1 tests/testutils.h:1 tests/test_conf.sh:1 tests/reconnect.c:1 src/xm_internal.c:1 src/virsh.c:1 src/sexpr.c:1 src/proxy_internal.h:1 src/openvz_driver.h:1 src/hash.c:1 src/buf.c:1 qemud/mdns.h:1 qemud/libvirtd.sasl:1 python/types.c:1 python/tests/uuid.py:1 python/tests/create.py:1 python/Makefile.am:1 po/ru.po:1 docs/windows.html:1 docs/windows-cygwin-3.png:1 docs/uri.html:1 docs/testnode.xml:1 docs/structures.fig:1 docs/node.fig:1 docs/news.xsl:1 docs/newapi.xsl:1 docs/libvirtLogo.png:1 docs/html/libvirt-libvirt.html:1 docs/examples/python/domstart.py:1 docs/examples/python/domsave.py:1 docs/examples/python/dominfo.py:1 docs/devhelp/Makefile.am:1 docs/architecture.fig:1 docs/Makefile.am:1 autobuild.sh:1
Jim Meyering wrote:
Any objection to a mass removal of trailing blanks?
[ If you haven't yet been adversely affected by e.g., pointless conflicts due solely to differences in white space, count yourself lucky, and chalk it up to not having to deal with many branches. The sooner we do this the better, IME. ]
I'm proposing two things: - remove all trailing blanks - add a "make syntax-check" check to ensure that no new ones are added
Yup, no problems with such a patch. Interestingly emacs perl-mode highlights trailing spaces. I looked into it, and there is a show-trailing-whitespace variable in emacs, so one might consider modifying our "standard" trailer to be: /* * Local variables: * indent-tabs-mode: nil * c-indent-level: 4 * c-basic-offset: 4 * tab-width: 4 * show-trailing-whitespace: t <--- NB * End: */ HOWEVER this doesn't work, or at least not automatically. Emacs complains: "The local variables list in foo.c contains values that may not be safe" and marks show-trailing-whitespace as one such variable. Although one can mark it as safe in your local preferences, every emacs user would have to do this. I've no idea why show-trailing-whitespace should be unsafe. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Mon, Feb 04, 2008 at 06:42:01PM +0000, Richard W.M. Jones wrote:
Jim Meyering wrote:
Any objection to a mass removal of trailing blanks?
[ If you haven't yet been adversely affected by e.g., pointless conflicts due solely to differences in white space, count yourself lucky, and chalk it up to not having to deal with many branches. The sooner we do this the better, IME. ]
I'm proposing two things: - remove all trailing blanks - add a "make syntax-check" check to ensure that no new ones are added
Yup, no problems with such a patch.
Interestingly emacs perl-mode highlights trailing spaces.
I looked into it, and there is a show-trailing-whitespace variable in emacs, so one might consider modifying our "standard" trailer to be:
/* * Local variables: * indent-tabs-mode: nil * c-indent-level: 4 * c-basic-offset: 4 * tab-width: 4 * show-trailing-whitespace: t <--- NB * End: */
HOWEVER this doesn't work, or at least not automatically. Emacs complains: "The local variables list in foo.c contains values that may not be safe" and marks show-trailing-whitespace as one such variable. Although one can mark it as safe in your local preferences, every emacs user would have to do this.
I've no idea why show-trailing-whitespace should be unsafe.
Hmm, I've got the following in my $HOME/.emacsrc file (require 'show-wspace) (add-hook 'font-lock-mode-hook 'highlight-trailing-whitespace) Which does the trick for me Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
"Daniel P. Berrange" <berrange@redhat.com> wrote:
(require 'show-wspace) (add-hook 'font-lock-mode-hook 'highlight-trailing-whitespace)
Which does the trick for me
I've found it useful to highlight space-before-TAB and blank-lines-at-EOB, too. (when window-system ;;; Create faces for highlighting white space. (copy-face 'underline 'ws-space-face) (set-face-foreground 'ws-space-face "red") (copy-face 'underline 'ws-unbreakable-space-face) (set-face-foreground 'ws-unbreakable-space-face "grey") ;;; Things to highlight in nearly every mode. (defvar always-font-lock-keywords '(("\\<\\(FIXME\\|XXX\\)[-!:]?\\>" 1 font-lock-warning-face t) ("\240" 0 'ws-unbreakable-space-face t) ("[ \t]+$" 0 'ws-space-face t) ; trailing white space ("\\( +\\)\t" 1 'ws-space-face t) ; spaces before a TAB ("\n\\(\n+\\)\\'" 1 'ws-space-face t)) ; blank lines at end of buffer "expressions to highlight in every font-lock enabled mode.") ...
On Mon, Feb 04, 2008 at 06:45:33PM +0000, Daniel P. Berrange wrote:
On Mon, Feb 04, 2008 at 06:42:01PM +0000, Richard W.M. Jones wrote:
Jim Meyering wrote: I looked into it, and there is a show-trailing-whitespace variable in emacs, so one might consider modifying our "standard" trailer to be:
/* * Local variables: * indent-tabs-mode: nil * c-indent-level: 4 * c-basic-offset: 4 * tab-width: 4 * show-trailing-whitespace: t <--- NB * End: */
HOWEVER this doesn't work, or at least not automatically. Emacs complains: "The local variables list in foo.c contains values that may not be safe" and marks show-trailing-whitespace as one such variable. Although one can mark it as safe in your local preferences, every emacs user would have to do this.
I've no idea why show-trailing-whitespace should be unsafe.
Hmm, I've got the following in my $HOME/.emacsrc file
(require 'show-wspace) (add-hook 'font-lock-mode-hook 'highlight-trailing-whitespace)
Which does the trick for me
But is useless for anyone else unless I also mention that it requires a copy of http://www.emacswiki.org/cgi-bin/wiki/show-wspace.el in your emacs lisp search path Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Mon, Feb 04, 2008 at 06:04:32PM +0100, Jim Meyering wrote:
Any objection to a mass removal of trailing blanks?
[ If you haven't yet been adversely affected by e.g., pointless conflicts due solely to differences in white space, count yourself lucky, and chalk it up to not having to deal with many branches. The sooner we do this the better, IME. ]
I'm proposing two things: - remove all trailing blanks - add a "make syntax-check" check to ensure that no new ones are added
There aren't that many, but the change would affect many files. IMHO, such a change should wait until after big patches like Dan's StorageAPI changes are in.
No need to wait for that. My patches don't really touch existing code anymore - I got the general cleanup patches all merged already - all the rest are basically just adding new code. So any merge conflicts should be fairly minimal / easy to resolve Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
"Daniel P. Berrange" <berrange@redhat.com> wrote:
No need to wait for that. My patches don't really touch existing code anymore - I got the general cleanup patches all merged already - all the rest are basically just adding new code. So any merge conflicts should be fairly minimal / easy to resolve
Good! Here's the non-mechanical part of the change: Remove all trailing blanks; turn on the rule to detect them. * Makefile.cfg (local-checks-to-skip): Remove sc_trailing_blank. * .x-sc_trailing_blank: New file, to exempt the few binary files. diff --git a/Makefile.cfg b/Makefile.cfg index dca7c5b..a9578ab 100644 --- a/Makefile.cfg +++ b/Makefile.cfg @@ -30,7 +30,6 @@ local-checks-to-skip = \ makefile-check \ sc_no_have_config_h \ sc_tight_scope \ - sc_trailing_blank \ sc_GPL_version \ sc_always_defined_macros \ sc_cast_of_alloca_return_value \ diff --git a/.x-sc_trailing_blank b/.x-sc_trailing_blank new file mode 100644 index 0000000..9c6439d --- /dev/null +++ b/.x-sc_trailing_blank @@ -0,0 +1,3 @@ +\.png$ +\.fig$ +\.gif$ The remaining 4500 lines of diffs are produced like this: git grep -l '[ ]$'|grep -Ev '\.(png|gif|fig)$' \ | xargs perl -pi -e 's/[ \t]+$//' Then, I used this to verify that nothing significant changed: git diff -b And with that, "make sc_trailing_blank" now passes. I'll commit this in an hour or two, barring objections.
participants (3)
-
Daniel P. Berrange -
Jim Meyering -
Richard W.M. Jones