[PATCH] Lindent: Handle missing indent gracefully

[PATCH] Lindent: Handle missing indent gracefully

[Jean Delvare]

If indent is not found, bail out immediately instead of spitting
random shell script error messages.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
---
 scripts/Lindent |    3 +++
 1 file changed, 3 insertions(+)

--- linux-4.2-rc0.orig/scripts/Lindent	2015-06-22 07:05:43.000000000 +0200
+++ linux-4.2-rc0/scripts/Lindent	2015-07-10 13:45:56.864031472 +0200
@@ -1,6 +1,9 @@
 #!/bin/sh
 PARAM="-npro -kr -i8 -ts8 -sob -l80 -ss -ncs -cp1"
 RES=`indent --version`
+if [ "$RES" = "" ]; then
+	exit 1
+fi
 V1=`echo $RES | cut -d' ' -f3 | cut -d'.' -f1`
 V2=`echo $RES | cut -d' ' -f3 | cut -d'.' -f2`
 V3=`echo $RES | cut -d' ' -f3 | cut -d'.' -f3`

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] Lindent: Handle missing indent gracefully

[Joe Perches]

On Fri, 2015-07-10 at 13:47 +0200, Jean Delvare wrote:
> If indent is not found, bail out immediately instead of spitting
> random shell script error messages.

OK, but can't we just delete Lindent instead?

> --- linux-4.2-rc0.orig/scripts/Lindent	2015-06-22 07:05:43.000000000 +0200
> +++ linux-4.2-rc0/scripts/Lindent	2015-07-10 13:45:56.864031472 +0200
> @@ -1,6 +1,9 @@
>  #!/bin/sh
>  PARAM="-npro -kr -i8 -ts8 -sob -l80 -ss -ncs -cp1"
>  RES=`indent --version`
> +if [ "$RES" = "" ]; then
> +	exit 1
> +fi
>  V1=`echo $RES | cut -d' ' -f3 | cut -d'.' -f1`
>  V2=`echo $RES | cut -d' ' -f3 | cut -d'.' -f2`
>  V3=`echo $RES | cut -d' ' -f3 | cut -d'.' -f3`
> 

Re: [PATCH] Lindent: Handle missing indent gracefully

[Jean Delvare]

Hi Joe,

Le Friday 10 July 2015 à 04:51 -0700, Joe Perches a écrit :
> On Fri, 2015-07-10 at 13:47 +0200, Jean Delvare wrote:
> > If indent is not found, bail out immediately instead of spitting
> > random shell script error messages.
> 
> OK, but can't we just delete Lindent instead?

Because...?

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] Lindent: Handle missing indent gracefully

[Joe Perches]

On Fri, 2015-07-10 at 15:36 +0200, Jean Delvare wrote:
> Hi Joe,

howdy Jean.

> Le Friday 10 July 2015 à 04:51 -0700, Joe Perches a écrit :
> > On Fri, 2015-07-10 at 13:47 +0200, Jean Delvare wrote:
> > > If indent is not found, bail out immediately instead of spitting
> > > random shell script error messages.
> > 
> > OK, but can't we just delete Lindent instead?
> 
> Because...?

It's just not very useful in today's development space.

indent is quite bad at handling long lines.

It wraps code at arbitrary points to fit a column boundary
rather than for readability or sensibility.

Code that may have deep indentation using a few spaces
per block level can look horrible post Lindent.

For instance, reiserfs commit bd4c625c061c
("reiserfs: run scripts/Lindent on reiserfs code")

There are other tools that reflow whitespace styles.

Re: [PATCH] Lindent: Handle missing indent gracefully

[Andrew Morton]

On Fri, 10 Jul 2015 10:04:07 -0700 Joe Perches <joe@perches.com> wrote:

> > Le Friday 10 July 2015 __ 04:51 -0700, Joe Perches a __crit :
> > > On Fri, 2015-07-10 at 13:47 +0200, Jean Delvare wrote:
> > > > If indent is not found, bail out immediately instead of spitting
> > > > random shell script error messages.
> > > 
> > > OK, but can't we just delete Lindent instead?
> > 
> > Because...?
> 
> It's just not very useful in today's development space.

I've very occasionally used Lindent.  It's useful if the input is an
utter mess.  You feed it through Lindent as a first pass then get in and
do the remainder by hand.

It can be less work than doing the whole conversion by hand.

Re: [PATCH] Lindent: Handle missing indent gracefully

[Joe Perches]

