Subject: Re: bug#21981: request: dired: require dired-x by default

From: Stefan Kangas <[email-masked]>
Date: Sat, 29 Aug 2020 09:16:20 -0700

John Wiegley <[email-masked]> writes:

>>>>>> Glenn Morris <[email-masked]> writes:
>
>> AFAIK we are still waiting for someone to
>>   [add] the remaining useful functionality of dired-x into dired and
>>   then [make] dired-x obsolete.
>
> I would prefer this also, rather than pulling in all of dired-x. Michael,
> would you like to review the parts of dired-x that should move over?

I took a stab at one part of this: dired-jump and
dired-jump-other-window.  Please see the attached patch.

The patch is slightly opinionated in that it obsoletes `dired-bind-jump'
and makes it a no-op: it binds `C-x C-j' and `C-x 4 C-j'
unconditionally.  As a replacement, it documents in NEWS how to unbind
them in your init file.  I think this makes much more sense as an
interface to this functionality.

I also moved the corresponding documentation to the Manual and took the
opportunity to rewrite and improve the documentation of those commands.

Best regards,
Stefan Kangas
From bb84918eaa6d615a1356b9761dbc2e1ab3530867 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <[email-masked]>
Date: Sat, 29 Aug 2020 17:37:40 +0200
Subject: [PATCH] Move dired-jump from dired-x to dired (Bug#21981)

* lisp/dired-x.el (dired-bind-jump): Change into defvar and make
obsolete.
(dired-extra-startup): Doc fix.
(dired-jump, dired-jump-other-window): Move from here...
* lisp/dired.el (dired-jump, dired-jump-other-window): ...to here.

* lisp/bindings.el (ctl-x-map, ctl-x-4-map): Bind 'C-j' to
'dired-jump' and 'dired-jump-other-window'.

* doc/misc/dired-x.texi (Features, Installation)
(Miscellaneous Commands): Remove documentation of 'dired-jump'.
* doc/emacs/dired.texi (Dired Enter): Document 'dired-jump' and
dired-jump-other-window.
* etc/NEWS: Announce the above changes.

* test/lisp/dired-tests.el (dired-autoload): Adjust test.
---
 doc/emacs/dired.texi     | 10 +++++
 doc/misc/dired-x.texi    | 50 +------------------------
 etc/NEWS                 | 13 +++++++
 lisp/bindings.el         |  3 ++
 lisp/dired-x.el          | 79 +---------------------------------------
 lisp/dired.el            | 64 ++++++++++++++++++++++++++++++++
 test/lisp/dired-tests.el |  4 +-
 7 files changed, 96 insertions(+), 127 deletions(-)

diff --git a/doc/emacs/dired.texi b/doc/emacs/dired.texi
index 19aaca962d..0a6cf322f6 100644
--- a/doc/emacs/dired.texi
+++ b/doc/emacs/dired.texi
@@ -109,6 +109,16 @@ Dired Enter
   You can also invoke Dired by giving @kbd{C-x C-f} (@code{find-file})
 a directory's name.
 
+@findex dired-jump
+@findex dired-jump-other-window
+@kindex C-x C-j
+@kindex C-x 4 C-j
+  Typing @kbd{C-x C-j} (@code{dired-jump}) in any buffer will open a
+Dired buffer and move point to the line of the current file.  In
+Dired, move up a level and go to the previous directory's line.
+Typing @kbd{C-x 4 C-j} (@code{dired-jump-other-window} has the same
+effect but opens a new window for the Dired buffer.
+
   The variable @code{dired-listing-switches} specifies the options to
 give to @command{ls} for listing the directory; this string
 @emph{must} contain @samp{-l}.  If you use a prefix argument with the
diff --git a/doc/misc/dired-x.texi b/doc/misc/dired-x.texi
index d749780660..243b59b242 100644
--- a/doc/misc/dired-x.texi
+++ b/doc/misc/dired-x.texi
@@ -150,10 +150,8 @@ Features
 
 @noindent
 @file{dired-x.el} binds some functions to keys in Dired Mode (@pxref{Key
-Index}) and also binds @kbd{C-x C-j} and @kbd{C-x 4 C-j} @emph{globally} to
-@code{dired-jump} (@pxref{Miscellaneous Commands}).  Optionally, it
-also binds @kbd{C-x C-f} and @kbd{C-x 4 C-f} to
-@code{dired-x-find-file} and @code{dired-x-find-file-other-window},
+Index}).  Optionally, it also binds @kbd{C-x C-f} and @kbd{C-x 4 C-f}
+to @code{dired-x-find-file} and @code{dired-x-find-file-other-window},
 respectively (@pxref{Find File At Point}).
 
 @node Technical Details
