pre-commit should enforce 79 char limit for docstrings and comments

[Mike Edmunds]

Django's coding style has long required ​limiting docstrings and comments to 79 characters, while allowing a larger 88-char limit for code lines. (The latter matches Black's default.)

Currently, only the 88 char limit is enforced by pre-commit linting, via flake8. The 79 char limit is manually enforced during PR review.

Early versions of flake8 supported only a single max-line-length limit that applied to both code and comments/docstrings. flake8 3.7.8 (2019-07-08) added a separate max-doc-length configuration option.

Django should set that option so reviewers and contributors don't need to spend time cycling on line length.

[Jacob Walls]

To follow up on a question asked in the PR, the existing files are fixed in ​https://github.com/django/django/pull/19549. There was some ​discussion about pre-commit hooks there as well. David, what do you think of this proposal -- can flake8 handle the complexity I see in check_line_too_long_django() in your PR?

[Mike Edmunds]

This issue is about the max line length for triple-quoted """docstrings""" and # comment lines in .py files in the django/ and tests/ directories. (The PR Jacob linked is for reStructuredText .txt files in the docs/ directory, which have a similar length restriction but are otherwise unrelated to this issue. I'm not sure what the correct category is, but it's not "Documentation".)

Right now, Django's Linters CI job and pre-commit checks enforce only the 88-char overall limit in .py files. The smaller 79-char limit for docstrings and comments is enforced manually during PR review, if the reviewer happens to notice it. This creates extra hurdles and noise for both reviewers and contributors.

Unfortunately, the limit for docstrings and comments hasn't been consistently enforced, and there are over 1300 cases where Django's existing .py code doesn't comply with its own style requirements (see ​https://github.com/django/django/pull/19627#issuecomment-3050890027).

So setting flake8 max-doc-length = 79 would require fixing all those exceptions first, or grandfathering them in (e.g., by listing the problem files in .flake8 per-file-ignores).

An alternative might be adding args: ["--max-docs-length=79"] to the flake8 section of pre-commit-config.yaml (without changing the global .flake8 config). That would make pre-commit enforce the limit on modified files, without needing to address all the existing violations. The downside is it only works for people who have configured pre-commit, and the CI Linters job wouldn't catch the problem.

[Mike Edmunds]

Discussion: ​https://forum.djangoproject.com/t/enforce-or-eliminate-comment-docstring-line-length-limit/41760

[Jacob Walls]

Appreciate the clarification: that was hasty of me to link that PR, as well as to not check for related tickets first.

Previous discussion about auto-fixers for docstrings can be found #25263 (wontfix/Documentation). (Core(Other) also seems reasonable, since we have other flake8 tickets there.) It might make sense to treat this as a duplicate like #27668 pending the forum discussion (thanks for starting one!)

An alternative might be adding args: --max-docs-length=79 to the flake8 section of pre-commit-config.yaml (without changing the global .flake8 config). That would make pre-commit enforce the limit on modified files, without needing to address all the existing violations.

I doubt this would be workable, as it would format the whole file. (To me, the practice of avoiding unrelated cosmetic changes is more important than the line length.)

[Sarah Boyce]

Given this is currently being discussed, we can reopen the ticket once we've come to a conclusion