bug#11571: 24.0.97; doc string of `dired-get-subdir-min'

[Stefan Kangas <stefan@marxist.se>]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> "Drew Adams" <drew.adams@oracle.com> writes:
>
>> Actually, the real bug is that `dired-get-subdir-min' has no doc string, so the
>> doc string of `cdr' is used in its stead.  The whole point of defining and using
>> a separate function for this, instead of just coding with `cdr', is to enable
>> readers of the code - and a fortiori the doc - to think in terms of the data
>> structure of directories and not just abstract cons cells.  There should be doc
>> at the proper level of abstraction here.
[...]
> ;; can't use macro,  must be redefinable for other alist format in dired-nstd.
> (defalias 'dired-get-subdir-min 'cdr)
>
> Which is kinda er.
>
> I don't know what nstd is, and the function doesn't seem to be redefined
> anywhere, so is this just legacy gunk?  Should `dired-get-subdir-min'
> just be replaced by `cdr' in all the calling sites?

From searching the web, I think dired-nstd is "dired nested", which
indeed seems to be some old, now non-existent stuff from Emacs 19 or
older.  See this for example:
https://www.cs.cmu.edu/cgi-bin/info2www?(dired.info)Dired%20Known%20Problems

I therefore think it's safe to obsolete this function, like in the below
patch.  Comments?

Best regards,
Stefan Kangas


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-dired-get-subdir-min-obsolete.patch --]
[-- Type: text/x-diff, Size: 5623 bytes --]

From 1a6c799863380e5986f26a9c8fd9d3fc98241bb1 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Thu, 31 Oct 2019 02:39:20 +0100
Subject: [PATCH] Make dired-get-subdir-min obsolete

* lisp/dired.el (dired-get-subdir-min): Redefine as obsolete function
alias for 'cdr'.  (Bug#11571)
(dired-get-subdir, dired-get-subdir-max, dired-clear-alist)
(dired-next-subdir, dired-current-directory):
* lisp/dired-aux.el (dired-rename-subdir-2)
(dired-alist-sort, dired-insert-subdir-del)
(dired-insert-subdir-doupdate, dired-goto-subdir)
(dired-hide-subdir, dired-hide-all): Use 'cdr' instead of
the above obsolete function.
---
 lisp/dired-aux.el | 18 +++++++++---------
 lisp/dired.el     | 13 ++++++-------
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index b1521ecf01..7f690e7e72 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -1713,7 +1713,7 @@ dired-rename-subdir-2
     (let ((regexp (regexp-quote (directory-file-name dir)))
 	  (newtext (directory-file-name to))
 	  buffer-read-only)
-      (goto-char (dired-get-subdir-min elt))
+      (goto-char (cdr elt))
       ;; Update subdir headerline in buffer
       (if (not (looking-at dired-subdir-regexp))
 	  (error "%s not found where expected - dired-subdir-alist broken?"
@@ -2476,8 +2476,8 @@ dired-alist-sort
   (setq dired-subdir-alist
 	(sort dired-subdir-alist
 	      (lambda (elt1 elt2)
-		(> (dired-get-subdir-min elt1)
-		   (dired-get-subdir-min elt2))))))
+		(> (cdr elt1)
+		   (cdr elt2))))))
 
 (defun dired-kill-tree (dirname &optional remember-marks kill-root)
   "Kill all proper subdirs of DIRNAME, excluding DIRNAME itself.
@@ -2520,7 +2520,7 @@ dired-insert-subdir-newpos
 (defun dired-insert-subdir-del (element)
   ;; Erase an already present subdir (given by ELEMENT) from buffer.
   ;; Move to that buffer position.  Return a mark-alist.
-  (let ((begin-marker (dired-get-subdir-min element)))
+  (let ((begin-marker (cdr element)))
     (goto-char begin-marker)
     ;; Are at beginning of subdir (and inside it!).  Now determine its end:
     (goto-char (dired-subdir-max))
@@ -2551,7 +2551,7 @@ dired-insert-subdir-doupdate
   ;; BEG-END is the subdir-region (as list of begin and end).
   (if elt				; subdir was already present
       ;; update its position (should actually be unchanged)
-      (set-marker (dired-get-subdir-min elt) (point-marker))
+      (set-marker (cdr elt) (point-marker))
     (dired-alist-add dirname (point-marker)))
   ;; The hook may depend on the subdir-alist containing the just
   ;; inserted subdir, so run it after dired-alist-add:
@@ -2665,7 +2665,7 @@ dired-goto-subdir
   (setq dir (file-name-as-directory dir))
   (let ((elt (assoc dir dired-subdir-alist)))
     (and elt
-	 (goto-char (dired-get-subdir-min elt))
+	 (goto-char (cdr elt))
 	 ;; dired-subdir-hidden-p and dired-add-entry depend on point being
 	 ;; at \n after this function succeeds.
 	 (progn (end-of-line)
@@ -2763,7 +2763,7 @@ dired-hide-subdir
 	     (end-pos (1- (dired-get-subdir-max elt)))
 	     buffer-read-only)
 	;; keep header line visible, hide rest
-	(goto-char (dired-get-subdir-min elt))
+	(goto-char (cdr elt))
 	(end-of-line)
 	(if hidden-p
 	    (dired--unhide (point) end-pos)
@@ -2782,14 +2782,14 @@ dired-hide-all
       ;; hide
       (let ((pos (point-max)))		; pos of end of last directory
         (dolist (subdir dired-subdir-alist)
-	  (let ((start (dired-get-subdir-min subdir)) ; pos of prev dir
+	  (let ((start (cdr subdir)) ; pos of prev dir
 		(end (save-excursion
 		       (goto-char pos) ; current dir
 		       ;; we're somewhere on current dir's line
 		       (forward-line -1)
 		       (point))))
             (dired--hide start end))
-	  (setq pos (dired-get-subdir-min subdir))))))) ; prev dir gets current dir
+	  (setq pos (cdr subdir))))))) ; prev dir gets current dir
 
 ;;;###end dired-ins.el
 
diff --git a/lisp/dired.el b/lisp/dired.el
index 05789a3516..a94bf528c2 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -2857,21 +2857,20 @@ dired-get-subdir
     (let ((cur-dir (dired-current-directory)))
       (beginning-of-line)		; alist stores b-o-l positions
       (and (zerop (- (point)
-		     (dired-get-subdir-min (assoc cur-dir
+		     (cdr (assoc cur-dir
 						  dired-subdir-alist))))
 	   cur-dir))))
 
-;; can't use macro,  must be redefinable for other alist format in dired-nstd.
-(defalias 'dired-get-subdir-min 'cdr)
+(define-obsolete-function-alias 'dired-get-subdir-min 'cdr "27.1")
 
 (defun dired-get-subdir-max (elt)
   (save-excursion
-    (goto-char (dired-get-subdir-min elt))
+    (goto-char (cdr elt))
     (dired-subdir-max)))
 
 (defun dired-clear-alist ()
   (while dired-subdir-alist
-    (set-marker (dired-get-subdir-min (car dired-subdir-alist)) nil)
+    (set-marker (cdr (car dired-subdir-alist)) nil)
     (setq dired-subdir-alist (cdr dired-subdir-alist))))
 
 (defun dired-subdir-index (dir)
@@ -2895,7 +2894,7 @@ dired-next-subdir
     ;; nth with negative arg does not return nil but the first element
     (setq index (- (dired-subdir-index this-dir) arg))
     (setq pos (if (>= index 0)
-		  (dired-get-subdir-min (nth index dired-subdir-alist))))
+		  (cdr (nth index dired-subdir-alist))))
     (if pos
 	(progn
 	  (goto-char pos)
@@ -3116,7 +3115,7 @@ dired-current-directory
       (setq elt (car alist)
 	    dir (car elt)
 	    ;; use `<=' (not `<') as subdir line is part of subdir
-	    alist (if (<= (dired-get-subdir-min elt) here)
+	    alist (if (<= (cdr elt) here)
 		      nil		; found
 		    (cdr alist))))
     (if localp
-- 
2.20.1