From: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
---|---|
To: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Subject: | Re: Emacs vs pg_indent's weird indentation for function declarations |
Date: | 2019-04-07 23:36:25 |
Message-ID: | CA+hUKGKCvoUsU_HpW+hg8WNDUFjQEgQvVi9AwYdsqgJfV8TzxQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
0001-Fix-pgindent-s-postprocessing-of-multi-line-prototyp.patch | application/octet-stream | 1.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Haribabu Kommi | 2019-04-08 00:04:00 | Re: Transaction commits VS Transaction commits (with parallel) VS query mean time |
Previous Message | Justin Pryzby | 2019-04-07 21:28:29 | Re: ToDo: show size of partitioned table |