11:04 a.m.
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
12:42 p.m.
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
12:45 p.m.
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 -=|
12:50 p.m.
"Daniel P. Berrange" <berrange(a)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.")
...
1:03 p.m.
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 -=|
10:32 a.m.
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 -=|
10:53 a.m.
"Daniel P. Berrange" <berrange(a)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.
6165
days inactive
6166
days old
6 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Jim Meyering -
Richard W.M. Jones