bug#62732: 29.0.60; uniquify-trailing-separator-p affects any buffer whose name matches a dir in CWD

[sbaugh]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Ping again on this latest version of my patch.  I have some actual
>> features I'd like to add to uniquify, but they're on top of this patch.
>> Plus it would be nice to land the tests added in my patch.
>
> Sorry for not following up further, but I was waiting for a reaction to
> my proposal to replace the additional arg to `create-file-buffer`
> by the patch below.

Ah, that unfortunately doesn't work, at least on its own.  It causes the
tests I added for uniquify to immediately fail.  The reason it doesn't
work, I believe, is that it makes us try to uniquify buffers with their
<n> numeric suffix included (which is the initial name the buffer gets).
And such buffers are of course already unique, so uniquify stops doing
anything.

We could remove the <n> suffix, but what we really want is just "the
name we wanted to give the buffer, before any uniquifying", aka the
basename.  And we have that in create-file-buffer, so it seems better to
just pass it down directly.

> If we can't avoid changing the API of `create-file-buffer`, I'd like
> a comment explaining clearly why.

So, the thing we want to communicate to uniquify is, the basename of the
buffer: the name we want for the buffer before it's made unique.  For
regular files this is (file-name-nondirectory file), the last component
of the file name.

Before this patch, the basename (as passed to uniquify) was always 
(file-name-nondirectory
(directory-file-name dir)) for directories.  So directories and regular
files looked the same to uniquify.  So it needed to add the trailing
separator some other way (by checking file-directory-p).

After this patch, the trailing separator is added *as part of the
basename* as passed to uniquify.  So uniquify doesn't need to do
anything, just uniquify the buffer like it would any other, and indeed
uniquify.el no longer uses uniquify-trailing-separator-p at all.

> As it stands the patch is a bit vague
> about that, if not confusing:
>
>      ;; FIXME we really need to fold the uniquify stuff in here by default,
>     -;; not using advice, and add it to the doc string.
>     -(defun create-file-buffer (filename)
>     +(defun create-file-buffer (filename &optional directory)
>        "Create a suitably named buffer for visiting FILENAME, and return it.
>      FILENAME (sans directory) is used unchanged if that name is free;
>     -otherwise a string <2> or <3> or ... is appended to get an unused name.
>     +otherwise the buffer is renamed according to
>     +`uniquify-buffer-name-style' to get an unused name.
>      
>      Emacs treats buffers whose names begin with a space as internal buffers.
>      To avoid confusion when visiting a file whose name begins with a space,
>     -this function prepends a \"|\" to the final result if necessary."
>     +this function prepends a \"|\" to the final result if necessary.
>     +
>     +If DIRECTORY is non-nil, a file name separator will be added to
>     +the buffer name according to `uniquify-trailing-separator-p'."
>
> Where will that separator be added?

It is added in create-file-buffer, in the calculation of basename.

> And why is the arg called `directory`?

Because we are indicating that this filename is a directory, or at least
that we want to visit this filename as if it was a directory.

> And why/when is that arg needed, since the separator will often be
> introduced anyway by uniquify even with a nil arg?

This is now the only place that uniquify-trailing-separator-p is used,
so the separator won't be introduced if DIRECTORY is nil.