bug#31446: 26.1; sql-interactive-mode buffer naming is broken

[Filipp Gunbin <fgunbin@fastmail.fm>]

On 18/05/2018 23:24 +0300, Eli Zaretskii wrote:

>> From: Filipp Gunbin <fgunbin@fastmail.fm>
>> Cc: psainty@orcon.net.nz,  31446@debbugs.gnu.org
>> Date: Fri, 18 May 2018 21:03:00 +0300
>>
>> Your patch, while it may have fixed the bug, introduces some logic on
>> setting buffer name - so we could get undesired behaviour change.
>
> AFAIK, the logic was already there, I just fixed it to behave more
> reasonably.
>
> The original code was
>
>                          (if (and new-name (string-prefix-p "SQL" new-name t))
>                              new-name
>                            (concat "SQL: " new-name))))
>
> This is now
>                          (cond
>                           ((zerop new-name)
>                            "*SQL*")
>                           ((stringp new-name)
>                            (if (string-prefix-p "SQL" new-name t)
>                                new-name
>                              (concat "*SQL: " new-name "*")))
>                           ((eq new-name '(4))
>                            (sql-rename-buffer new-name)
>                            sql-alternate-buffer-name)
>                           (t
>                            (format "*SQL: %s*" new-name)))))
>
> which (a) avoids concatenating a string and a list '(4); (b) avoids
> calling string-prefix-p with 2nd arg not a string, something that
> works only by sheer luck; (c) calls sql-rename-buffer to choose the
> buffer name, because that function implements the logic of naming the
> buffer, and duplicating that sounds redundant; (d) does something
> reasonable when new-name is neither a string nor a prefix arg nor nil.
>
> And that is the only thing that I changed.

That looks reasonable, but, as I wrote earlier, why implement
name-choosing logic here, if it's in `sql-comint' already.  Even if some
of such logic was here before the fix, it doesn't necessarily mean that
was right.

>                           ((eq new-name '(4))
>                            (sql-rename-buffer new-name)
>                            sql-alternate-buffer-name)

`sql-alternate-buffer-name' not always matches the effective name set by
`sql-rename-buffer'..

I'll test my suggestion tomorrow and will write about the result, if you
don't mind.

Filipp