Thread: pgindent complaint of the day

pgindent complaint of the day

From
Tom Lane
Date:
This case in xlog.c is representative of a disease that pgindent has had
for awhile:

@@ -4276,7 +4300,8 @@ StartupXLOG(void)               if (needNewTimeLine)    /* stopped because of stop request */
                 ereport(FATAL,                                       (errmsg("requested recovery stop point is before
endtime of backup dump")));
 
-               else                                    /* ran off end of WAL */
+               else
+/* ran off end of WAL */                       ereport(FATAL,                                       (errmsg("WAL ends
beforeend time of backup dump")));       }
 

I'm not sure of all the triggering conditions, but every so often it
decides to move a line-ending comment to its own line (which is a wrong
policy in the first place IMHO) and forgets to indent it.  I've mostly
seen it on "else" lines but I'm not sure that's the only case.
        regards, tom lane


Re: pgindent complaint of the day

From
Bruce Momjian
Date:
Tom Lane wrote:
> This case in xlog.c is representative of a disease that pgindent has had
> for awhile:
> 
> @@ -4276,7 +4300,8 @@ StartupXLOG(void)
>                 if (needNewTimeLine)    /* stopped because of stop request */
>                         ereport(FATAL,
>                                         (errmsg("requested recovery stop point is before end time of backup
dump")));
> -               else                                    /* ran off end of WAL */
> +               else
> +/* ran off end of WAL */
>                         ereport(FATAL,
>                                         (errmsg("WAL ends before end time of backup dump")));
>         }
> 
> I'm not sure of all the triggering conditions, but every so often it
> decides to move a line-ending comment to its own line (which is a wrong
> policy in the first place IMHO) and forgets to indent it.  I've mostly
> seen it on "else" lines but I'm not sure that's the only case.

You are correct that it only happens on comments on an else line.  The
problem is that there is a BSD indent bug that will stop processing the
file in such cases so we have in pgindent:
# workaround for indent bug with 'else' handling       sed 's;\([}     ]\)else[        ]*\(/\*.*\)$;\1else\\2;g' |

and this does exactly as you describe by putting the comment on its own
line.  I just changed it to:
# workaround for indent bug with 'else' handling       sed 's;\([}     ]\)else\([      ]*\)\(/\*.*\)$;\1else\\2\3;g' |

so that the new comment will have the same indenting as the else that
was input.  It should help but will not be perfect.

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073
 


Re: pgindent complaint of the day

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > and this does exactly as you describe by putting the comment on its own
> > line.  I just changed it to:
> > ...
> > so that the new comment will have the same indenting as the else that
> > was input.
> 
> If it were "the else's indent plus one more tab" it would be reasonably
> sane; it'd match the indentation of what comes next.

OK, I can do that but consider:
else /* comment */{    something;}

If we go without the special indent we get at worst case:
else/* comment */    something;

while with an extra indent we get this as worst case:
else     /* comment */{    something;}

which seems like a worse worst case.  :-)

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073
 


Re: pgindent complaint of the day

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> and this does exactly as you describe by putting the comment on its own
> line.  I just changed it to:
> ...
> so that the new comment will have the same indenting as the else that
> was input.

If it were "the else's indent plus one more tab" it would be reasonably
sane; it'd match the indentation of what comes next.
        regards, tom lane


Re: pgindent complaint of the day

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> If it were "the else's indent plus one more tab" it would be reasonably
>> sane; it'd match the indentation of what comes next.

> OK, I can do that but consider:
> [ other case ]

Just out of curiosity, what will pgindent do when re-run on the file
with the comment already split to the next line?  My experience with
it so far is that it will not move a comment that starts in column 1,
but it will feel free to re-indent a comment that has some indentation.
A reasonable goal here would be that running pgindent a second time does
not create immediate further changes.
        regards, tom lane


Re: pgindent complaint of the day

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Tom Lane wrote:
> >> If it were "the else's indent plus one more tab" it would be reasonably
> >> sane; it'd match the indentation of what comes next.
> 
> > OK, I can do that but consider:
> > [ other case ]
> 
> Just out of curiosity, what will pgindent do when re-run on the file
> with the comment already split to the next line?  My experience with
> it so far is that it will not move a comment that starts in column 1,
> but it will feel free to re-indent a comment that has some indentation.
> A reasonable goal here would be that running pgindent a second time does
> not create immediate further changes.

Right. I don't see it moving comments up on to an else line.  You are
right that if it is in the first column it will not be properly indented
so I just indent it 4 spaces before passing to BSD indent and that
works:# workaround for indent bug with 'else' handling# indent comment so BSD indent will move it        sed 's;\([}
]\)else[        ]*\(/\*.*\)$;\1else\    \2;g' |
 


--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073