➜
This issue tracker will soon become read-only and move to GitHub.
For a smoother transition, remember to
log in and link your GitHub username to your profile.
For more information,
see this post about the migration.
Created on 2017-11-27 16:40 by serhiy.storchaka, last changed 2017-11-28 16:02 by serhiy.storchaka. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 4583 | merged | serhiy.storchaka, 2017-11-27 16:40 | |
| Messages (8) | |||
|---|---|---|---|
| msg307068 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2017-11-27 16:40 | |
While most C files use spaces for indenting and aligning, there are few sites in which tabs are left. Sometimes mixed tabs and spaces are used in the same file. This adds a noise in the diff if the committer uses an editor which doesn't preserve tabs (see for example PR 4390). This can hide semantic changes in tab expansion changes. I reviewed many patches with accidentally expanded tabs to spaces. Usually I request removing unrelated changes from the patch. But this still happened, and tabs slowly are disappeared from the sources. There are less tabs in the current default branch than in any maintained branch. I think it is better to make the expansion in a single not so large commit that allow them be mixed with semantical changes. PR 4583 does this. The changes first were made automatically, and after that I have edited them manually for removing excessive indentations in some files (4 spaces are enough). Tabs have been kept only in generated file Modules/unicodedata_db.h and in imported files Modules/_ctypes/* and Modules/expat/siphash.h. |
|||
| msg307071 - (view) | Author: Guido van Rossum (gvanrossum) * |
Date: 2017-11-27 17:33 | |
I think I'm cool with this (even though it will make blame-finding on GitHub a tad harder -- what's the magic URL appendage again?) |
|||
| msg307073 - (view) | Author: Guido van Rossum (gvanrossum) * |
Date: 2017-11-27 17:57 | |
Maybe we also need (in a separate PR) a git pre-commit hook that prevents tabs in the source (except in the few exceptions you left alone). |
|||
| msg307074 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-11-27 18:16 | |
Tools/scripts/patchcheck.py does this check. It is used for .py and .rst files, and there is issue8912 for applying it to .c/.h files. patchcheck.py also checks trailing whitespaces. While I and other core developers constantly remove trailing whitespaces (the recent clean up is PR 4130), the new trailing whitespaces are constantly added (for example in PR 4150). |
|||
| msg307075 - (view) | Author: Guido van Rossum (gvanrossum) * |
Date: 2017-11-27 19:09 | |
Yeah, some editors don't clean up trailing ws. Even with Emacs it occasionally happens to me. |
|||
| msg307134 - (view) | Author: Guido van Rossum (gvanrossum) * |
Date: 2017-11-28 15:51 | |
OK, so I think you can go ahead with this. I was fine with the state of the patch yesterday, so whatever you added today is also fine. |
|||
| msg307136 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-11-28 15:56 | |
New changeset 598ceae876ff4a23072e59945597e945583de4ab by Serhiy Storchaka in branch 'master': bpo-32150: Expand tabs to spaces in C files. (#4583) https://github.com/python/cpython/commit/598ceae876ff4a23072e59945597e945583de4ab |
|||
| msg307140 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-11-28 16:02 | |
Thanks Guido! I think now we can revive issue8912. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2017-12-09 03:11:24 | martin.panter | link | issue28185 superseder |
| 2017-11-28 16:02:59 | serhiy.storchaka | set | status: open -> closed resolution: fixed messages: + msg307140 stage: patch review -> resolved |
| 2017-11-28 15:56:12 | serhiy.storchaka | set | messages: + msg307136 |
| 2017-11-28 15:51:06 | gvanrossum | set | messages: + msg307134 |
| 2017-11-27 19:09:28 | gvanrossum | set | messages: + msg307075 |
| 2017-11-27 18:16:13 | serhiy.storchaka | set | messages: + msg307074 |
| 2017-11-27 17:57:55 | gvanrossum | set | messages: + msg307073 |
| 2017-11-27 17:33:05 | gvanrossum | set | messages: + msg307071 |
| 2017-11-27 16:40:51 | serhiy.storchaka | set | keywords:
+ patch pull_requests: + pull_request4512 |
| 2017-11-27 16:40:18 | serhiy.storchaka | create | |