Subject: Re: bug#26712: other-window/frame versions of find-library

From: "Charles A. Roelli" <[email-masked]>
Date: Wed, 17 May 2017 21:16:57 +0200

Thank you for your comments.  Please see again the attachment.

On 16/05/2017 21:36, Eli Zaretskii wrote:
>> +** Two new commands 'find-library-other-window' and
>> +'find-library-other-frame'.
> This is too terse.  A few words regarding what these do would be much
> better.

Updated:

+** Two new commands for finding the source code of Emacs Lisp
+libraries: 'find-library-other-window' and 'find-library-other-frame'.
+

>> +(defun find-library (library)
>> +  "Find the Emacs Lisp source of the LIBRARY near point.
> This is confusing, IMO.  Suggest to reword:
>
>      Find the Emacs Lisp source of LIBRARY.
>    Interactively, prompt for LIBRARY using the one at or near point as
>    the default.

Thanks, I've updated it.

>> +(defun read-library-name ()
>> +  "Read and return a library name, defaulting to the one near point."
> I would explain here what constitutes a "library name", i.e. how does
> the function determine the library name at point.

+A library name is the filename of an Emacs Lisp library located
+in a directory under `load-path' (or `find-function-source-path',
+if non-nil)."

>> +(defun find-library-other-window (library)
>> +  "Find, in another window, the file defining LIBRARY at or near point.
> Any reason why the wording is different from that of find-library?

Nope, I've also amended this.

>From 44f5a4f6fce2002c4d0c0030a3ef60d31f4b649a Mon Sep 17 00:00:00 2001
From: Charles A. Roelli <[email-masked]>
Date: Wed, 17 May 2017 21:13:17 +0200
Subject: [PATCH] New commands: find-library-other-window,
 find-library-other-frame

* lisp/emacs-lisp/find-func.el (find-library-other-window)
(find-library-other-frame): New commands to complement the existing
`find-library' command.
(read-library-name): New function to read a library name.
---
 etc/NEWS                     |    3 +
 lisp/emacs-lisp/find-func.el |   92 ++++++++++++++++++++++++++----------------
 2 files changed, 60 insertions(+), 35 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 25f0f18..2b04ea5 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -344,6 +344,9 @@ for DNS-querying functions 'nslookup-host', 'dns-lookup-host',
 and 'run-dig'.  Each function now accepts an optional name server
 argument interactively (with a prefix argument) and non-interactively.
 
+** Two new commands for finding the source code of Emacs Lisp
+libraries: 'find-library-other-window' and 'find-library-other-frame'.
+
 
 * Editing Changes in Emacs 26.1
 
diff --git a/lisp/emacs-lisp/find-func.el b/lisp/emacs-lisp/find-func.el
index d0acc14..9b98f05 100644
--- a/lisp/emacs-lisp/find-func.el
+++ b/lisp/emacs-lisp/find-func.el
@@ -271,43 +271,65 @@ find-function-C-source
     (cons (current-buffer) (match-beginning 0))))
 
 ;;;###autoload
-(defun find-library (library &optional other-window)
+(defun find-library (library)
   "Find the Emacs Lisp source of LIBRARY.
-LIBRARY should be a string (the name of the library).  If the
-optional OTHER-WINDOW argument (i.e., the command argument) is
-specified, pop to a different window before displaying the
-buffer."
-  (interactive
-   (let* ((dirs (or find-function-source-path load-path))
-          (suffixes (find-library-suffixes))
-          (table (apply-partially 'locate-file-completion-table
-                                  dirs suffixes))
-	  (def (if (eq (function-called-at-point) 'require)
-		   ;; `function-called-at-point' may return 'require
-		   ;; with `point' anywhere on this line.  So wrap the
-		   ;; `save-excursion' below in a `condition-case' to
-		   ;; avoid reporting a scan-error here.
-		   (condition-case nil
-		       (save-excursion
-			 (backward-up-list)
-			 (forward-char)
-			 (forward-sexp 2)
-			 (thing-at-point 'symbol))
-		     (error nil))
-		 (thing-at-point 'symbol))))
-     (when (and def (not (test-completion def table)))
-       (setq def nil))
-     (list
-      (completing-read (if def
-                           (format "Library name (default %s): " def)
-			 "Library name: ")
-		       table nil nil nil nil def)
-      current-prefix-arg)))
+
+Interactively, prompt for LIBRARY using the one at or near point."
+  (interactive (list (read-library-name)))
+  (prog1
+      (switch-to-buffer (find-file-noselect (find-library-name library)))
+    (run-hooks 'find-function-after-hook)))
+
+(defun read-library-name ()
+  "Read and return a library name, defaulting to the one near point.
+
+A library name is the filename of an Emacs Lisp library located
+in a directory under `load-path' (or `find-function-source-path',
+if non-nil)."
+  (let* ((dirs (or find-function-source-path load-path))
+         (suffixes (find-library-suffixes))
+         (table (apply-partially 'locate-file-completion-table
+                                 dirs suffixes))
+         (def (if (eq (function-called-at-point) 'require)
+                  ;; `function-called-at-point' may return 'require
+                  ;; with `point' anywhere on this line.  So wrap the
+                  ;; `save-excursion' below in a `condition-case' to
+                  ;; avoid reporting a scan-error here.
+                  (condition-case nil
+                      (save-excursion
+                        (backward-up-list)
+                        (forward-char)
+                        (forward-sexp 2)
+                        (thing-at-point 'symbol))
+                    (error nil))
+                (thing-at-point 'symbol))))
+    (when (and def (not (test-completion def table)))
+      (setq def nil))
+    (completing-read (if def
+                         (format "Library name (default %s): " def)
+                       "Library name: ")
+                     table nil nil nil nil def)))
+
+;;;###autoload
+(defun find-library-other-window (library)
+  "Find the Emacs Lisp source of LIBRARY in another window.
+
+See `find-library' for more details."
+  (interactive (list (read-library-name)))
+  (prog1
+      (switch-to-buffer-other-window (find-file-noselect
+                                      (find-library-name library)))
+    (run-hooks 'find-function-after-hook)))
+
+;;;###autoload
+(defun find-library-other-frame (library)
+  "Find the Emacs Lisp source of LIBRARY in another frame.
+
+See `find-library' for more details."
+  (interactive (list (read-library-name)))
   (prog1
-      (funcall (if other-window
-                   'pop-to-buffer
-                 'pop-to-buffer-same-window)
-               (find-file-noselect (find-library-name library)))
+      (switch-to-buffer-other-frame (find-file-noselect
+                                     (find-library-name library)))
     (run-hooks 'find-function-after-hook)))
 
 ;;;###autoload
-- 
1.7.4.4