On Fri, 2015-07-10 at 14:37 -0700, Andrew Morton wrote:
> On Fri, 10 Jul 2015 10:04:07 -0700 Joe Perches <joe@perches.com> wrote:
> 
> > > Le Friday 10 July 2015 __ 04:51 -0700, Joe Perches a __crit :
> > > > On Fri, 2015-07-10 at 13:47 +0200, Jean Delvare wrote:
> > > > > If indent is not found, bail out immediately instead of spitting
> > > > > random shell script error messages.
> > > > 
> > > > OK, but can't we just delete Lindent instead?
> > > 
> > > Because...?
> > 
> > It's just not very useful in today's development space.
> 
> I've very occasionally used Lindent.  It's useful if the input is an
> utter mess.  You feed it through Lindent as a first pass then get in and
> do the remainder by hand.
> 
> It can be less work than doing the whole conversion by hand.

That's true, it can be, but I think Lindent mostly
doesn't work particularly well for reviewing and
it can require a lot more rework.

My biggest complaint about Lindent is that it can
produce _awful_ looking code when it has to wrap
longish lines.

I think that generally, checkpatch --fix-inplace
works better and it can work in discrete steps.

I submitted a little script a while back that does
most of what Lindent does.

https://lkml.org/lkml/2014/7/11/794

uncrustify also kinda works without the line
wrapping nuttiness.  It's not very good about
using Linux's pointer location style.

http://uncrustify.sourceforge.net/

clang-format works reasonably well.
It can respect existing line wrapping.

http://clang.llvm.org/docs/ClangFormatStyleOptions.html

Re: [PATCH] Lindent: Handle missing indent gracefully

[Jean Delvare]

On Sat, 11 Jul 2015 05:56:37 -0700, Joe Perches wrote:
> On Fri, 2015-07-10 at 14:37 -0700, Andrew Morton wrote:
> > On Fri, 10 Jul 2015 10:04:07 -0700 Joe Perches <joe@perches.com> wrote:
> > 
> > > > Le Friday 10 July 2015 __ 04:51 -0700, Joe Perches a __crit :
> > > > > On Fri, 2015-07-10 at 13:47 +0200, Jean Delvare wrote:
> > > > > > If indent is not found, bail out immediately instead of spitting
> > > > > > random shell script error messages.
> > > > > 
> > > > > OK, but can't we just delete Lindent instead?
> > > > 
> > > > Because...?
> > > 
> > > It's just not very useful in today's development space.
> > 
> > I've very occasionally used Lindent.  It's useful if the input is an
> > utter mess.  You feed it through Lindent as a first pass then get in and
> > do the remainder by hand.
> > 
> > It can be less work than doing the whole conversion by hand.
> 
> That's true, it can be, but I think Lindent mostly
> doesn't work particularly well for reviewing and
> it can require a lot more rework.

Well, this is a tool for submitters, not reviewers, so no surprise here.

> My biggest complaint about Lindent is that it can
> produce _awful_ looking code when it has to wrap
> longish lines.
>
> I think that generally, checkpatch --fix-inplace
> works better and it can work in discrete steps.

Thanks for the hint, this is indeed useful. Using the same tool to
fix-up formatting and to check for incoming patches makes a lot of
sense. However it seems that checkpatch.pl does not fix curly brace
placement, nor does it delete blank lines at end of files. So indent
can still do. So Lindent (or just indent actually, see below) can still
be useful, at least until the missing features are added to checkpatch.

> I submitted a little script a while back that does
> most of what Lindent does.
> 
> https://lkml.org/lkml/2014/7/11/794
> 
> uncrustify also kinda works without the line
> wrapping nuttiness.  It's not very good about
> using Linux's pointer location style.
> 
> http://uncrustify.sourceforge.net/
> 
> clang-format works reasonably well.
> It can respect existing line wrapping.
> 
> http://clang.llvm.org/docs/ClangFormatStyleOptions.html

Good to know, but unless these tools come with a preset for the Linux
kernel style, or the kernel itself provides it (as Lindent does for
for indent) I'm afraid it won't be too useful in practice.

BTW I noticed while looking for other options that recent versions of
indent (as of November 2007) have a "--linux-style" option which serves
the same purpose as Lindent. The result is almost the same in my case,
but --linux-style translates to a much longer list of options. Also
some options seem better than what Lindent uses (e.g. -il1 instead of
-il0.)

In the light of this I would tend to agree with Joe that Lindent could
go away and references to it be replaced with "indent --linux-style".

-- 
Jean Delvare
SUSE L3 Support