Re: Emacs vs pg_indent's weird indentation for function declarations

On Mon, Jan 28, 2019 at 9:48 PM Thomas Munro
<thomas(dot)munro(at)enterprisedb(dot)com> wrote:
> On Mon, Jan 28, 2019 at 8:08 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> > If you could get pgindent smarter in this area, it would be really
> > nice..
>
> Ah, it's not indent doing it, it's pgindent's post_indent subroutine
> trying to correct the effects of the (implied) -psl option, but not
> doing a complete job of it (it should adjust the indentation lines of
> later lines if it changes the first line).
>
> One idea I had was to tell indent not to do that by using -npsl when
> processing headers, like in the attached. That fixes all the headers
> I looked at, though of course it doesn't fix the static function
> declarations that appear in .c files, so it's not quite the right
> answer.

I tried teaching pgindent's post_indent subroutine to unmangle the
multi-line declarations it mangles. That produces correct
indentation! But can also produce lines that exceed the column limit
we would normally wrap at (of course, because pg_bsd_indent had less
whitespace on the left when it made wrapping decisions). Doh.
Attached for posterity, but it's useless.

So I think pg_bsd_indent itself needs to be fixed. I think I know
where the problem is. lexi.c isn't looking far ahead enough to
recognise multi-line function declarations:

if (*buf_ptr == '(' && state->tos <= 1 && state->ind_level == 0 &&
state->in_parameter_declaration == 0 && state->block_init == 0) {
char *tp = buf_ptr;
while (tp < buf_end)
if (*tp++ == ')' && (*tp == ';' || *tp == ','))
goto not_proc;
strncpy(state->procname, token, sizeof state->procname - 1);
if (state->in_decl)
state->in_parameter_declaration = 1;
return (funcname);
not_proc:;
}

That loop that looks for ')' followed by ';' is what causes the lexer
to conclude that the "foo" is an "ident" rather than a "funcname",
given the following input:

extern void foo(int i);

But if because buf_end ends at the newline, it can't see the
semi-colon and concludes that "foo" is a "funcname" here:

extern void foo(int i,
int j);

That determination causes indent.c's procnames_start_line (-psl) mode
to put "extern void" on its own line here and stop thinking of it as a
declaration:

case funcname:
case ident: /* got an identifier or constant */
if (ps.in_decl) {
if (type_code == funcname) {
ps.in_decl = false;
if (procnames_start_line && s_code != e_code) {
*e_code = '\0';
dump_line();
}

I guess it'd need something smarter than fill_buffer() to see far
enough, but simply loading N lines at once wouldn't be enough because
you could still happen to be looking at the final line in the buffer;
you'd probably need a sliding window. I'm not planning on trying to
fix that myself in the short term, but since it annoys me every time I
commit anything, I couldn't resist figuring out where it's coming from
at least...

--
Thomas Munro
https://enterprisedb.com