[Ksummit-discuss] checkkpatch (in)sanity ?

[Josh Triplett]

On Mon, Aug 29, 2016 at 11:01:40AM -0700, Joe Perches wrote:
> On Mon, 2016-08-29 at 17:46 +0000, Luck, Tony wrote:
> > > 
> > > 80 columns is simply silly when dealing with either
> > > long identifiers or many levels of indentation.
> > > 
> > > One thing that 80 column limit does do is encourage
> > > shorter identifiers and fewer levels of indentation.
> > > 
> > > Generally, both of those are good things.
> > I think the main complaint with the limit is that people fix it by simply
> > breaking the long line, which often makes for less readable code.
> > 
> > Perhaps there would be less pushback on this if checkpatch also
> > complained about clumsily broken long lines and offered the advice
> > to restructure the code with helper functions etc. to avoid deep
> > indentation?
> 
> It suggests that already for 6+ leading tabs, but some more
> intelligence for nominally ugly added line breaks would
> definitely help.
> 
> Using longish simple identifiers or multiple dereferences
> can make the line breaks at 80 columns silly.
> 
> Simple things like:
> 
> 			if (longish_identifier != AN_EVEN_LONGER_DEFINED_CONSTANT_VALUE)
> and
> 			if (some_pointer->member[index].another_member >> shift_constant)
> 
> shouldn't really ever be broken into multiple lines,

Agreed.

Honestly, I almost never see a line that should break solely based on
length.  Almost any line that makes sense to break at a given point
would make sense to break at that point even with a target line length
of 200.

For instance:

			if (an_interesting_function(x) == TARGET_VALUE_FOR_X
			    || an_interesting_function(y) == TARGET_VALUE_FOR_Y) {

That line break makes sense whether you want to break lines at 80
characters, 100, or 800.  (You could argue about
before-or-after-operator, or about line alignment.)  In almost no
circumstances would you want to also break around the '==', even though
that second line takes up 82 characters.