Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Aug 11, 2009 at 10:08 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> If that's not it, you'd need to mention details.

> Well, one thing I've noticed is that when a function prototype wraps
> around to the next line, you often change the number of spaces in the
> hanging indent.

Ah. That's a bit idiosyncratic to pgindent. What it does for a
function definition makes sense, I think: it lines up all the
parameters to start in the same column:

static int
myfunction(int foo,
int bar)

What is not obvious is that the same amount of hanging indent is
used for the function's *prototype*, even though the first line
is not the same:

static int myfunction(int foo,
int bar);

ie, the indent is length of function name plus 1. I find this a bit
stupid myself and would prefer that the prototypes looked like this:

static int myfunction(int foo,
int bar);

which is what my editor will do with it if left to its own devices.
So depending on whether I had occasion to touch the prototype, you might
see it get reindented to the second style. (I know pgindent will change
it again, but I'm not sufficiently anal to override Emacs here.) Also,
I tend to make sure that there are not so many parameters declared on
the same line that it would look bad if pgindent ever starts doing
things my way, ie, if I see

static someveryverylongtypename *somefunctionname(int foo,
int bar, int baz, int quux, int somethingelse, int more);

I'll probably split the second line, even though there's room for it
according to pgindent's current rule.

So, yeah, I'm being a bit anal about this and yet I'm not always
consistent. But that's what pgindent is good at fixing ...

> I am also a bit confused about the rule for indenting the variables in
> variable declarations. I get that the *, if any, is suppose to hang
> to the left of the name, so that the first character of each variable
> name lines up - and in particular, line up on a tab stop. But it's
> not obvious to me how to choose which tab stop to line everything up
> on.

I'm not too sure either. It's the fourth tab stop for declarations at
the outer level of a function, but I've never entirely figured out what
pgindent's rule is for nested declarations. It seems to be willing to
move the target column over to some extent, but eventually it gives up
and doesn't add any more whitespace. I don't usually try very hard to
align declarations exactly where pgindent will put them, although if
it's visually ragged as-submitted sometimes I'll fix that.

One thing I frequently *do* do is remove submitted whitespace changes
if I know that pgindent would undo them anyway, just to reduce the size
of the final diff. A case that happened at least once in this
commitfest is that somebody had taken a block of variables aligned to
the fourth tab stop:

int foo;
int bar;
int baz;

and realigned them to the fifth stop, because he was adding another
variable with a longer type name:

int foo;
int bar;
int baz;
longtypename *ptr;

Well, that looks nice, but pgindent is going to do this anyway:

int foo;
int bar;
int baz;
longtypename *ptr;

so I tend to revert such dead-end whitespace changes just as part of
making sure there wasn't an unintentional change of the existing lines.
(This is the sort of thing that having submitters pgindent beforehand
might avoid ...)

Bored yet?

regards, tom lane