Re: Preliminary results for proposed new pgindent implementation

Tom, all,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > On Thu, May 18, 2017 at 11:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> The reason PGSSTrackLevel is "unrecognized" is that it's not in
> >> typedefs.list, which is a deficiency in our typedef-collection
> >> technology not in indent.  (I believe the problem is that there
> >> are no variables declared with that typename, causing there to
> >> not be any of the kind of symbol table entries we are looking for.)
> 
> > This, however, doesn't sound so good.  Isn't there some way this can be fixed?
> 
> I'm intending to look into it, but I think it's mostly independent of
> whether we replace pgindent itself.  The existing code has the same
> problem, really.
> 
> One brute-force way we could deal with the problem is to have a "manual"
> list of names to be treated as typedefs, in addition to whatever the
> buildfarm produces.  I see no other way than that to get, for instance,
> simplehash.h's SH_TYPE to be formatted as a typedef.  There are also
> some typedefs that don't get formatted correctly because they are only
> used for wonky options that no existing typedef-reporting buildfarm member
> builds.  Manual addition might be the path of least resistance there too.
> 
> Now the other side of this coin is that, by definition, such typedefs
> are not getting used in a huge number of places.  If we just had to
> live with it, it might not be awful.

Dealing with the typedef lists in general is a bit of a pain to get
right, to make sure that new just-written code gets correctly indented.
Perhaps we could find a way to incorporate the buildfarm typedef lists
and a manual list and a locally generated/provided set in a simpler
fashion in general.

> >> All in all, this looks pretty darn good from here, and I'm thinking
> >> we should push forward on it.
> 
> > What does that exactly mean concretely?
> 
> That means I plan to continue putting effort into it with the goal of
> making a switchover sometime pretty darn soon.  We do not have a very
> wide window for fooling with pgindent rules, IMO --- once v11 development
> starts I think we can't touch it again (until this time next year).

Agreed.

> > We've talked about pulling pgindent into our main repo, or posting a
> > link to a tarball someplace.  An intermediate plan might be to give it
> > its own repo, but on git.postgresql.org, which seems like it might
> > give us the best of both worlds.  But I really want something that's
> > going to be easy to set up and configure.  It took me years to be
> > brave enough to get the current pgindent set up.
> 
> Yes, moving the goalposts on ease-of-use is an important consideration
> here.  What that says to me is that we ought to pull FreeBSD indent
> into our tree, and provide Makefile support that makes it easy for
> any developer to build it and put it into their PATH.  (I suppose
> that means support in the MSVC scripts too, but somebody else will
> have to do that part.)

I'm not a huge fan of this, however.  Do we really need to carry around
the FreeBSD indent in our tree?  I had been expecting that these changes
would eventually result in a package that's available in the common
distributions (possibly from apt/yum.postgresql.org, at least until it's
in the main Debian-based and RHEL-based package systems).  Are you
thinking that we'll always have to have our own modified version?

> We should also think hard about getting rid of the entab dependency,
> to eliminate the other nonstandard prerequisite program.  We could
> either accept that that will result in some tab-vs-space changes in
> our code, or try to convert those steps in pgindent into pure Perl,
> or try to convince Piotr to add an option to indent that will make
> it do tabs the way we want (ie use a space not a tab if the tab
> would only move one space anyway).

What about perltidy itself..?  We don't include that in our tree either.

I do think it'd be good to if Piotr would add such an option, hopefully
that's agreeable.

> Lastly (and I've said this before, but you pushed back on it at
> the time), if we're doing this then we're going all in.  That
> means reformatting the back branches to match too.  That diff
> is already big enough to be a disaster for back-patching, and
> we haven't even considered whether we want to let pgindent adopt
> less-inconsistent rules for comment indentation.  So I think that
> as soon as the dust has settled in HEAD, we back-patch the addition
> of FreeBSD indent, and the changes in pgindent proper, and then
> run pgindent in each supported back branch.

Ugh.  This would be pretty painful, but I agree that back-patching
without doing re-indenting the back-branches would also suck, so I'm on
the fence about this.

Thanks!

Stephen