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

[Eli Zaretskii <eliz@gnu.org>]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: sbaugh@catern.com,  monnier@iro.umontreal.ca,  62732@debbugs.gnu.org
> Date: Mon, 10 Jul 2023 09:39:10 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> -        (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)

If the only reason is to avoid an additional argument, I'd prefer an
additional argument.  Documenting a problematic design doesn't mean
the design ceases to be problematic, and relies too heavily on people
paying attention to subtle aspects of the documented behavior.

> 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.

Didn't that problem exist before your changes?

And anyway, if we want to support that, adding an extra variable, or
even requiring the trailing slash only for non-existing directories,
would be a better solution.

> - 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)

I don't see why that would follow.  It is a very minor change in the
code, and doesn't affect the logic, AFAICT.

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

But create-file-buffer is not documented as not hitting the disk, so I
don't see a problem here.

> > 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?

Does the fix result in different buffer names?  If so, it is not just
a bugfix, it changes user-facing behavior.