bug#47132: 28.0.50; [PATCH] Uniquify: trailing-separator-p + strip-common-suffix

bug#47132: 28.0.50; [PATCH] Uniquify: trailing-separator-p + strip-common-suffix

[Gabriel]

[-- Attachment #1: Type: text/plain, Size: 2938 bytes --]

A possible bug exists on uniquify.el when 'uniquify-strip-common-suffix'
and 'uniquify-trailing-separator-p' are both set to true.

Case 1: when 'uniquify-strip-common-suffix' is t and
'uniquify-trailing-separator-p' is nil

1. emacs -Q
2. Configure uniquify:
(setq-default uniquify-buffer-name-style 'forward
              uniquify-strip-common-suffix t
              uniquify-trailing-separator-p nil)
3. Open two similar dired buffers, e.g.:
C-x C-f "~/A/gnu/emacs/lisp" RET
C-x C-f "~/B/gnu/emacs/lisp" RET
4. The buffers will be renamed to 'A/lisp' and 'B/lisp' as expected,
because 'uniquify-strip-common-suffix' is true and
'uniquify-trailing-separator-p' is nil

Case 2: when 'uniquify-strip-common-suffix' is nil and
'uniquify-trailing-separator-p' is t

1. emacs -Q
2. Configure uniquify:
(setq-default uniquify-buffer-name-style 'forward
              uniquify-strip-common-suffix nil
              uniquify-trailing-separator-p t)
3. Open two similar dired buffers, e.g.:
C-x C-f "~/A/gnu/emacs/lisp" RET
C-x C-f "~/B/gnu/emacs/lisp" RET
4. The buffers will be renamed to 'A/gnu/emacs/lisp/' and
'B/gnu/emacs/lisp/' as expected, because 'uniquify-strip-common-suffix' is nil and
'uniquify-trailing-separator-p' is true

Case 3: when 'uniquify-strip-common-suffix' is t and
'uniquify-trailing-separator-p' is t

1. emacs -Q
2. Configure uniquify:
(setq-default uniquify-buffer-name-style 'forward
              uniquify-strip-common-suffix t
              uniquify-trailing-separator-p t)
3. Open two similar dired buffers, e.g.:
C-x C-f "~/A/gnu/emacs/lisp" RET
C-x C-f "~/B/gnu/emacs/lisp" RET
4. The buffers will be renamed to 'A/lisp' and 'B/lisp'. The common
suffix was removed as expected, but the trailing separator is missing

Cause: It seems that the following verification on 'uniquify-get-proposed-name'
fails because dirname is not the true dirname, but the one with the
common suffix stripped. Thus, 'file-directory-p' fails to find the
directory:

;; Distinguish directories by adding extra separator.
(if (and uniquify-trailing-separator-p
         (file-directory-p (expand-file-name base dirname)) ;; <-- HERE
         (not (string-equal base "")))
    (cond ((eq uniquify-buffer-name-style 'forward)
           (setq base (file-name-as-directory base)))
          ;; (setq base (concat base "/")))
          ((eq uniquify-buffer-name-style 'reverse)
           (setq base (concat (or uniquify-separator "\\") base)))))

The dirname on the line above is '~/A' and '~/B' for each invocation
(common suffix was stripped), so 'expand-file-name' expands to
'~/A/lisp' and '~/B/lisp' and 'file-directory-p' fails to find it. The
function that removes the common suffix is 'uniquify-rationalize'.

I managed to create a patch where the original dirname is preserved in
the 'uniquify-item' struct and passed along as an optional argument, so
'uniquify-get-proposed-name' can properly verify if the directory
exists.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-uniquify-trailing-separator-p-work-with-uniquif.patch --]
[-- Type: text/x-diff, Size: 3981 bytes --]

From 2ca185f2f7fbbc7f68f78c0d052242a3329a6e53 Mon Sep 17 00:00:00 2001
From: Gabriel do Nascimento Ribeiro <gabriel.nascimento@nubank.com.br>
Date: Sun, 14 Mar 2021 02:33:20 -0300
Subject: [PATCH] Make uniquify-trailing-separator-p work with
 uniquify-strip-common-suffix

lisp/uniquify.el:
(uniquify-item): New slot 'original-dirname'.
(uniquify-rationalize-file-buffer-names): Use new slot.
(uniquify-rationalize): Use new slot.
(uniquify-get-proposed-name): New optional argument 'original-dirname'
to properly add a trailing separator when the corresponding user option
is set and the dirname is an existing directory.
---
 lisp/uniquify.el | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/lisp/uniquify.el b/lisp/uniquify.el
index c1ec90e290..9c0576ee93 100644
--- a/lisp/uniquify.el
+++ b/lisp/uniquify.el
@@ -175,8 +175,8 @@ uniquify-list-buffers-directory-modes
 (cl-defstruct (uniquify-item
 	    (:constructor nil) (:copier nil)
 	    (:constructor uniquify-make-item
-	     (base dirname buffer &optional proposed)))
-  base dirname buffer proposed)
+	     (base dirname buffer &optional proposed original-dirname)))
+  base dirname buffer proposed original-dirname)
 
 ;; Internal variables used free
 (defvar uniquify-possibly-resolvable nil)
