"Where possible, try to keep lines no wider than 80 positions." There are several examples in the tree now where line lengths exceed 100 "positions" and even double that, so we should have at least a warning about this.
80 is unrealistic. certainly 300 is way too long. i'd say 150 is a more realistic warning limit. although we'd have to determine whether this applies to lines that don't typically get edited (like KEYWORDS). i wouldn't bother with those ...
(In reply to comment #1) > 80 is unrealistic. I was only quoting devmanual. http://devmanual.gentoo.org/ebuild-writing/file-format/index.html
A quick and dirty check says that ebuilds with maximum line lengths of 200 characters make up some 96% of all ebuilds and eclasses.
89% of ebuilds and eclasses max out at 150 characters.
(In reply to comment #3) > A quick and dirty check says that ebuilds with maximum line lengths of 200 > characters make up some 96% of all ebuilds and eclasses. (In reply to comment #4) > 89% of ebuilds and eclasses max out at 150 characters. Have you excluded the KEYWORDS, IUSE and DESCRIPTION (which has its own warning already) lines? Otherwise, these numbers are almost meaningless.
Created attachment 327958 [details] gentoo-x86-line-lengths-no-vars.gz for i in */*/*.ebuild eclass/*.eclass; do printf '%s' $( sed -e '/^[[:space:]]*[A-Z_]*\=/d' $i | wc -L ); echo " $i"; done | sort -n
(In reply to comment #2) it's fine making a recommendation in the documentation to use 80 cols. my point was enforcing that w/repoman warnings is wrong. (In reply to comment #5) yeah, i'd also exclude DESCRIPTION. some people (myself included) like to keep IUSE unwrapped, but some will split it.
(In reply to comment #7) > (In reply to comment #2) > > it's fine making a recommendation in the documentation to use 80 cols. my > point was enforcing that w/repoman warnings is wrong. Enforcing 80 chars is a bit much - that should be revised, I guess. > (In reply to comment #5) > > yeah, i'd also exclude DESCRIPTION. some people (myself included) like to > keep IUSE unwrapped, but some will split it. When I further filter on lower case variable assignments, allowing for += too, the number of ebuilds with lines over 200 chars comes down to about 100. This catches lines that do not set variables but define commands, and those probably should be split up into multiple lines as bash/ebuild can handle that fine, unlike variables, with the exception of perhaps strings like sed expressions.
I can't reassign this for some reason to the pkgcore component.
(In reply to Jeroen Roovers (RETIRED) from comment #8) > When I further filter on lower case variable assignments, allowing for += > too, the number of ebuilds with lines over 200 chars comes down to about 100. Coming back to this. Two weeks from now, we can celebrate this bug's 10th birthday. Maybe I am old school, but I keep my Emacs windows at 80 characters width so I can have several windows side by side (even on my laptop where screen estate is limited). Also most code still follows that convention for reasons of readability. So IMHO a limit of 200 would be useless. Once lines wrap around, it makes little difference if they're 200 or 1000 chars long. I'd suggest to warn if 80 positions (counting TAB as 4) are exceeded, but exclude DESCRIPTION, KEYWORDS and IUSE from the check. No hard enforcement of the limit.
I think I will do an experiment (In reply to Ulrich Müller from comment #10) > Coming back to this. Two weeks from now, we can celebrate this bug's 10th > birthday. OK, I see, I have a deadline of 2 weeks to make ulm wrong once ;-) OK, I will stop joking now. > Maybe I am old school, but I keep my Emacs windows at 80 characters width so > I can have several windows side by side (even on my laptop where screen > estate is limited). Also most code still follows that convention for reasons > of readability. > > So IMHO a limit of 200 would be useless. Once lines wrap around, it makes > little difference if they're 200 or 1000 chars long. I quite like this idea, but I'm afraid of the fallout size. So, it will warn once per version, and list all lines exceeding suggested length (style level). I think I will experiment with different lengths, and see how bad it is. If I see that it is like 90 chars, maybe we should go with it. I just myself sometimes feel like 80 is too little (but I'm from young school). > I'd suggest to warn if 80 positions (counting TAB as 4) are exceeded, but > exclude DESCRIPTION, KEYWORDS and IUSE from the check. No hard enforcement > of the limit. I would also bring another exception: when the line consists of a single "word" (AKA no spaces) - for things like ESELECT_DESLECT where just the path::class::test_name might be very long.
OK, some results after implementing it (still no pull request): If I put threshold at 80 chars per line, we have 4313 ebuilds At 90 chars, 2785 ebuilds At 100 chars, 1803 ebuilds At 120 chars, 849 For those numbers, I skip based on the talk here: 1. DESCRIPTION, KEYWORDS, IUSE lines 2. Lines which have a word with length over N-10 (so we have a super long word there, we remain with it as we can't break it) 3. lines with long quoted string over N chars So: is it a lot? I think yes. Should we do it as style level? yes. Should we use threshold at 80 - hard to me to day. On one point I say yes, but maybe 90 is more realistic? I always feel like 80 is too little (although I hold it in ebuilds if possible). What does QA team says? 80 chars?
The bug has been closed via the following commit(s): https://gitweb.gentoo.org/proj/pkgcore/pkgcheck.git/commit/?id=c415e94f833acc4b824cf2b6db332244e97313e9 commit c415e94f833acc4b824cf2b6db332244e97313e9 Author: Arthur Zamarin <arthurzam@gentoo.org> AuthorDate: 2022-10-22 08:21:13 +0000 Commit: Arthur Zamarin <arthurzam@gentoo.org> CommitDate: 2022-10-28 13:15:03 +0000 ExcessiveLineCheck: check for too long lines Closes: https://bugs.gentoo.org/440686 Signed-off-by: Arthur Zamarin <arthurzam@gentoo.org> src/pkgcheck/checks/codingstyle.py | 39 +++++++++++++++++++++++++ tests/checks/test_codingstyle.py | 59 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+)
