On Tue, 16 Jun 2015 at 00:52 Vik Fearing <v...@2ndquadrant.fr> wrote:
> While reviewing the seqam patches, I noticed that psql has tab > completion for ALTER SEQUENCE, but not for CREATE SEQUENCE. > > The attached trivial patch fixes that. > Hi Vik, I'm doing an initial review of this patch. It applies cleanly, although for some reason the patch seems to introduce carriage returns in the line endings. `patch` warned about those and then very sensibly ignored them. Everything compiled fine after applying. The new code's style is not quite in accordance with the surrounds. I think that the comments introducing each section should be de-indented to the first column. "RESTART", "SET SCHEMA", "OWNER TO", and "RENAME TO" are not valid completions for CREATE SEQUENCE. It looks like these have been blindly copy-pasted from ALTER SEQUENCE. Likewise, the valid completion "START [WITH]" is missing. "CREATE TEMP[ORARY] SEQUENCE" is not supported, and I think it probably should be. I think that the patch could be improved with support for the TEMP form fairly easily. Apart from the above points, the new completions worked as advertised. Docs: none needed. Tests: we don't currently have any test coverage for psql tab completion. I'm marking this "Waiting on Author". Once the problems have been corrected, it should be ready for a committer. Cheers, BJ
