On Mon, Jan 28, 2019 at 9:48 PM Thomas Munro <thomas.mu...@enterprisedb.com> wrote: > On Mon, Jan 28, 2019 at 8:08 PM Michael Paquier <mich...@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
0001-Fix-pgindent-s-postprocessing-of-multi-line-prototyp.patch
Description: Binary data
