Skip to content

make patchcheck: Allow Tabs in Some Cases #92266

Closed as not planned
Closed as not planned
make patchcheck: Allow Tabs in Some Cases#92266
Labels
buildThe build process and cross-buildtype-featureA feature request or enhancement

Description

@ericsnowcurrently
ericsnowcurrently
opened on May 3, 2022

Currently Tools/patchcheck/reindent.py1 replaces all tabs with spaces. However, in Tools/c-analyzer/cpython._parser.py there are embedded tab-based tables. Up to now I've basically ignored CI failures (from make patchcheck) in _parser.py, but I'd rather reindent.py be a little smarter (even if just a whitelist). 2 Tools/scripts/untabify.py sounds like it may have a similar story.

Linked PRs

Footnotes

  1. reindent.check() is used by Tools/patchcheck/patchcheck.py (AKA make patchcheck). ↩

  2. Addressing trailing-tabs-in-str-literals is even more relevant since I enabled the check-c-globals CI check. For example, see https://github.com/python/cpython/pull/102735#issuecomment-1470516458. ↩

Activity

  1. ericsnowcurrently
    changed the title [-]make patchcheck: Allow tabs in some cases.[/-] [+]make patchcheck: Allow Tabs in Some Cases[/+] on May 3, 2022

  2. serhiy-storchaka commented on May 4, 2022

    @serhiy-storchaka
    serhiy-storchaka
    on May 4, 2022
    Member

    Why not to use \t instead of literal tab?

  3. ezio-melotti commented on May 4, 2022

    @ezio-melotti
    ezio-melotti
    on May 4, 2022
    Member

    I think this is the mentioned table:

    cpython/Tools/c-analyzer/cpython/_parser.py

    Lines 130 to 141 in d20bb33

    MACROS = clean_lines('''
    # @begin=tsv@
    glob name value
    Include/internal/*.h Py_BUILD_CORE 1
    Python/**/*.c Py_BUILD_CORE 1
    Parser/**/*.c Py_BUILD_CORE 1
    Objects/**/*.c Py_BUILD_CORE 1
    Modules/_asynciomodule.c Py_BUILD_CORE 1

    Here \ts would make it less readable. Would it make sense to move that to a stand-alone .tsv file?

  4. ericsnowcurrently commented on May 5, 2022

    @ericsnowcurrently
    ericsnowcurrently
    on May 5, 2022
    MemberAuthor

    Here \ts would make it less readable.

    Correct.

    Would it make sense to move that to a stand-alone .tsv file?

    I considered that but didn't have time to try it out. I'd rather have that particular data stay in the current file if possible though. One alternative is to switch to csv. I don't recall that these specific tables need tsv.

  5. ericsnowcurrently commented on Mar 16, 2023

    @ericsnowcurrently
    ericsnowcurrently
    on Mar 16, 2023
    MemberAuthor

    CC @AlexWaygood

  6. iritkatriel
    added
    buildThe build process and cross-build
    on Nov 27, 2023

  7. hugovk commented on Jun 26, 2025

    @hugovk
    hugovk
    on Jun 26, 2025
    Member

    Up to now I've basically ignored CI failures (from make patchcheck) in _parser.py

    Tools/patchcheck/reindent.py is no longer run as part of make patchcheck, and patchcheck is no longer run on the CI.

    reindent.py includes a few lints:

    • strip trailing whitespace (now done by trailing-whitespace in CI via pre-commit)
    • replace tabs with spaces
    • reindent to multiples of four-space indents

    PR #135983 re-adds the second of these to the CI via pre-commit.

    but I'd rather reindent.py be a little smarter (even if just a whitelist).

    PR #135984 adds Tools/c-analyzer/cpython/_parser.py to an allowlist in reindent.py so its tabs are not changed. But as reindent.py is no longer part of the CI, and therefore less often run, this is less pressing.

    Tools/scripts/untabify.py sounds like it may have a similar story.

    It does. We could easily add an allowlist here too, possibly shared with reindent.py. However, this is not run by the CI, by any make commands, nor is recommended by the devguide, so I've not changed it.

  8. hugovk
    added a commit that references this issue on Jun 27, 2025

    gh-92266: Replace tabs with four spaces in Python files (#135983)

    de0d014
  9. miss-islington
    added 2 commits that reference this issue on Jun 27, 2025

    pythongh-92266: Replace tabs with four spaces in Python files (python…

    ba155d1

    pythongh-92266: Replace tabs with four spaces in Python files (python…

    1ef6e99
  10. hugovk
    added 2 commits that reference this issue on Jun 27, 2025

    [3.14] gh-92266: Replace tabs with four spaces in Python files (GH-13…

    7e4637e

    [3.13] gh-92266: Replace tabs with four spaces in Python files (GH-13…

    1e972c7
  11. picnixz
    added
    type-featureA feature request or enhancement
    on Jun 27, 2025
  12. AndPuQing
    added a commit that references this issue on Jul 11, 2025

    pythongh-92266: Replace tabs with four spaces in Python files (python…

    9f48f74
  13. Pranjal095
    added a commit that references this issue on Jul 12, 2025

    pythongh-92266: Replace tabs with four spaces in Python files (python…

    0659dcb
  14. taegyunkim
    added a commit that references this issue on Aug 4, 2025

    pythongh-92266: Replace tabs with four spaces in Python files (python…

    2d7d981
  15. Agent-Hellboy
    added a commit that references this issue on Aug 19, 2025

    pythongh-92266: Replace tabs with four spaces in Python files (python…

    4b616bb
  16. AA-Turner
    added a commit that references this issue on Sep 17, 2025

    GH-92266: Remove embedded tabs from ``c-analyzer/cpython/_parser.py`` (…

    2212ae5

  17. AA-Turner commented on Sep 17, 2025

    @AA-Turner
    AA-Turner
    on Sep 17, 2025
    Member

    The tabs have been removed from Tools/c-analyzer/cpython/_parser.py & there are no other instances, as far as I'm aware. Going forwards, I think we should just avoid embedded tabs.

    A

  18. AA-Turner
    closed this as not plannedon Sep 17, 2025
  19. hugovk
    added a commit that references this issue on Oct 1, 2025

    gh-92266: Update pre-commit (#139411)

    800d856
  20. miss-islington
    added 2 commits that reference this issue on Oct 1, 2025

    pythongh-92266: Update pre-commit (pythonGH-139411)

    bb18c88

    pythonGH-92266: Remove embedded tabs from ``c-analyzer/cpython/_parse…

    df89f5f
  21. AA-Turner
    added a commit that references this issue on Oct 7, 2025

    [3.14] GH-92266: Remove embedded tabs from ``c-analyzer/cpython/_pars…

    db8b943
  22. hugovk
    added 2 commits that reference this issue on Oct 8, 2025

    [3.14] gh-92266: Update pre-commit (GH-139411) (#139466)

    297157d

    [3.13] pythongh-92266: Update pre-commit (pythonGH-139411)

    fea7b7c
  23. hugovk
    added a commit that references this issue on Oct 27, 2025

    [3.13] gh-92266: Update pre-commit (GH-139411) (#139760)

    2d29aa2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    buildThe build process and cross-buildtype-featureA feature request or enhancement

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions