Date: Thu, 22 Dec 2016 17:26:22 -0500
From: Stephen Frost <sfrost@snowman.net>
To: Gilles Darold <gilles.darold@dalibo.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: [patch] psql tab completion for ALTER DEFAULT
 PRIVILEGES
Message-ID: <20161222222622.GU18360@tamriel.snowman.net>
References: <1614593c-e356-5b27-6dba-66320a9bc68b@dalibo.com>
 <9905474c-ab29-bb66-408f-ca776fb3d69c@dalibo.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="+61ZvX8xaeyevaKt"
Content-Disposition: inline
In-Reply-To: <9905474c-ab29-bb66-408f-ca776fb3d69c@dalibo.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org


--+61ZvX8xaeyevaKt
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Gilles,

* Gilles Darold (gilles.darold@dalibo.com) wrote:
> Le 20/11/2016 =E0 15:46, Gilles Darold a =E9crit :
> > When tab-completing after ALTER DEFAULT PRIVILEGES ... GRANT|REVOKE,
> > currently psql injects completion from the GRANT|REVOKE order, rather
> > than the one expected.
> >
> > A patch is attached. It adds the right completion to GRANT|REVOKE after
> > ALTER DEFAULT PRIVILEGES and after FOR ROLE|USER + IN SCHEMA.
>=20
> Added to next commitfest. To explain more this patch, the completion of
> SQL command:

I've started looking at this.  First off, it looks pretty good and seems
like it's actually a bug fix which should be back-patched since the
current behavior in released branches is also wrong.  There's been some
changes in this area, so it might not be practical to go all the way
back, will have to see once I start getting into it.

One minor nit is that multi-line comments should be of the form:

/*
 * ...
 */

The tab-completion code does do some like this:

/* ... */
    /* ... */

Which is probably alright, but you've add some like:

/* ...
      .... */

Which we really don't do.  I'll clean that up and might do a bit of
word-smithing on the comments also, so no need for a new patch, but
thought I'd mention it for future patches.

Thanks!

Stephen

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

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

iQIcBAEBCgAGBQJYXFMOAAoJEO1sijiDR2RVdFwP/15cWyI1f1v+xtWDXaaGK68/
A1CnAeBemu3/ZtxOCbkVLVFaPAndijdJAbnA7dCQgHl9pJ0GjT1BcZR0UuJqseDs
Q44dMUzKgBLZmaAe/tC9aHgiA6I2PsFTMnJH4W7GoMYHJgB2FdzE5wjNlIhvYPWh
7h5M/EmTuMAoZ51u7G+tNTmfBqyAOryOiqkFt5/R030AsyBDm0GIfEUUUT24WAJw
zkgJn4t0vCzsVr0EYpltgt5jymTx/M0BATvFNEbpnZqg13A6OxRQxnJ4XjmaY9m7
UDOaJbZ5fWpKVAgYz5JLDqzApcbKBoj2t71l3cHGuvdE0/p74Q/uXvWHpVvqiv9m
Hgdi7wBEKzENwDsojn0b1KI+3Iiw8okjIHU/OjW4GwyY7YXs1+0K1nrf1/4WtfMZ
WJ43KXN37CYFZU0wsVmOSKw4q3IQ2/B8p0zOeOD9YoqMNrqSZk/1hInQX89xLe/J
mKAwm7zcMbpF7QC+1JAKWIQCPKzcgrxwVui+RKYHjjdVSs7oqALERxTVl9hxh5Zy
++P0k8CzA3oa6QOQMLWvPugLIJNNKWmDhtcSRYNSssqWhVUSEt1EnmVssRr0WyJx
4IHv4zjQX4HAn+a7k0+cBQh0PcgM0yG6WIglqkikM499j6TKH8rWsOwqRUFfNcgB
s+25bVwlkbLVCr1NC01A
=KWUk
-----END PGP SIGNATURE-----

--+61ZvX8xaeyevaKt--