On Sun, Apr 06, 2008 at 06:54:54AM +0200, Jan Engelhardt wrote:
Why is it better for the end of the line to sometimes go a bit off the
right of the screen, but not too far? Are you trying to stop checkpatch
whinging about lines which simply cannot be split pleasently and so poke
a little off, or are you keen to have a bit more space to write code in
as a general rule?
If its the former then you are missing the point of checkpatch, it is
mearly an advisor trying to point those things you have done which the
maintainer is very likely going to notice and going to have to deal with
either by rejecting your patch or fixing it themselves; it is a time
saving aid to them and you. If the statement really cannot be sanely
wrapped somewhere then do not wrap it. The maintainer should be able to
see you are correct, if they dissagree they will re-wrap it where they
think it can be sanely wrapped. While I can imagine that you might have
one or two difficult wraps in a large set of patches, I would not expect
the number of false reports to be significant. I personally have seen
three or four a year in all the patches I have produced and reviewed.
So it does not seem worth changing the underlying rule just to avoid these
easily ignored reports, expecially considering the number of genuinely
bad lines it would then pass.
If its the latter (you want more space generally), then we should just
say "line length is now N" and be done with it, 95, 128, 200 whatever.
Letting you have 95 characters before telling you should not have had 81
is very non-intuitive and bound to confuse.
This is about visually representing the tabs as smaller units, and still
wanting the rest of the code to line up correctly. Although I can see
that the effect is somewhat desirable, it feels very much like doing
just this will then go on to encourage the writer to want to violate the
overall line length (as you have more space) and lead to the need to have
more characters on a line in the first place.
For myself I would not necessarily have a problem with this, as I should
be unable to see it in my tabs=8 world. But unless the code base is
consistant in its use of these then it would seem that their inconsistant
use would distroy the overall effect, and render them ineffective to you
as the consumer. Also they _will_ get broken by the general populace as
they edit without regard in their tabs=8 world, and their replacements
would be just as acceptable under the new rule as coded. That would
imply that simply allowing these would not be sufficient, but enforcing
this style (which is much harder) would be required.
As things stand Documentation/CodingStyle is pretty direct in its language
about what is and what is not acceptable in both these areas; I do not
need to run git blame to guess at its author. It seems entirely reasonable
for checkpatch to implement (as closely as it can) what is carved on that
stone tablet. The point of having a CodingStyle (and this has been said
many times) is not to please everyone (or indeed anyone but Linus), but to
try and engender consistancy in the code base to ease maintenance for those
higher up the food chain, for those that read all this code. We all have
our pet styles, and I can assure you Linux kernel style is not my style,
but I write code for the kernel in that style because those are the rules.
To justify changing checkpatch to loosen its checks I would hope to see
an agreed to change to the CodingStyle detailing actually what is now
acceptable. Failing that strong expressions from maintainers that they
are keen to have code in some alternative style, and presumably _all_
code for their area in that style. Then it might well make sense to
have different style applied automatically by maintainers area perhaps.
Of course those maintainers would need to be sure their style was going
to be accepted up the chain too.
Comments on the change as it stands follow inline.
I cannot see how we can fail to confuse our users if we only tell them
they have exceeded 80, when they hit 95.
This seem like it would allow a lot of lines to be aligned using just
spaces. Even where they are undesirable.
This looks very likely to false trigger on any number of unrelated
things. Almost all lines start with spaces then a non-space character?
Would this not reduce the test to be "you can use any number of spaces
as long as they follow tabs." And without actually calculating the
indent on the previous line we are not in a position to make any more of
a reasoned check than "\t* *" is ok, else whinge.
Now, we do know the indent to some degree, and we have some feel for
statements, and this align only indent seems only valid "within" a
statement. So we might well be able to be significantly more targetted.
Indeed were we to want to do this I think that would be required.
-apw
--
