[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Change by Mark Dickinson : -- nosy: -mark.dickinson ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Change by Brett Cannon : -- nosy: -brett.cannon ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Changes by Ezio Melotti ezio.melo...@gmail.com: -- nosy: +ezio.melotti versions: +Python 3.3, Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Changes by Jesús Cea Avión j...@jcea.es: -- nosy: +jcea ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___ ___ Python-bugs-list mailing list

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Chris Jerdonek added the comment: The following commit from today is related to this issue: 815b88454e3e Remove trailing whitespace in order to silence warnings on HP-UX. -- nosy: +cjerdonek ___ Python tracker rep...@bugs.python.org

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Changes by Serhiy Storchaka storch...@gmail.com: -- nosy: +storchaka ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___ ___ Python-bugs-list

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Changes by Sandro Tosi sandro.t...@gmail.com: -- nosy: +sandro.tosi ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___ ___ Python-bugs-list

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Changes by STINNER Victor victor.stin...@haypocalc.com: -- nosy: +haypo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___ ___

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Changes by Brett Cannon br...@python.org: -- assignee: brett.cannon - ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___ ___ Python-bugs-list

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Éric Araujo mer...@netwok.org added the comment: 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

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Alexander Belopolsky belopol...@users.sourceforge.net added the comment: 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):

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Alexander Belopolsky belopol...@users.sourceforge.net added the comment: I opened issue9598 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

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Changes by Dave Malcolm dmalc...@redhat.com: -- nosy: +dmalcolm ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___ ___ Python-bugs-list

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Brett Cannon br...@python.org added the comment: On Sun, Jul 4, 2010 at 22:40, Éric Araujo rep...@bugs.python.org wrote: Éric Araujo mer...@netwok.org 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

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Brett Cannon br...@python.org added the comment: Committed in r82567. I also cleaned up the output slightly so that N file(s) pluralizes file properly based on N. Also now list what files have their whitespace fixed. -- ___ Python tracker

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Changes by Brett Cannon br...@python.org: -- status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___ ___ Python-bugs-list

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Alexander Belopolsky belopol...@users.sourceforge.net added the comment: 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);

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Éric Araujo mer...@netwok.org 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. -- ___ Python

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Éric Araujo mer...@netwok.org added the comment: 1) Okay, so I’ll refactor some code in patchcheck into a grep-like function to reduce duplication and support adding new checks. 2) I’ll add more useful report. Perhaps I’ll ask first on python-dev to see if I should add a command-line option

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Changes by Éric Araujo mer...@netwok.org: -- keywords: +needs review resolution: - accepted ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Mark Dickinson dicki...@gmail.com added the comment: I think I’ll go with replacing instead of just reporting. How about a --report-only (insert better name here) command line option to patchcheck, then? My editor doesn't delete trailing whitespace *automatically*, but it's trivial to

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Éric Araujo mer...@netwok.org added the comment: Agreed. reindent.py has a dry-run mode too, so adding such a global flag and making it the default is okay with me. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Mark Dickinson dicki...@gmail.com added the comment: Nitpick: shouldn't that len(line) 79 be len(line) 80 ? Either that, or strip the line ending from the line before computing its length. -- nosy: +mark.dickinson ___ Python tracker

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Mark Dickinson dicki...@gmail.com added the comment: 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? -- ___ Python tracker rep...@bugs.python.org

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Changes by Éric Araujo mer...@netwok.org: Removed file: http://bugs.python.org/file17672/patchcheck-pep7.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Éric Araujo mer...@netwok.org added the comment: Forgot readling kept the line end, thanks for catching. Updated (yay for Mercurial Queues!) -- Added file: http://bugs.python.org/file17673/patchcheck-pep7.diff ___ Python tracker

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Éric Araujo mer...@netwok.org added the comment: s/readling/readline/ s/readline/iterating over the file/ -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Mark Dickinson dicki...@gmail.com added the comment: 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

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Mark Dickinson dicki...@gmail.com added the comment: s/actually//g -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___ ___ Python-bugs-list

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Changes by Éric Araujo mer...@netwok.org: Removed file: http://bugs.python.org/file17673/patchcheck-pep7.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Éric Araujo mer...@netwok.org added the comment: Thanks for the helpful reviews. I have fixed the trailing whitespace check with “line[-2:-1].isspace()”, but I have a bug with my file counting. Before I go further, I’d like feedback from people using patchcheck: 1) Reindenting Python is a

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Éric Araujo mer...@netwok.org added the comment: Another one: I’d like to move the file name filtering from the worker functions into the main function. With that change, looping over all file names to get only the C files would be written and run only once, even if there are four functions

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Mark Dickinson dicki...@gmail.com added the comment: Answering as a rather infrequent user of 'make patchcheck', but someone who vows to use it more often in future... :) (1) Well, it would be awkward to use grep or wc on Windows, so it's convenient not to need external tools. (2) +1 to a

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Mark Dickinson dicki...@gmail.com added the comment: and your proposed refactoring sounds fine to me. I can't really see any backwards compatibility concerns. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Brett Cannon br...@python.org added the comment: Thanks for the patch, Éric. I will get to it when I can. -- assignee: - brett.cannon ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Éric Araujo mer...@netwok.org added the comment: 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

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Alexander Belopolsky belopol...@users.sourceforge.net added the comment: Éric, It will be helpful if your code could also check for lines longer than 79 characters. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Alexander Belopolsky belopol...@users.sourceforge.net added the comment: .. and trailing white space. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Éric Araujo mer...@netwok.org added the comment: 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. -- Added file: http://bugs.python.org/file17672/patchcheck-pep7.diff

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Changes by Éric Araujo mer...@netwok.org: Removed file: http://bugs.python.org/file17663/check-tabs.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Changes by Éric Araujo mer...@netwok.org: Removed file: http://bugs.python.org/file17664/check-tabs.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Éric Araujo mer...@netwok.org added the comment: Here’s a quick crack at it. Conservative approach: Don’t reindent, just complain. -- keywords: +patch nosy: +merwok stage: - patch review versions: +Python 2.7, Python 3.2 Added file: http://bugs.python.org/file17663/check-tabs.diff

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Changes by Éric Araujo mer...@netwok.org: -- nosy: +pitrou ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___ ___ Python-bugs-list mailing

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Éric Araujo mer...@netwok.org added the comment: Updating patch to check header files too. Untested. -- Added file: http://bugs.python.org/file17664/check-tabs.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

Changes by Alexander Belopolsky belopol...@users.sourceforge.net: -- nosy: +belopolsky ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8912 ___ ___

[issue8912] `make patchcheck` should check the whitespace of .c/.h files

New submission from Brett Cannon br...@python.org: This would be especially useful now that only spaces are used in .c/.h files and not tabs. -- messages: 107178 nosy: brett.cannon priority: low severity: normal status: open title: `make patchcheck` should check the whitespace of .c/.h