Date: Fri, 19 May 2017 11:05:26 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
	Piotr Stefaniak <postgres@piotr-stefaniak.me>
Subject: Re: Preliminary results for proposed new pgindent
 implementation
Message-ID: <20170519150526.GX3151@tamriel.snowman.net>
References: <30527.1495162840@sss.pgh.pa.us>
 <CA+TgmoamXhAoKY+pPSfGiu=3q6u-+1EOksdPimGS8YuqNuaSYQ@mail.gmail.com>
 <26654.1495203668@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="acetvMt2RaRpbGOQ"
Content-Disposition: inline
In-Reply-To: <26654.1495203668@sss.pgh.pa.us>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org


--acetvMt2RaRpbGOQ
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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.)
>=20
> > This, however, doesn't sound so good.  Isn't there some way this can be=
 fixed?
>=20
> 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.
>=20
> 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.
>=20
> 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.
>=20
> > What does that exactly mean concretely?
>=20
> 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.
>=20
> 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

--acetvMt2RaRpbGOQ
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJZHwm2AAoJEO1sijiDR2RVXQwP/RlZ5hNsYjnB0LmOBG0eyeQS
ws9KjC+X9CYUzi6BBaf+SAaIxcLmi37eWNLcL/RE3BQZODgbNolf8prtFR2uXzMA
l00u+zvw2e2TJSWCD8ECblRwaNWFaqyDdphcnLMf7up/18zzFUFzYsjMOyq78RTq
SQJVWJnqAzH2B6a50J9Esf8beuO31NuIOtozVfkNIU9TVV59bFbXDzVml31LADHd
bdCj05tHg9UAYD/X5MWXUy1popYgrx3KLrtYeDTmgdtdTTXSoaMuOJHJ/4EBKmxQ
ohaDcxKGDupvit8CeL4eefY7ZBJt5rUgcy6liU7H/GKoWJ6uB15sOhsxeHybEA9W
0KLGnr62eerUMUCbFxLwg1yegd9SDAeOoOl6z5Uv2DhayDoA99k9kDoETYZ+F1Vg
jnu1G6VT8C6ojxqZE5059SBZxvgYSM0xaQsyXeMK6mnQjh903Fg4KE0tNxK8KV7E
6zDiIcrmMaMN6XeVkeosO73syB4BoDR0uXCGhW3RkuTJXLkU178Lg45HrROWaE2o
DjPOKwiigxRNG9r1AteHUWgpZvus2RTRY3GmeZLyCyki3fyizjBDPX1CDPbetgLD
sLW5V8j9yncBHDfHwHsInTIco5Zs9EmUkQfSMU6BcLgF1BgL8Lo293PoEvB+5Guf
iSFlL3YuHkm8e7coUEC3
=3S9g
-----END PGP SIGNATURE-----

--acetvMt2RaRpbGOQ--