@@ -204,32 +202,10 @@ Installation
 
 @ifnottex
 @menu
-* Optional Installation Dired Jump::
 * Optional Installation File At Point::
 @end menu
 @end ifnottex
 
-@node Optional Installation Dired Jump
-@section Optional Installation Dired Jump
-
-@cindex autoloading @code{dired-jump} and @code{dired-jump-other-window}
-
-In order to have @code{dired-jump} and @code{dired-jump-other-window}
-(@pxref{Miscellaneous Commands}) work @emph{before} @code{dired} and
-@code{dired-x} have been properly loaded you should set-up an autoload
-for these functions.  In your @file{.emacs} file put
-
-@example
-(autoload 'dired-jump "dired-x"
-  "Jump to Dired buffer corresponding to current buffer." t)
-
-(autoload 'dired-jump-other-window "dired-x"
-  "Like \\[dired-jump] (dired-jump) but in other window." t)
-
-(define-key global-map "\C-x\C-j" 'dired-jump)
-(define-key global-map "\C-x4\C-j" 'dired-jump-other-window)
-@end example
-
 @node Optional Installation File At Point
 @section Optional Installation File At Point
 
@@ -919,28 +895,6 @@ Miscellaneous Commands
 
 @table @code
 
-@item dired-jump
-@findex dired-jump
-@kindex C-x C-j
-@cindex jumping to Dired listing containing file.
-Bound to @kbd{C-x C-j}.  Jump back to Dired: If in a file, edit the current
-directory and move to file's line.  If in Dired already, pop up a level and
-go to old directory's line.  In case the proper Dired file line cannot be
-found, refresh the Dired buffer and try again.
-
-@item dired-jump-other-window
-@findex dired-jump-other-window
-@kindex C-x 4 C-j
-Bound to @kbd{C-x 4 C-j}. Like @code{dired-jump}, but to other window.
-
-These functions can be autoloaded so they work even though @file{dired-x.el}
-has not been loaded yet (@pxref{Optional Installation Dired Jump}).
-
-@vindex dired-bind-jump
-If the variable @code{dired-bind-jump} is @code{nil}, @code{dired-jump} will not be
-bound to @kbd{C-x C-j} and @code{dired-jump-other-window} will not be bound to
-@kbd{C-x 4 C-j}.
-
 @item dired-vm
 @cindex reading mail.
 @kindex V
diff --git a/etc/NEWS b/etc/NEWS
index ad63955f7b..3a6c5fbc8d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -272,6 +272,19 @@ details of marking the file at the end of the region.
 *** State changing VC operations are supported in Dired on files and
 directories with the help of new command 'dired-vc-next-action'.
 
++++
+*** Moved commands from dired-x.
+The 'dired-jump' and 'dired-jump-other-window' commands have been
+moved from the 'dired-x' package to 'dired'.  The variable
+'dired-bind-jump' no longer has any effect and is now obsolete.
+The commands are bound to 'C-x C-j' and 'C-x 4 C-j'.
+
+To get the old behavior of 'dired-bind-jump' back and unbind the above
+keys, add the following to your Init file:
+
+(global-set-key "\C-x\C-j" nil)
+(global-set-key "\C-x4\C-j" nil)
+
 ** Change Logs and VC
 
 *** More VC commands can be used from non-file buffers.
diff --git a/lisp/bindings.el b/lisp/bindings.el
index e3fc5637fa..20342bc40a 100644
--- a/lisp/bindings.el
+++ b/lisp/bindings.el
@@ -1383,6 +1383,9 @@ esc-map
 (define-key ctl-x-map "'" 'expand-abbrev)
 (define-key ctl-x-map "\C-b" 'list-buffers)
 
+(define-key ctl-x-map "\C-j" 'dired-jump)
+(define-key ctl-x-4-map "\C-j" 'dired-jump-other-window)
+
 (define-key ctl-x-map "z" 'repeat)
 
 (define-key esc-map "\C-l" 'reposition-window)
diff --git a/lisp/dired-x.el b/lisp/dired-x.el
index 873d586ca1..e99aa32140 100644
--- a/lisp/dired-x.el
+++ b/lisp/dired-x.el
@@ -64,21 +64,8 @@ dired-bind-vm
   :type 'boolean
   :group 'dired-keys)
 
-(defcustom dired-bind-jump t
-  "Non-nil means bind `dired-jump' to C-x C-j, otherwise do not.
-Setting this variable directly after dired-x is loaded has no effect -
-use \\[customize]."
-  :type 'boolean
-  :set (lambda (sym val)
-         (if (set sym val)
-             (progn
-               (define-key ctl-x-map "\C-j" 'dired-jump)
-               (define-key ctl-x-4-map "\C-j" 'dired-jump-other-window))
-           (if (eq 'dired-jump (lookup-key ctl-x-map "\C-j"))
-               (define-key ctl-x-map "\C-j" nil))
-           (if (eq 'dired-jump-other-window (lookup-key ctl-x-4-map "\C-j"))
-               (define-key ctl-x-4-map "\C-j" nil))))
-  :group 'dired-keys)
+(defvar dired-bind-jump t)
+(make-obsolete-variable 'dired-bind-jump "not used." "28.1")
 
 (defcustom dired-bind-man t
   "Non-nil means bind `dired-man' to \"N\" in Dired, otherwise do not.
@@ -307,7 +294,6 @@ dired-extra-startup
 See also the functions:
   `dired-flag-extension'
   `dired-virtual'
-  `dired-jump'
   `dired-man'
   `dired-vm'
   `dired-rmail'
@@ -447,68 +433,7 @@ dired-very-clean-tex
                                 dired-bibtex-unclean-extensions
                                 dired-tex-unclean-extensions
                                 (list ".dvi"))))
-
-(defvar archive-superior-buffer)
-(defvar tar-superior-buffer)
-;;; JUMP.
 
-;;;###autoload
-(defun dired-jump (&optional other-window file-name)
-  "Jump to Dired buffer corresponding to current buffer.
-If in a file, Dired the current directory and move to file's line.
-If in Dired already, pop up a level and goto old directory's line.
-In case the proper Dired file line cannot be found, refresh the dired
-buffer and try again.
-When OTHER-WINDOW is non-nil, jump to Dired buffer in other window.
-When FILE-NAME is non-nil, jump to its line in Dired.
-Interactively with prefix argument, read FILE-NAME."
-  (interactive
-   (list nil (and current-prefix-arg
-                  (read-file-name "Jump to Dired file: "))))
-  (cond
-   ((and (bound-and-true-p archive-subfile-mode)
-         (buffer-live-p archive-superior-buffer))
-    (switch-to-buffer archive-superior-buffer))
-   ((and (bound-and-true-p tar-subfile-mode)
-         (buffer-live-p tar-superior-buffer))
-    (switch-to-buffer tar-superior-buffer))
-   (t
-    ;; Expand file-name before `dired-goto-file' call:
-    ;; `dired-goto-file' requires its argument to be an absolute
-    ;; file name; the result of `read-file-name' could be
-    ;; an abbreviated file name (Bug#24409).
-    (let* ((file (or (and file-name (expand-file-name file-name))
-                     buffer-file-name))
-           (dir (if file (file-name-directory file) default-directory)))
-      (if (and (eq major-mode 'dired-mode) (null file-name))
-          (progn
-            (setq dir (dired-current-directory))
-            (dired-up-directory other-window)
-            (unless (dired-goto-file dir)
-              ;; refresh and try again
-              (dired-insert-subdir (file-name-directory dir))
-              (dired-goto-file dir)))
-        (if other-window
-            (dired-other-window dir)
-          (dired dir))
-        (if file
-            (or (dired-goto-file file)
-                ;; refresh and try again
-                (progn
-                  (dired-insert-subdir (file-name-directory file))
-                  (dired-goto-file file))
-                ;; Toggle omitting, if it is on, and try again.
-                (when dired-omit-mode
-                  (dired-omit-mode)
-                  (dired-goto-file file)))))))))
-
-;;;###autoload
-(defun dired-jump-other-window (&optional file-name)
-  "Like \\[dired-jump] (`dired-jump') but in other window."
-  (interactive
-   (list (and current-prefix-arg
-	      (read-file-name "Jump to Dired file: "))))
-  (dired-jump t file-name))
 
 ;;; OMITTING.
 
diff --git a/lisp/dired.el b/lisp/dired.el
index d122869a5e..71e851e2c4 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -4465,6 +4465,70 @@ dired-restore-desktop-buffer
 (add-to-list 'desktop-buffer-mode-handlers
 	     '(dired-mode . dired-restore-desktop-buffer))
 
+
+;;;; Jump to Dired
+
+(defvar archive-superior-buffer)
+(defvar tar-superior-buffer)
+
+;;;###autoload
+(defun dired-jump (&optional other-window file-name)
+  "Jump to Dired buffer corresponding to current buffer.
+If in a file, Dired the current directory and move to file's line.
+If in Dired already, pop up a level and goto old directory's line.
+In case the proper Dired file line cannot be found, refresh the dired
+buffer and try again.
+When OTHER-WINDOW is non-nil, jump to Dired buffer in other window.
+When FILE-NAME is non-nil, jump to its line in Dired.
+Interactively with prefix argument, read FILE-NAME."
+  (interactive
+   (list nil (and current-prefix-arg
+                  (read-file-name "Jump to Dired file: "))))
+  (cond
+   ((and (bound-and-true-p archive-subfile-mode)
+         (buffer-live-p archive-superior-buffer))
+    (switch-to-buffer archive-superior-buffer))
+   ((and (bound-and-true-p tar-subfile-mode)
+         (buffer-live-p tar-superior-buffer))
+    (switch-to-buffer tar-superior-buffer))
+   (t
+    ;; Expand file-name before `dired-goto-file' call:
+    ;; `dired-goto-file' requires its argument to be an absolute
+    ;; file name; the result of `read-file-name' could be
+    ;; an abbreviated file name (Bug#24409).
+    (let* ((file (or (and file-name (expand-file-name file-name))
+                     buffer-file-name))
+           (dir (if file (file-name-directory file) default-directory)))
+      (if (and (eq major-mode 'dired-mode) (null file-name))
+          (progn
+            (setq dir (dired-current-directory))
+            (dired-up-directory other-window)
+            (unless (dired-goto-file dir)
+              ;; refresh and try again
+              (dired-insert-subdir (file-name-directory dir))
+              (dired-goto-file dir)))
+        (if other-window
+            (dired-other-window dir)
+          (dired dir))
+        (if file
+            (or (dired-goto-file file)
+                ;; refresh and try again
+                (progn
+                  (dired-insert-subdir (file-name-directory file))
+                  (dired-goto-file file))
+                ;; Toggle omitting, if it is on, and try again.
+                (when dired-omit-mode
+                  (dired-omit-mode)
+                  (dired-goto-file file)))))))))
+
+;;;###autoload
+(defun dired-jump-other-window (&optional file-name)
+  "Like \\[dired-jump] (`dired-jump') but in other window."
+  (interactive
+   (list (and current-prefix-arg
+	      (read-file-name "Jump to Dired file: "))))
+  (dired-jump t file-name))
+
 (provide 'dired)
 
 (run-hooks 'dired-load-hook)		; for your customizations
diff --git a/test/lisp/dired-tests.el b/test/lisp/dired-tests.el
index 5c6649cba4..cec533ddfa 100644
--- a/test/lisp/dired-tests.el
+++ b/test/lisp/dired-tests.el
@@ -24,11 +24,11 @@
 (ert-deftest dired-autoload ()
   "Tests to see whether dired-x has been autoloaded"
   (should
-   (fboundp 'dired-jump))
+   (fboundp 'dired-do-relsymlink))
   (should
    (autoloadp
     (symbol-function
-     'dired-jump))))
+     'dired-do-relsymlink))))
 
 (ert-deftest dired-test-bug22694 ()
   "Test for https://debbugs.gnu.org/22694 ."
-- 
2.28.0