bug#24646: [PATCH] Complete the name of PostgreSQL databases

[Simen Heggestøyl]

Thank you for your detailed feedback, Michael.

I agree with all of your points and have modified the patch accordingly,
except for one:

On Mon, Nov 7, 2016 at 5:03 AM, Michael Mauger <michael@mauger.com> 
wrote:
> * `dolist' can specify the return value rather than having a separate 
> expression after the loop. That is, (dolist (row (process-lines ...) 
> (nreverse res)) ...)) is equivalent to (dolist (row (process-lines 
> ...)) ...) (nreverse res)

I'm aware of it, but I tend to avoid using it, since I have many times
myself overlooked it when reading code that uses it. If it's OK with you
I'll leave it like it is, but if you insist I can change it.

> * I'm concerned about the change to the `completing-read' call in 
> `sql-get-login-ext'. Rather than `nil', I'd suggest `confirm' so that 
> if the value isn't in the list, it must be confirmed.
> * If the REQUIRE-MATCH parameter should really be something other 
> than `t', then possibbly we should add another keyword 
> :completion-required whose value would be used in the 
> `completing-read' call (default to `t' to preserve current 
> functionality).

It's not important to me. I made the change because I thought it could
be frustrating for the user if `sql-postgres-list-databases' doesn't
work as it should, but I realize now that changing it probably deserves
a discussion and a patch on its own.

-- Simen

0001-Complete-the-name-of-PostgreSQL-databases.patch
Description: Text Data