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

On Mon, Jan 28, 2019 at 9:48 PM Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> On Mon, Jan 28, 2019 at 8:08 PM Michael Paquier <michael@paquier.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