@@ -211,7 +211,7 @@ uniquify-rationalize-file-buffer-names
   (with-current-buffer newbuf (setq uniquify-managed nil))
   (when dirname
     (setq dirname (expand-file-name (directory-file-name dirname)))
-    (let ((fix-list (list (uniquify-make-item base dirname newbuf)))
+    (let ((fix-list (list (uniquify-make-item base dirname newbuf nil dirname)))
 	  items)
       (dolist (buffer (buffer-list))
 	(when (and (not (and uniquify-ignore-buffers-re
@@ -284,7 +284,9 @@ uniquify-rationalize
       ;; Refresh the dirnames and proposed names.
       (setf (uniquify-item-proposed item)
 	    (uniquify-get-proposed-name (uniquify-item-base item)
-					(uniquify-item-dirname item)))
+					(uniquify-item-dirname item)
+                                        nil
+                                        (uniquify-item-original-dirname item)))
       (setq uniquify-managed fix-list)))
   ;; Strip any shared last directory names of the dirname.
   (when (and (cdr fix-list) uniquify-strip-common-suffix)
@@ -307,7 +309,8 @@ uniquify-rationalize
 					      (uniquify-item-dirname item))))
 				      (and f (directory-file-name f)))
 				    (uniquify-item-buffer item)
-				    (uniquify-item-proposed item))
+				    (uniquify-item-proposed item)
+                                    (uniquify-item-original-dirname item))
 		fix-list)))))
   ;; If uniquify-min-dir-content is 0, this will end up just
   ;; passing fix-list to uniquify-rationalize-conflicting-sublist.
@@ -335,13 +338,13 @@ uniquify-rationalize-a-list
     (uniquify-rationalize-conflicting-sublist conflicting-sublist
 					      old-proposed depth)))
 
-(defun uniquify-get-proposed-name (base dirname &optional depth)
+(defun uniquify-get-proposed-name (base dirname &optional depth original-dirname)
   (unless depth (setq depth uniquify-min-dir-content))
   (cl-assert (equal (directory-file-name dirname) dirname)) ;No trailing slash.
 
   ;; Distinguish directories by adding extra separator.
   (if (and uniquify-trailing-separator-p
-	   (file-directory-p (expand-file-name base dirname))
+	   (file-directory-p (expand-file-name base original-dirname))
 	   (not (string-equal base "")))
       (cond ((eq uniquify-buffer-name-style 'forward)
 	     (setq base (file-name-as-directory base)))
@@ -410,7 +413,8 @@ uniquify-rationalize-conflicting-sublist
 		  (uniquify-get-proposed-name
 		   (uniquify-item-base item)
 		   (uniquify-item-dirname item)
-		   depth)))
+		   depth
+                   (uniquify-item-original-dirname item))))
 	  (uniquify-rationalize-a-list conf-list depth))
       (unless (string= old-name "")
 	(uniquify-rename-buffer (car conf-list) old-name)))))
-- 
2.27.0

bug#47132: 28.0.50; [PATCH] Uniquify: trailing-separator-p + strip-common-suffix

[Lars Ingebrigtsen]

Gabriel <gabriel376@hotmail.com> writes:

> I managed to create a patch where the original dirname is preserved in
> the 'uniquify-item' struct and passed along as an optional argument, so
> 'uniquify-get-proposed-name' can properly verify if the directory
> exists.

Your patch fixes the problem here, too, so I've applied it to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no