You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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=Noneclosed_at=<Date2017-11-28.16:02:59.465>created_at=<Date2017-11-27.16:40:18.392>labels= ['interpreter-core', '3.7']
title='Expand tabs to spaces in C files'updated_at=<Date2017-11-28.16:02:59.464>user='https://github.com/serhiy-storchaka'
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.
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).
Activity
serhiy-storchaka commentedon Nov 27, 2017
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.
gvanrossum commentedon Nov 27, 2017
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?)
gvanrossum commentedon Nov 27, 2017
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).
serhiy-storchaka commentedon Nov 27, 2017
Tools/scripts/patchcheck.py does this check. It is used for .py and .rst files, and there is bpo-8912 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).
gvanrossum commentedon Nov 27, 2017
Yeah, some editors don't clean up trailing ws. Even with Emacs it occasionally happens to me.
gvanrossum commentedon Nov 28, 2017
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.
serhiy-storchaka commentedon Nov 28, 2017
New changeset 598ceae by Serhiy Storchaka in branch 'master':
bpo-32150: Expand tabs to spaces in C files. (bpo-4583)
598ceae
serhiy-storchaka commentedon Nov 28, 2017
Thanks Guido!
I think now we can revive bpo-8912.