bug#3224: 23.0.92; vc-dir vs uniquify: wrong directory used

[Juanma Barranquero]

On Wed, Jan 6, 2010 at 15:28, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> AFAIK it already *does* the right thing (try it with Dired buffers,
> since these are the (only?) ones that should have a directory name),
> unless maybe we don't agree on what is the right thing in that case.

The dired and vc-dired cases are not exactly equivalent. In the dired
case, uniquify-buffer-file-name is called when there's a conflict like

  (dired "/my/dir-1/A")
  (dired "/my/dir-2/A")

(because /my/dir/A vs my/dir/B obviously does not produce any conflict).

In this case, u-b-f-n gets, via `list-buffers-directory', the full
path including the A:  /my/dir1/A, and strips the last element and
returns /my/dir1. That works for uniquify, because it will be getting
path elements from /my/dir1 vs. /my/dir2, just as it needs. The
resulting buffers (with forward syntax) will be "A|dir-1" and
"A|dir-2".

In the OP's vc-dir case, the conflicts happens in this:

  (vc-dir "/my/dir/A")
  (vc-dir "/my/dir/B")

because the conflict uniquify tries to solve is at the buffer-name
level, which is always *vc-dir*. /my/dir/A and /my/dir/B are
directories, and so elements for uniquifying; the expected result is
"*vc-dir*|A" and "*vc-dir*|B". However, u-b-f-n gets "/my/dir/B" (via
list-buffers-directory), which is correct, and again strips the last
element and returns "/my/dir". So uniquify ends producting
"*vc-dir*|A" and "*vc-dir*|dir", which is incorrect.

Now, if you consider than always removing an element from BUFFER is
the right thing to do for u-b-f-n, we'll have to agree to disagree;
IMHO, that's not what its docstring says. From it, I would expect
u-b-f-n to return a directory unchanged. That said, my "fix" to
u-b-f-n would break uniquifying of dired buffers (thanks for pointing
that out), so perhaps we'll have to live with such behavior. In that
case, I'd suggest reworking the docstring of u-b-f-n.

Going with your proposed fix via `list-buffers-directory', the
following patch works. I have not added a comment to the change to
`list-buffers-directory' because I don't really know how to explain
it; it seems a hack to me to force a variable named
`list-buffers-directory' to contain a bogus name part just to help
uniquify.

Comments? Dan, what do you think?

    Juanma



=== modified file 'lisp/vc-dir.el'
--- lisp/vc-dir.el      2009-12-05 00:24:03 +0000
+++ lisp/vc-dir.el      2010-01-07 11:09:16 +0000
@@ -101,7 +101,9 @@
                       (return buffer))))))))
     (or buf
         ;; Create a new buffer named BNAME.
-        (with-current-buffer (create-file-buffer bname)
+       ;; We pass a filename to create-file-buffer because it is what
+       ;; the function expects, and also what uniquify needs (if active)
+        (with-current-buffer (create-file-buffer (expand-file-name bname dir))
           (cd dir)
           (vc-setup-buffer (current-buffer))
           ;; Reset the vc-parent-buffer-name so that it does not appear
@@ -928,7 +930,7 @@
     (set (make-local-variable 'vc-ewoc) (ewoc-create #'vc-dir-printer))
     (set (make-local-variable 'revert-buffer-function)
         'vc-dir-revert-buffer-function)
-    (setq list-buffers-directory default-directory)
+    (setq list-buffers-directory (expand-file-name "*vc-dir*"
default-directory))
     (add-to-list 'vc-dir-buffers (current-buffer))
     ;; Make sure that if the directory buffer is killed, the update
     ;; process running in the background is also killed.