[Emacs-diffs] master 7c30ad1: Restore focus to Bookmark List after editing annotation

[Eli Zaretskii]

branch: master
commit 7c30ad184fee3254e3f37601c9af8280f15e3409
Author: Stefan Kangas <address@hidden>
Commit: Eli Zaretskii <address@hidden>

    Restore focus to Bookmark List after editing annotation
    
    There are two entry points to bookmark-edit-annotation-mode: the first
    when we add a bookmark and bookmark-use-annotations is non-nil; the
    second when bookmark-bmenu-edit-annotation is run from the bookmark
    list.  When editing is concluded, in the first case, we should just
    quit window.  In the second case, we should instead return to the
    bookmark list.
    
    * lisp/bookmark.el (text-property-search): Require.
    (bookmark-annotation-name): Make buffer-local and improve doc string.
    (bookmark--annotation-from-bookmark-list): New buffer-local variable.
    (bookmark-edit-annotation): New argument from-bookmark-list sets
    bookmark--annotation-from-bookmark-list.
    (bookmark-bmenu-edit-annotation): Call bookmark-edit-annotation with
    argument from-bookmark-list set to t.
    (bookmark-send-edited-annotation): When editing originated in the
    bookmark list, restore focus to bookmark list and move point back to
    edited bookmark. (Bug#20150)
    
    (bookmark-edit-annotation-mode): Fix typo.
    (bookmark-bmenu-buffer): New variable.
    (bookmark-bmenu-surreptitiously-rebuild-list)
    (bookmark-bmenu-list): Use it.
    
    * test/lisp/bookmark-tests.el (with-bookmark-bmenu-test): New macro.
    (bookmark-tests-set/bookmark-use-annotations-t)
    (bookmark-bmenu-edit-annotation/show-annotation)
    (bookmark-bmenu-send-edited-annotation)
    (bookmark-bmenu-send-edited-annotation/restore-focus): New test cases.
---
 lisp/bookmark.el            | 49 ++++++++++++++++++++++++++++-----------------
 test/lisp/bookmark-tests.el | 49 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 79 insertions(+), 19 deletions(-)

diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index 2640b21..f564cd6 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -32,6 +32,7 @@
 ;;; Code:
 
 (require 'pp)
+(require 'text-property-search)
 (eval-when-compile (require 'cl-lib))
 
 ;;; Misc comments:
@@ -122,6 +123,9 @@ recently set ones come first, oldest ones come last)."
   "Non-nil means show annotations when jumping to a bookmark."
   :type 'boolean)
 
+(defconst bookmark-bmenu-buffer "*Bookmark List*"
+  "Name of buffer used for Bookmark List.")
+
 (defcustom bookmark-bmenu-use-header-line t
   "Non-nil means to use an immovable header line.
 This is as opposed to inline text at the top of the buffer."
@@ -889,13 +893,13 @@ Does not affect the kill ring."
     (when (and newline-too (= (following-char) ?\n))
       (delete-char 1))))
 
-
-;; Defvars to avoid compilation warnings:
 (defvar bookmark-annotation-name nil
-  "Variable holding the name of the bookmark.
-This is used in `bookmark-edit-annotation' to record the bookmark
-whose annotation is being edited.")
+  "Name of bookmark under edit in `bookmark-edit-annotation-mode'.")
+(make-variable-buffer-local 'bookmark-annotation-name)
 
+(defvar bookmark--annotation-from-bookmark-list nil
+  "If non-nil, `bookmark-edit-annotation-mode' should return to bookmark 
list.")
+(make-variable-buffer-local 'bookmark--annotation-from-bookmark-list)
 
 (defun bookmark-default-annotation-text (bookmark-name)
   "Return default annotation text for BOOKMARK-NAME.
@@ -937,7 +941,7 @@ It takes one argument, the name of the bookmark, as a 
string.")
 (define-derived-mode bookmark-edit-annotation-mode
   text-mode "Edit Bookmark Annotation"
   "Mode for editing the annotation of bookmarks.
-When you have finished composing, type \\[bookmark-send-annotation].
+When you have finished composing, type \\[bookmark-send-edited-annotation].
 
 \\{bookmark-edit-annotation-mode-map}")
 
@@ -955,21 +959,31 @@ Lines beginning with `#' are ignored."
       (forward-line 1)))
   ;; Take no chances with text properties.
   (let ((annotation (buffer-substring-no-properties (point-min) (point-max)))
-       (bookmark-name bookmark-annotation-name))
+        (bookmark-name bookmark-annotation-name)
+        (from-bookmark-list bookmark--annotation-from-bookmark-list)
+        (old-buffer (current-buffer)))
     (bookmark-set-annotation bookmark-name annotation)
     (setq bookmark-alist-modification-count
           (1+ bookmark-alist-modification-count))
-    (bookmark-bmenu-surreptitiously-rebuild-list))
-  (kill-buffer (current-buffer)))
+    (message "Annotation updated for \"%s\"" bookmark-name)
+    (quit-window)
+    (bookmark-bmenu-surreptitiously-rebuild-list)
+    (when from-bookmark-list
+      (pop-to-buffer (get-buffer bookmark-bmenu-buffer))
+      (goto-char (point-min))
+      (text-property-search-forward 'bookmark-name-prop bookmark-name))
+    (kill-buffer old-buffer)))
 
 
-(defun bookmark-edit-annotation (bookmark-name-or-record)
-  "Pop up a buffer for editing bookmark BOOKMARK-NAME-OR-RECORD's annotation."
+(defun bookmark-edit-annotation (bookmark-name-or-record &optional 
from-bookmark-list)
+  "Pop up a buffer for editing bookmark BOOKMARK-NAME-OR-RECORD's annotation.
+If optional argument FROM-BOOKMARK-LIST is non-nil, return to the
+bookmark list when editing is done."
   (pop-to-buffer (generate-new-buffer-name "*Bookmark Annotation Compose*"))
   (bookmark-insert-annotation bookmark-name-or-record)
   (bookmark-edit-annotation-mode)
-  (set (make-local-variable 'bookmark-annotation-name)
-       bookmark-name-or-record))
+  (setq bookmark--annotation-from-bookmark-list from-bookmark-list)
+  (setq bookmark-annotation-name bookmark-name-or-record))
 
 
 (defun bookmark-buffer-name ()
@@ -1555,10 +1569,9 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
        (progress-reporter-done reporter)))))
 
 
-;;; Code supporting the dired-like bookmark menu.
+;;; Code supporting the dired-like bookmark list.
 ;; Prefix is "bookmark-bmenu" for "buffer-menu":
 
-
 (defvar bookmark-bmenu-hidden-bookmarks ())
 
 
@@ -1642,7 +1655,7 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
 (defun bookmark-bmenu-surreptitiously-rebuild-list ()
   "Rebuild the Bookmark List if it exists.
 Don't affect the buffer ring order."
-  (if (get-buffer "*Bookmark List*")
+  (if (get-buffer bookmark-bmenu-buffer)
       (save-excursion
         (save-window-excursion
           (bookmark-bmenu-list)))))
@@ -1656,7 +1669,7 @@ The leftmost column displays a D if the bookmark is 
flagged for
 deletion, or > if it is flagged for displaying."
   (interactive)
   (bookmark-maybe-load-default-file)
-  (let ((buf (get-buffer-create "*Bookmark List*")))
+  (let ((buf (get-buffer-create bookmark-bmenu-buffer)))
     (if (called-interactively-p 'interactive)
         (switch-to-buffer buf)
       (set-buffer buf)))
@@ -2059,7 +2072,7 @@ bookmark menu visible."
   "Edit the annotation for the current bookmark in another window."
   (interactive)
   (let ((bookmark (bookmark-bmenu-bookmark)))
-    (bookmark-edit-annotation bookmark)))
+    (bookmark-edit-annotation bookmark t)))
 
 
 (defun bookmark-bmenu-unmark (&optional backup)
diff --git a/test/lisp/bookmark-tests.el b/test/lisp/bookmark-tests.el
index 6fa0b1e..a774dae 100644
--- a/test/lisp/bookmark-tests.el
+++ b/test/lisp/bookmark-tests.el
@@ -186,6 +186,18 @@ the lexically-bound variable `buffer'."
        ;; 3. bookmark-set-internal
        (should-error (bookmark-set-internal "foo" "bar" t))))))
 
+(ert-deftest bookmark-tests-set/bookmark-use-annotations-t ()
+  (with-bookmark-test-file
+   (let ((bookmark-use-annotations t))
+     (save-window-excursion
+       (switch-to-buffer buffer)
+       ;; Should jump to edit annotation buffer
+       (bookmark-set "foo")
+       (should (equal major-mode 'bookmark-edit-annotation-mode))
+       ;; Should return to the original buffer
+       (bookmark-send-edited-annotation)
+       (should (equal (current-buffer) buffer))))))
+
 (ert-deftest bookmark-tests-kill-line ()
   (with-temp-buffer
     (insert "foobar\n")
@@ -313,7 +325,42 @@ that saves and then loads the bookmark file."
                     (list bookmark-tests-bookmark
                           (cons "name<2>" (cdr bookmark-tests-bookmark))))))))
 
-;; TODO: Add tests for bookmark-bmenu.
+;; TODO: Add more tests for bookmark-bmenu.
+
+(defmacro with-bookmark-bmenu-test (&rest body)
+  "Create environment for testing `bookmark-bmenu-list' and evaluate BODY.
+Same as `with-bookmark-test' but with additions suitable for
+testing `bookmark-bmenu-list'."
+  `(with-bookmark-test
+    (let ((bookmark-bmenu-buffer "*Bookmark List - Testing*"))
+      (unwind-protect
+          (save-window-excursion
+            (bookmark-bmenu-list)
+            ,@body)
+        (kill-buffer bookmark-bmenu-buffer)))))
+
+(ert-deftest bookmark-bmenu.enu-edit-annotation/show-annotation ()
+  (with-bookmark-bmenu-test
+   (bookmark-set-annotation "name" "foo")
+   (bookmark-bmenu-edit-annotation)
+   (should (string-match "foo" (buffer-string)))
+   (kill-buffer (current-buffer))))
+
+(ert-deftest bookmark-bmenu-send-edited-annotation ()
+  (with-bookmark-bmenu-test
+   (bookmark-bmenu-edit-annotation)
+   (insert "foo")
+   (bookmark-send-edited-annotation)
+   (should (equal (bookmark-get-annotation "name") "foo"))))
+
+(ert-deftest bookmark-bmenu-send-edited-annotation/restore-focus ()
+  "Test for https://debbugs.gnu.org/20150 ."
+  (with-bookmark-bmenu-test
+   (bookmark-bmenu-edit-annotation)
+   (insert "foo")
+   (bookmark-send-edited-annotation)
+   (should (equal (buffer-name (current-buffer)) bookmark-bmenu-buffer))
+   (should (looking-at "name"))))
 
 (provide 'bookmark-tests)
 ;;; bookmark-tests.el ends here