This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Trailing white spaces
Type: enhancement Stage: resolved
Components: Versions: Python 3.4
process
Status: closed Resolution: duplicate
Dependencies: Superseder: `make patchcheck` should check the whitespace of .c/.h files
View: 8912
Assigned To: Nosy List: chris.jerdonek, georg.brandl, jcea, ned.deily, pitrou, serhiy.storchaka, skrah, terry.reedy
Priority: normal Keywords: patch

Created on 2012-08-03 15:40 by serhiy.storchaka, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
libffi_trailing_whitespaces.diff serhiy.storchaka, 2012-08-04 20:49 review
libmpdec_trailing_whitespaces.diff serhiy.storchaka, 2012-08-04 20:50 review
c_trailing_whitespaces.diff serhiy.storchaka, 2012-08-04 20:51 review
other_trailing_whitespaces.diff serhiy.storchaka, 2012-08-04 20:53 review
Messages (14)
msg167337 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-08-03 15:40
My editors are configured to remove trailing spaces (this is useful for removing artifacts of indentation). The flip side of this is that my patches sometimes contain unrelated trailing spaces fixes.

Trailing spaces are not significant in any CPython source file, their presence I believe mistake. Easier once we remove all spaces and then prevent the appearance of new, than constantly face to unrelated changes. I'm not attaching a patch (it is too big, over 5 MB), anyone can create it by the following commands:

    hg status -cn | tr '\n' '\0' | xargs -0 sed -i -re 's/[ \t]+$//'

It would be good if the Mercurial would had hook, which automatically remove trailing spaces or prohibit to commit patches that contain trailing spaces.
msg167349 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-08-03 19:58
Because CPython repository contains binary files too, they should be reverted:

    hg diff | sed -nre 's/^Binary file (.*) has changed/\1/p' | tr '\n' '\0' | xargs -0 hg revert
msg167350 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2012-08-03 20:16
There already is a hook in place for the main python.org repository that checks for and rejects changesets that include files with space issues:

http://hg.python.org/hooks/file/bd04c6b37749/checkwhitespace.py

You can add it to your local repo to check patches before they are pushed.
msg167357 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-08-03 21:12
Or you can use "make patchcheck" which will (hopefully) warn you of such issues.
msg167421 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-08-04 17:10
> There already is a hook in place for the main python.org repository that checks for and rejects changesets that include files with space issues:

If there is already a hook, then why do some files have spurious white space (i.e. at the end of a line)?  Is that because those issues were present prior to putting the hook in place?
msg167430 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-08-04 19:14
AFAIR the hook only applies to Python and reST files, not C files.
I think Georg wrote it, perhaps he knows the reasons.
msg167433 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-08-04 19:35
> There already is a hook in place for the main python.org repository that
> checks for and rejects changesets that include files with space issues:

Now I understand why there is no problem with .py files. But there are a lot of 
other files (including .c and .h) with trailing whitespaces.
msg167434 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-08-04 19:43
Well, I'm -0 on extending the hook to C files.
msg167438 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-08-04 20:49
I found a few files where trailing spaces are significant (patches, RTF, test data). Excluding them and three generated file (Unicode data, generating scripts should be smarter), I divided the remaining into several parts:

1) the libffi library;
2) the libmpdec library;
3) other C sources;
4) the rest of the files (mainly readme-like files and build scripts).
msg167440 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2012-08-04 20:59
-1 for making wholesale whitespace changes. It potentially makes merging harder for little benefit. Imported files from other projects should definitely not be touched. IMO, the only thing potentially worth considering is extending the existing hook to C files.  I'm +0 on that myself.
msg167444 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-08-04 21:35
I'm not against whitespace cleanup every now and then, but also -0
on a hook for C files. I think that (for C) the annoyance of having
a patch rejected because of trailing whitespace outweighs the
overall benefit.
msg167532 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-08-06 02:09
How is it more of a nuisance for C files than for .py files?
Editor 'trim trailing' tools certainly don't care.
msg167535 - (view) Author: Chris Jerdonek (chris.jerdonek) *