Re: Tab completion for SET TimeZone - Mailing list pgsql-hackers

Tom Lane <tgl@sss.pgh.pa.us> writes:

> =?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org> writes:
>> I just realised there's no point in the subselect when I'm not applying
>> the same function in the WHERE and the SELECT, so here's an updated
>> version that simplifies that.  It also fixes a typo in the commit
>> message.
>
> This doesn't work right for me under libedit -- it will correctly
> complete "am<TAB>" to "'America/", but then it fails to complete
> anything past that.  The reason seems to be that once we have a
> leading single quote, libedit will include that in the text passed
> to future completion attempts, while readline won't.  I ended up
> needing three query variants, as attached (bikeshedding welcome).

Well spotted, I must have just tested that it completed something on the
first tab. The fix looks reasonable to me, and I have no better ideas
for the names of the query string #defines.

> I think the reason the COMPLETE_WITH_ENUM_VALUE macro doesn't look
> similar is that it hasn't made an attempt to work with input that
> the user didn't quote --- that is, if you type
>     alter type planets rename value ur<TAB>
> it just fails to match anything, instead of providing "'uranus'".
> Should we upgrade that likewise?`

The comment says it will add the quote before text if it's not there, so
maybe we should adjust that to say that it will only add the quote if
the user hasn't typed anything?

> Not sure it's worth the trouble though; I think
> COMPLETE_WITH_ENUM_VALUE is there more as a finger exercise than
> because people use it regularly.

I agree, I mostly implemented that for completeness after adding ALTER
TYPE … RENAME VALUE. Also, enum values always need quoting, while SET
TIMEZONE doesn't if the zone name follows identifier syntax and people
might start typing it without quotes if they intend set one of those, so
the extra effort to make that work even though we can't suggest a mix of
quoted and non-quoted names is worth it.

> I added a regression test case too.

Good idea idea, I keep forgetting that we actually have tests for tab
completion, since most cases are so simple and obviously correct that we
don't bother with tests for them.

> ... btw, I forgot to mention that I don't see any problem with
> the patch's behavior for DEFAULT.  What I see with both readline
> and libedit is that if you type something reasonable, like
>     set timezone to d<TAB>
> then it will correctly complete "efault ", without any extra
> quotes.  Now, if you're silly and do
>     set timezone to 'd<TAB>
> then readline gives you "efault' " and libedit gives you nothing.
> But I think that's your own, um, fault.

I agree, it was just a quirk noticed while testing. Also, it helps that
there aren't any actual time zone names starting with 'd'.

> We don't have any other places where tab completion is so aggressive
> as to remove quotes from a keyword.

I noticed afterwards that other config varables behave the same, so even
if we wanted to improve that, it's outwith the scope of this patch.

>             regards, tom lane

- ilmari