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

[Spencer Baugh <sbaugh@janestreet.com>]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: 62732@debbugs.gnu.org
>> From: sbaugh@catern.com
>> Date: Mon, 10 Jul 2023 01:36:00 +0000 (UTC)
>> 
>> Great!  Here's that as a complete patch again.
>
>> --- a/lisp/dired.el
>> +++ b/lisp/dired.el
>> @@ -1306,7 +1306,7 @@ dired-internal-noselect
>>  	 ;; Note that buffer already is in dired-mode, if found.
>>  	 (new-buffer-p (null buffer)))
>>      (or buffer
>> -        (setq buffer (create-file-buffer (directory-file-name dirname))))
>> +        (setq buffer (create-file-buffer dirname)))
>
> This seems to imply that callers of create-file-buffer will now have
> to remember to ensure the argument ends in a slash if it is the name
> of a directory.  If so, I'd prefer that create-file-buffer did that
> internally, when its argument is a directory.  Callers shouldn't know
> to much about the internals of the callee.

I can (and should) add this to the docstring of create-file-buffer.  It
seems intuitive to me that the last non-empty component of the filename
passed in by the caller is what create-file-buffer uses, including if
that "last component" ends in a slash.  (It's a nice way to avoid the
additional DIRECTORY argument which says whether the filename is
intended to refer to a directory)

By doing this internally in create-file-buffer, you mean running
file-directory-p to see if the filename actually points to an existing
directory?  I'm hesitant to do that:

- That prevents running create-file-buffer to create a buffer to visit a
directory which does not yet exist (in the same way you can visit a file
which does not yet exist).  dired doesn't currently support that but
other packages might want to.

- Checking file-directory-p is what uniquify did which caused these bugs
in the first place, and I think this could partially recreate the same
bug, where we add a trailing slash just because there happens to be a
directory of the right name.  (Although I'm not sure, just worried)

- It adds filesystem access to what is currently a pure function.

> Does this changeset have any user-facing behavior changes?  If so,
> they should be at least in NEWS.

The only user-facing behavior change is fixing the two bugs mentioned in
the commit message.  Is that appropriate to include in NEWS?