Re: [PATCH] scsi: megaraid: cleanup formatting of megaraid

[Joe Perches]

On Fri, 2022-03-04 at 19:48 +0100, Miguel Ojeda wrote:
> On Fri, Mar 4, 2022 at 6:37 PM Joe Perches <joe@xxxxxxxxxxx> wrote:
> > 
> > I rather doubt clang-format will ever be 'close enough'.
> > 
> > A human's sense of 'taste' for reading code is very different than
> > what an automated tool produces.
> 
> Maybe,

Hey again Miguel.

Is that statement really disputable?

> but it is a trade-off. If it is close enough, the benefits of
> automatic formatting may overcome the downsides.

IYO.  I think using an SCCS with better language understanding rather
than a line oriented one could be an improvement.  Such a tool could
allow arbitrary style reformatting at check-in/check-out.

> > Also, try looking at the changes clang-format does on a file chosen
> > at random:
> > 
> > o columnarized definitions -> not columnarized
> > o odd line continuation placement using spaces and not tabs before \
> > o odd array definition layouts
> > o per line definitions with comments poorly laid out
> > o individual line definitions rewrapped
> > o enum definitions on multiple lines compressed to single lines
> > o u8 array definition layouts where the first element has a separate
> >   meaning than the subsequent elements are compressed and made
> >   difficult to understand
> 
> I am not sure what you are trying to show here -- some of these are
> precisely the things that the tool could improve or have already
> improved, and we may just need to use the new option.

All of these existing code are more human readable than the code
reformatted using clang-format.

I used whatever is the latest clang-format here with today's -next.
https://releases.llvm.org/download.html

> For instance, for the columnarized macros case, it is possible to
> align them since clang-format 9. For array of structures, there is
> also a new alignment option since clang-format 13. Etc.

Then perhaps you as the maintainer of the kernel's .clang-format file
could update the entries for those new options.

I believe the minimum clang version is already 11.  Maybe higher.
I don't track clang or use it very much.  The clang version I use
though is 13.

> For the wrapping and related bits, now that the limit on 80 is a bit
> more fuzzy, we could perhaps tweak the penalties to improve the
> decision making.

Please have at it.
But perhaps tweaking will just improve some cases and worsen others.

> In summary, what we should be trying to do is improve the tool
> configuration and tool itself to see if we can get it to be close
> enough to the kernel style to make it more widely used.
> 
> > I think _some_ clang-format output is ok, but the concept of
> > enabling/disabling specific reformatting bits would be quite useful.
> > 
> > And sprinkling "clang-format on/off" lines in the code is not good.
> 
> Definitely, but it is fine in some exceptional cases.

I don't think so.

> > Any control codes that determine when source code layout might be
> > immutable or allowed to be modified could be should be tool name
> > agnostic.
> 
> I don't see why would that be a problem, and I don't understand why we
> would use several different formatting tools (the point is to be
> consistent, after all); but sure, we could propose an alternative
> spelling.

Thanks.

There is no "one true editor".
There will not be "one true source code formatter" either.

cheers not jeers, just keep at it.  Joe