GeezerVison is showing github.com — `make patchcheck` should check the whitespace of .c/.h files · Issue #53158 · python/cpython
make patchcheck should check the whitespace of .c/.h files #53158
Description
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = None
created_at = <Date 2010-06-06.03:24:37.866>
labels = ['type-feature']
title = '`make patchcheck` should check the whitespace of .c/.h files'
updated_at = <Date 2020-02-03.21:38:44.046>
user = 'https://github.com/brettcannon' bugs.python.org fields:
activity = <Date 2020-02-03.21:38:44.046>
actor = 'mark.dickinson'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2010-06-06.03:24:37.866>
creator = 'brett.cannon'
dependencies = []
files = ['17741']
hgrepos = []
issue_num = 8912
keywords = ['patch', 'needs review']
message_count = 29.0
messages = ['107178', '107754', '107762', '107817', '107824', '107831', '107832', '107844', '107849', '107850', '107853', '107854', '107856', '107857', '107861', '107862', '107864', '107865', '108362', '108367', '108369', '109269', '109296', '109302', '109312', '113847', '113848', '113851', '169585']
nosy_count = 10.0
nosy_names = ['jcea', 'belopolsky', 'pitrou', 'vstinner', 'ezio.melotti', 'eric.araujo', 'dmalcolm', 'sandro.tosi', 'chris.jerdonek', 'serhiy.storchaka']
pr_nums = []
priority = 'low'
resolution = 'accepted'
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue8912'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3', 'Python 3.4'] Activitybrettcannon commented on Jun 6, 2010 This would be especially useful now that only spaces are used in .c/.h files and not tabs. merwok commented on Jun 13, 2010 Here’s a quick crack at it. Conservative approach: Don’t reindent, just complain. merwok commented on Jun 13, 2010 Updating patch to check header files too. Untested. brettcannon commented on Jun 14, 2010 Thanks for the patch, Éric. I will get to it when I can. merwok commented on Jun 14, 2010 I may add that although there is no formal test of any kind, I added tabs in two places in one .c file and in one place in another place, and it correctly reported two files. It’s a small function mostly copied from the one above; the only part where I used my brain is the counter (Python did the rest), so it should be ok. abalkin commented on Jun 14, 2010 Éric,
It will be helpful if your code could also check for lines longer than 79 characters. abalkin commented on Jun 14, 2010 .. and trailing white space. merwok commented on Jun 14, 2010 Replacing VCS hooks, are we? :) Updated and attached.
Ideas for other feature requests:
- Printing the name of files with issues;
- Fail-fast mode or full report.
mdickinson commented on Jun 15, 2010 Nitpick: shouldn't that
be
? Either that, or strip the line ending from the line before computing its length. mdickinson commented on Jun 15, 2010 Similarly, isn't
line[-1].isspace()
always going to be true (well, except possibly at the end of the file), thanks to the line-ending character? merwok commented on Jun 15, 2010 Forgot readling kept the line end, thanks for catching. Updated (yay for Mercurial Queues!) merwok commented on Jun 15, 2010 s/readling/readline/
s/readline/iterating over the file/ mdickinson commented on Jun 15, 2010 Sorry to be nitpicky, but:
(a) the line[-2] produces "IndexError: string index out of range" on empty
lines, and
(b) this won't detect trailing whitespace on the last line of a file, if there's no newline at the end of the file. Actually, it might also be worth checking that there *is* a newline at the end of the file; this is actually required by the C standards. (E.g., C99 5.1.1.2p2: "A source file that is not empty shall end in a new-line character".) abalkin commented on Jul 5, 2010 Hmm, it looks like patchcheck does not fix trailing whitespace in c files:
$ grep ' $' Modules/datetimemodule.c| cat -ve
$
GET_TD_DAYS(offset1) * 86400 - $
Py_DECREF(offset); $
if ((offset1 != offset2) && $
$
$ make patchcheck
Modules/Setup.dist is newer than Modules/Setup;
check to make sure you have all the updates you
need in your Modules/Setup file.
Usually, copying Modules/Setup.dist to Modules/Setup will work.
-----------------------------------------------
./python.exe ./Tools/scripts/patchcheck.py
Getting the list of files that have been added/changed ... 1 file
Fixing whitespace ... 0 files
Docs modified ... NO
Misc/ACKS updated ... NO
Misc/NEWS updated ... NO
Did you run the test suite? merwok commented on Jul 5, 2010 Sorry if I was unclear. Out of the four points I listed, this patch only did the refactoring (4). I have another patch to add and use a check_pep7 function, I’ll refresh it and post it. brettcannon commented on Jul 5, 2010 On Sun, Jul 4, 2010 at 22:40, Éric Araujo <[email protected]> wrote:
Éric Araujo <[email protected]> added the comment:
Sorry if I was unclear. Out of the four points I listed, this patch only did the refactoring (4). I have another patch to add and use a check_pep7 function, I’ll refresh it and post it.
You were clear, I just prematurely closed it by accident. merwok commented on Aug 13, 2010 I wonder how to have sane code reuse between patchcheck and Mercurial hooks. Mark seems to have started seconding Dirkjan with hooks, see e.g.
http://hg.python.org/hooks/rev/0344575ad60e, so I wonder if we’re going to use only hooks (pro: they will be invoked on every push, whereas patchcheck is opt-in, con: they would require that contributors clone the hooks repo and set them up) or still maintain patchcheck, which has pros (no setup, does not require a VCS) and cons (not automatic, not widely used, whereas hooks can be part of the new python-dev policy). abalkin commented on Aug 13, 2010 Note directly related to this issue, but untabify.py fails on files that contain non-ascii characters. For example:
$ ./python.exe Tools/scripts/untabify.py Modules/_heapqmodule.c
Traceback (most recent call last):
...
(result, consumed) = self._buffer_decode(data, self.errors, final)
UnicodeDecodeError: 'utf8' codec can't decode byte 0xe7 in position 173: invalid continuation byteI am not sure what relevant C standard has to say about using non-ascii characters in comments, but the checking tool should not fail with a traceback in such situation. abalkin commented on Aug 13, 2010 I opened bpo-9598 for the untabify bug. For the purposes of source checking, I believe non-ascii characters should be disallowed in python C source code. According to my understanding of C89 standard, interpretation of characters outside of the basic character set is implementation and locale dependent. cjerdonek commented on Aug 31, 2012 The following commit from today is related to this issue: 815b88454e3e
"Remove trailing whitespace in order to silence warnings on HP-UX." StanFromIreland commented on Apr 23, 2025 skirpichev commented on Apr 23, 2025 make patchcheck is still here
hugovk commented on Jun 26, 2025 patchcheck.py is still around, but whitespace checks have been removed from it (which doesn't run in CI), and added to pre-commit (run in CI), including .c and .h files.
We also have a EditorConfig file, I recommend enabling this in your editor.
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|