From: Stephen Frost <sfrost@snowman.net>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: Piotr Stefaniak <postgres@piotr-stefaniak.me>
Subject: Re: Preliminary results for proposed new pgindent implementation
Date: Fri, 19 May 2017 11:05:26 -0400
Message-ID: <20170519150526.GX3151@tamriel.snowman.net> (raw)
In-Reply-To: <26654.1495203668@sss.pgh.pa.us>
References: <30527.1495162840@sss.pgh.pa.us>
<CA+TgmoamXhAoKY+pPSfGiu=3q6u-+1EOksdPimGS8YuqNuaSYQ@mail.gmail.com>
<26654.1495203668@sss.pgh.pa.us>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
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
Attachments:
[application/pgp-signature] signature.asc (818B, ../20170519150526.GX3151@tamriel.snowman.net/2-signature.asc)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: sfrost@snowman.net, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, postgres@piotr-stefaniak.me
Subject: Re: Preliminary results for proposed new pgindent implementation
In-Reply-To: <20170519150526.GX3151@tamriel.snowman.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox