* Add new functions to mark/unmark/delete all bookmarks
@ 2020-07-23 23:00 Matthew White
2020-07-24 3:15 ` Drew Adams
` (2 more replies)
0 siblings, 3 replies; 52+ messages in thread
From: Matthew White @ 2020-07-23 23:00 UTC (permalink / raw)
To: emacs-devel
[-- Attachment #1.1: Type: text/plain, Size: 1023 bytes --]
Hello,
Attached here there is a patch to add new functions to mark, unmark,
and delete all bookmarks at once.
Menu entries and bindings are updated too, and tests are included.
You may consider this both a suggestion and a way to get an opinion
from the community.
I felt that some functions where missing, I'm used to type "U" in
dired to unmark all marks...
Summary (just main functions are described here):
* bookmark-delete-all: With ARG nil asks for confirmation. Deletes all
bookmarks. Bound to "D" in `bookmark-map'.
* bookmark-bmenu-mark-all: Operates in the *Bookmark List* buffer.
Marks all bookmarks. Bound to "M" in `bookmark-bmenu-mode-map'.
* bookmark-bmenu-unmark-all: Operates in the *Bookmark List* buffer.
Unmarks all bookmarks. Bound to "U" in `bookmark-bmenu-mode-map'.
* bookmark-bmenu-delete-all: Operates in the *Bookmark List* buffer.
Marks all bookmarks for deletion. Bound to "D" in `bookmark-bmenu-mode-map'.
--
Matthew White <mehw.is.me@inventati.org>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Add-new-functions-to-mark-unmark-delete-all-bookmark.patch --]
[-- Type: text/x-patch, Size: 23739 bytes --]
From 12d3ebba1115085e23fc8eb4968fc8a1f72ffd32 Mon Sep 17 00:00:00 2001
From: Matthew White <mehw.is.me@inventati.org>
Date: Thu, 23 Jul 2020 21:14:32 +0000
Subject: [PATCH] Add new functions to mark/unmark/delete all bookmarks
* lisp/bookmark.el (bookmark-delete-all): New function to delete all
bookmarks.
* lisp/bookmark.el (bookmark-bmenu-mark-all): New function to mark all
bookmarks in the bookmark list buffer.
* lisp/bookmark.el (bookmark-bmenu-unmark-all): New function to unmark
all bookmarks in the bookmark list buffer.
* lisp/bookmark.el (bookmark-bmenu-delete-all): New function to mark
for deletion all bookmarks in the bookmark list buffer.
* lisp/bookmark.el (bookmark-map): Map "D" to `bookmark-delete-all'.
* lisp/bookmark.el (bookmark-bmenu-mode-map): New mappping for "M" to
`bookmark-bmenu-mark-all'.
* lisp/bookmark.el (bookmark-bmenu-mode-map): New mappping for "U" to
`bookmark-bmenu-unmark-all'.
* lisp/bookmark.el (bookmark-bmenu-mode-map): New mappping for "D" to
`bookmark-bmenu-delete-all'.
* lisp/bookmark.el (bookmark-bmenu-mark-all): New bookmark menu to
`bookmark-delete-all'.
* lisp/bookmark.el (easy-menu-define): New bookmark menu to
`bookmark-bmenu-mark-all'.
* lisp/bookmark.el (easy-menu-define): New bookmark menu to
`bookmark-bmenu-unmark-all'.
* lisp/bookmark.el (easy-menu-define): New bookmark menu to
`bookmark-bmenu-delete-all'.
* lisp/bookmark.el (bookmark-bmenu-select): Update docstring to
include a reference to `bookmark-bmenu-mark-all'.
* lisp/bookmark.el (bookmark-bmenu-mode): Update docstring. Add/Update
description: `bookmark-bmenu-mark-all', `bookmark-bmenu-delete-all',
`bookmark-bmenu-execute-deletions', and `bookmark-bmenu-unmark-all'.
* test/lisp/bookmark-resources/test-list.bmk: New bookmark file to
test a list of bookmarks.
* test/lisp/bookmark-tests.el (bookmark-tests-bookmark-file-list): New
reference to the bookmark file used for testing a list of bookmarks.
* test/lisp/bookmark-tests.el (bookmark-tests-bookmark-list-0,
bookmark-tests-bookmark-list-1, bookmark-tests-bookmark-list-2): New
cached values for testing a list of bookmark.
* test/lisp/bookmark-tests.el (bookmark-tests-cache-timestamp-list):
New variable to set `bookmark-bookmarks-timestamp'.
* test/lisp/bookmark-tests.el (with-bookmark-test-list): New macro
environment to test a list of bookmarks.
* test/lisp/bookmark-tests.el (with-bookmark-test-file-list): New
macro environment to test a list of bookmarks with example.txt.
* test/lisp/bookmark-tests.el (with-bookmark-bmenu-test-list): New
macro environment to test functions about a list of bookmarks from
`bookmark-bmenu-list'.
* test/lisp/bookmark-tests.el (bookmark-tests-all-names-list,
bookmark-tests-get-bookmark-list,
bookmark-tests-get-bookmark-record-list): New functions to test the
records of the list of bookmarks.
* test/lisp/bookmark-tests.el (bookmark-tests-make-record-list): New
function to test the creation of a record from example.txt with a
list of bookmarks loaded.
* test/lisp/bookmark-tests.el (bookmark-tests-delete-all): New
function to test `bookmark-delete-all'.
* test/lisp/bookmark-tests.el (bookmark-test-bmenu-mark-all): New
function to test `bookmark-bmenu-mark-all'.
* test/lisp/bookmark-tests.el (bookmark-test-bmenu-unmark-all): New
function to test `bookmark-bmenu-unmark-all'.
* test/lisp/bookmark-tests.el (bookmark-test-bmenu-delete-all): New
function to test `bookmark-bmenu-delete-all'.
---
lisp/bookmark.el | 79 ++++++++-
test/lisp/bookmark-resources/test-list.bmk | 20 +++
test/lisp/bookmark-tests.el | 183 +++++++++++++++++++++
3 files changed, 280 insertions(+), 2 deletions(-)
create mode 100644 test/lisp/bookmark-resources/test-list.bmk
diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index de7d60f97e..8a9c032930 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -200,6 +200,7 @@ A non-nil value may result in truncated bookmark names."
(define-key map "f" 'bookmark-insert-location) ;"f"ind
(define-key map "r" 'bookmark-rename)
(define-key map "d" 'bookmark-delete)
+ (define-key map "D" 'bookmark-delete-all)
(define-key map "l" 'bookmark-load)
(define-key map "w" 'bookmark-write)
(define-key map "s" 'bookmark-save)
@@ -1374,6 +1375,21 @@ probably because we were called from there."
(bookmark-save)))
+;;;###autoload
+(defun bookmark-delete-all (&optional arg)
+ "Delete all bookmarks permanently.
+Doesn't ask for confirmation if ARG is non-nil."
+ (interactive "P")
+ (when (or arg (yes-or-no-p "Delete all bookmarks permanently? "))
+ (bookmark-maybe-load-default-file)
+ (setq bookmark-alist-modification-count
+ (+ bookmark-alist-modification-count (length bookmark-alist)))
+ (setq bookmark-alist nil)
+ (bookmark-bmenu-surreptitiously-rebuild-list)
+ (when (bookmark-time-to-save-p)
+ (bookmark-save))))
+
+
(defun bookmark-time-to-save-p (&optional final-time)
"Return t if it is time to save bookmarks to disk, nil otherwise.
Optional argument FINAL-TIME means this is being called when Emacs
@@ -1600,12 +1616,15 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
(define-key map "\C-d" 'bookmark-bmenu-delete-backwards)
(define-key map "x" 'bookmark-bmenu-execute-deletions)
(define-key map "d" 'bookmark-bmenu-delete)
+ (define-key map "D" 'bookmark-bmenu-delete-all)
(define-key map " " 'next-line)
(define-key map "n" 'next-line)
(define-key map "p" 'previous-line)
(define-key map "\177" 'bookmark-bmenu-backup-unmark)
(define-key map "u" 'bookmark-bmenu-unmark)
+ (define-key map "U" 'bookmark-bmenu-unmark-all)
(define-key map "m" 'bookmark-bmenu-mark)
+ (define-key map "M" 'bookmark-bmenu-mark-all)
(define-key map "l" 'bookmark-bmenu-load)
(define-key map "r" 'bookmark-bmenu-rename)
(define-key map "R" 'bookmark-bmenu-relocate)
@@ -1627,8 +1646,10 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
["Select Marked Bookmarks" bookmark-bmenu-select t]
"---"
["Mark Bookmark" bookmark-bmenu-mark t]
+ ["Mark all Bookmarks" bookmark-bmenu-mark-all t]
["Unmark Bookmark" bookmark-bmenu-unmark t]
["Unmark Backwards" bookmark-bmenu-backup-unmark t]
+ ["Unmark all Bookmarks" bookmark-bmenu-unmark-all t]
["Toggle Display of Filenames" bookmark-bmenu-toggle-filenames t]
["Display Location of Bookmark" bookmark-bmenu-locate t]
"---"
@@ -1636,6 +1657,7 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
["Rename Bookmark" bookmark-bmenu-rename t]
["Relocate Bookmark's File" bookmark-bmenu-relocate t]
["Mark Bookmark for Deletion" bookmark-bmenu-delete t]
+ ["Mark all Bookmarks for Deletion" bookmark-bmenu-delete-all t]
["Delete Marked Bookmarks" bookmark-bmenu-execute-deletions t])
("Annotations"
["Show Annotation for Current Bookmark" bookmark-bmenu-show-annotation t]
@@ -1748,6 +1770,7 @@ Letters do not insert themselves; instead, they are commands.
Bookmark names preceded by a \"*\" have annotations.
\\<bookmark-bmenu-mode-map>
\\[bookmark-bmenu-mark] -- mark bookmark to be displayed.
+\\[bookmark-bmenu-mark-all] -- mark all listed bookmarks to be displayed.
\\[bookmark-bmenu-select] -- select bookmark of line point is on.
Also show bookmarks marked using m in other windows.
\\[bookmark-bmenu-toggle-filenames] -- toggle displaying of filenames (they may obscure long bookmark names).
@@ -1764,13 +1787,15 @@ Bookmark names preceded by a \"*\" have annotations.
\\[bookmark-bmenu-relocate] -- relocate this bookmark's file (prompts for new file).
\\[bookmark-bmenu-delete] -- mark this bookmark to be deleted, and move down.
\\[bookmark-bmenu-delete-backwards] -- mark this bookmark to be deleted, and move up.
-\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]'.
+\\[bookmark-bmenu-delete-all] -- mark all listed bookmarks as to be deleted.
+\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]' or `\\[bookmark-bmenu-delete-all]'.
\\[bookmark-bmenu-save] -- save the current bookmark list in the default file.
With a prefix arg, prompts for a file to save in.
\\[bookmark-bmenu-load] -- load in a file of bookmarks (prompts for file.)
\\[bookmark-bmenu-unmark] -- remove all kinds of marks from current line.
With prefix argument, also move up one line.
\\[bookmark-bmenu-backup-unmark] -- back up a line and remove marks.
+\\[bookmark-bmenu-unmark-all] -- remove all kinds of marks from all listed bookmarks.
\\[bookmark-bmenu-show-annotation] -- show the annotation, if it exists, for the current bookmark
in another buffer.
\\[bookmark-bmenu-show-all-annotations] -- show the annotations of all bookmarks in another buffer.
@@ -1937,9 +1962,24 @@ If the annotation does not exist, do nothing."
(bookmark-bmenu-ensure-position))))
+(defun bookmark-bmenu-mark-all ()
+ "Mark all listed bookmarks to be displayed by \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-select]."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ ;; FIXME: This assumes that the last line is empty.
+ (while (not (eobp))
+ (delete-char 1)
+ (insert ?>)
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-select ()
"Select this line's bookmark; also display bookmarks marked with `>'.
-You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] command."
+You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] or \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark-all] commands."
(interactive)
(let ((bmrk (bookmark-bmenu-bookmark))
(menu (current-buffer))
@@ -2108,6 +2148,21 @@ Optional BACKUP means move up."
(bookmark-bmenu-ensure-position))
+(defun bookmark-bmenu-unmark-all ()
+ "Cancel all requested operations on all listed bookmarks."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ ;; FIXME: This assumes that the last line is empty.
+ (while (not (eobp))
+ (delete-char 1)
+ (insert " ")
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-delete ()
"Mark bookmark on this line to be deleted.
To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
@@ -2133,6 +2188,23 @@ To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\
(bookmark-bmenu-ensure-position))
+(defun bookmark-bmenu-delete-all ()
+ "Mark all listed bookmarks as to be deleted.
+To remove all deletion marks, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-unmark-all].
+To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ ;; FIXME: This assumes that the last line is empty.
+ (while (not (eobp))
+ (delete-char 1)
+ (insert ?D)
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-execute-deletions ()
"Delete bookmarks flagged `D'."
(interactive)
@@ -2292,6 +2364,9 @@ strings returned are not."
(bindings--define-key map [delete]
'(menu-item "Delete Bookmark..." bookmark-delete
:help "Delete a bookmark from the bookmark list"))
+ (bindings--define-key map [delete-all]
+ '(menu-item "Delete all Bookmarks..." bookmark-delete-all
+ :help "Delete all bookmarks from the bookmark list"))
(bindings--define-key map [rename]
'(menu-item "Rename Bookmark..." bookmark-rename
:help "Change the name of a bookmark"))
diff --git a/test/lisp/bookmark-resources/test-list.bmk b/test/lisp/bookmark-resources/test-list.bmk
new file mode 100644
index 0000000000..11a5c3f222
--- /dev/null
+++ b/test/lisp/bookmark-resources/test-list.bmk
@@ -0,0 +1,20 @@
+;;;; Emacs Bookmark Format Version 1 ;;;; -*- coding: utf-8-emacs -*-
+;;; This format is meant to be slightly human-readable;
+;;; nevertheless, you probably don't want to edit it.
+;;; -*- End Of Bookmark File Format Version Stamp -*-
+(("name0"
+ (filename . "/some/file0")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+("name1"
+ (filename . "/some/file1")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+("name2"
+ (filename . "/some/file2")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+)
diff --git a/test/lisp/bookmark-tests.el b/test/lisp/bookmark-tests.el
index b9c6ff9c54..3790498812 100644
--- a/test/lisp/bookmark-tests.el
+++ b/test/lisp/bookmark-tests.el
@@ -83,6 +83,70 @@ the lexically-bound variable `buffer'."
,@body)
(kill-buffer buffer))))
+(defvar bookmark-tests-bookmark-file-list
+ (expand-file-name "test-list.bmk" bookmark-tests-data-dir)
+ "Bookmark file used for testing a list of bookmarks.")
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-0 '("name0"
+ (filename . "/some/file0")
+ (front-context-string . "ghi")
+ (rear-context-string . "jkl")
+ (position . 4))
+ "Cached value used in bookmark-tests.el."))
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-1 '("name1"
+ (filename . "/some/file1")
+ (front-context-string . "mno")
+ (rear-context-string . "pqr")
+ (position . 5))
+ "Cached value used in bookmark-tests.el."))
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-2 '("name2"
+ (filename . "/some/file2")
+ (front-context-string . "stu")
+ (rear-context-string . "vwx")
+ (position . 6))
+ "Cached value used in bookmark-tests.el."))
+
+(defvar bookmark-tests-cache-timestamp-list
+ (cons bookmark-tests-bookmark-file-list
+ (nth 5 (file-attributes
+ bookmark-tests-bookmark-file-list)))
+ "Cached value used in bookmark-tests.el.")
+
+(defmacro with-bookmark-test-list (&rest body)
+ "Create environment for testing bookmark.el and evaluate BODY.
+Ensure a clean environment for testing, and do not change user
+data when running tests interactively."
+ `(with-temp-buffer
+ (let ((bookmark-alist (quote (,(copy-sequence bookmark-tests-bookmark-list-0)
+ ,(copy-sequence bookmark-tests-bookmark-list-1)
+ ,(copy-sequence bookmark-tests-bookmark-list-2))))
+ (bookmark-default-file bookmark-tests-bookmark-file-list)
+ (bookmark-bookmarks-timestamp bookmark-tests-cache-timestamp-list)
+ bookmark-save-flag)
+ ,@body)))
+
+(defmacro with-bookmark-test-file-list (&rest body)
+ "Create environment for testing bookmark.el and evaluate BODY.
+Same as `with-bookmark-test-list' but also opens the resource file
+example.txt in a buffer, which can be accessed by callers through
+the lexically-bound variable `buffer'."
+ `(let ((buffer (find-file-noselect bookmark-tests-example-file)))
+ (unwind-protect
+ (with-bookmark-test-list
+ ,@body)
+ (kill-buffer buffer))))
+
(ert-deftest bookmark-tests-all-names ()
(with-bookmark-test
(should (equal (bookmark-all-names) '("name")))))
@@ -95,6 +159,22 @@ the lexically-bound variable `buffer'."
(with-bookmark-test
(should (equal (bookmark-get-bookmark-record "name") (cdr bookmark-tests-bookmark)))))
+(ert-deftest bookmark-tests-all-names-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-all-names) '("name0" "name1" "name2")))))
+
+(ert-deftest bookmark-tests-get-bookmark-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-get-bookmark "name0") bookmark-tests-bookmark-list-0))
+ (should (equal (bookmark-get-bookmark "name1") bookmark-tests-bookmark-list-1))
+ (should (equal (bookmark-get-bookmark "name2") bookmark-tests-bookmark-list-2))))
+
+(ert-deftest bookmark-tests-get-bookmark-record-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-get-bookmark-record "name0") (cdr bookmark-tests-bookmark-list-0)))
+ (should (equal (bookmark-get-bookmark-record "name1") (cdr bookmark-tests-bookmark-list-1)))
+ (should (equal (bookmark-get-bookmark-record "name2") (cdr bookmark-tests-bookmark-list-2)))))
+
(ert-deftest bookmark-tests-record-getters-and-setters-new ()
(with-temp-buffer
(let* ((buffer-file-name "test")
@@ -130,6 +210,19 @@ the lexically-bound variable `buffer'."
;; calling twice gives same record
(should (equal (bookmark-make-record) record))))))
+(ert-deftest bookmark-tests-make-record-list ()
+ (with-bookmark-test-file-list
+ (let* ((record `("example.txt" (filename . ,bookmark-tests-example-file)
+ (front-context-string . "is text file is ")
+ (rear-context-string)
+ (position . 3)
+ (defaults "example.txt"))))
+ (with-current-buffer buffer
+ (goto-char 3)
+ (should (equal (bookmark-make-record) record))
+ ;; calling twice gives same record
+ (should (equal (bookmark-make-record) record))))))
+
(ert-deftest bookmark-tests-make-record-function ()
(with-bookmark-test
(let ((buffer-file-name "test"))
@@ -267,6 +360,11 @@ the lexically-bound variable `buffer'."
(bookmark-delete "name")
(should (equal bookmark-alist nil))))
+(ert-deftest bookmark-tests-delete-all ()
+ (with-bookmark-test-list
+ (bookmark-delete-all t)
+ (should (equal bookmark-alist nil))))
+
(defmacro with-bookmark-test-save-load (&rest body)
"Create environment for testing bookmark.el and evaluate BODY.
Same as `with-bookmark-test' but also sets a temporary
@@ -340,6 +438,18 @@ testing `bookmark-bmenu-list'."
,@body)
(kill-buffer bookmark-bmenu-buffer)))))
+(defmacro with-bookmark-bmenu-test-list (&rest body)
+ "Create environment for testing `bookmark-bmenu-list' and evaluate BODY.
+Same as `with-bookmark-test-list' but with additions suitable for
+testing `bookmark-bmenu-list'."
+ `(with-bookmark-test-list
+ (let ((bookmark-bmenu-buffer "*Bookmark List - Testing*"))
+ (unwind-protect
+ (save-window-excursion
+ (bookmark-bmenu-list)
+ ,@body)
+ (kill-buffer bookmark-bmenu-buffer)))))
+
(ert-deftest bookmark-test-bmenu-edit-annotation/show-annotation ()
(with-bookmark-bmenu-test
(bookmark-set-annotation "name" "foo")
@@ -396,6 +506,28 @@ testing `bookmark-bmenu-list'."
(beginning-of-line)
(should (looking-at "^>"))))
+(ert-deftest bookmark-test-bmenu-mark-all ()
+ (with-bookmark-bmenu-test-list
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-mark-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are marked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark)))))))
+
(ert-deftest bookmark-test-bmenu-any-marks ()
(with-bookmark-bmenu-test
(bookmark-bmenu-mark)
@@ -410,12 +542,63 @@ testing `bookmark-bmenu-list'."
(beginning-of-line)
(should (looking-at "^ "))))
+(ert-deftest bookmark-test-bmenu-unmark-all ()
+ (with-bookmark-bmenu-test-list
+ (bookmark-bmenu-mark-all)
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-unmark-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are unmarked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark)))))))
+
(ert-deftest bookmark-test-bmenu-delete ()
(with-bookmark-bmenu-test
(bookmark-bmenu-delete)
(bookmark-bmenu-execute-deletions)
(should (equal (length bookmark-alist) 0))))
+(ert-deftest bookmark-test-bmenu-delete-all ()
+ (with-bookmark-bmenu-test-list
+ ;; Verify that unmarked bookmarks aren't deleted
+ (bookmark-bmenu-execute-deletions)
+ (should-not (eq bookmark-alist nil))
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-delete-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are marked for deletion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ ;; Verify that all bookmarks are deleted
+ (bookmark-bmenu-execute-deletions)
+ (should (eq bookmark-alist nil)))))
+
(ert-deftest bookmark-test-bmenu-locate ()
(let (msg)
(cl-letf (((symbol-function 'message)
--
2.26.2
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 52+ messages in thread
* RE: Add new functions to mark/unmark/delete all bookmarks
2020-07-23 23:00 Add new functions to mark/unmark/delete all bookmarks Matthew White
@ 2020-07-24 3:15 ` Drew Adams
2020-07-24 9:07 ` Matthew White
2020-07-24 5:31 ` Yuri Khan
2020-07-24 19:07 ` Karl Fogel
2 siblings, 1 reply; 52+ messages in thread
From: Drew Adams @ 2020-07-24 3:15 UTC (permalink / raw)
To: Matthew White, emacs-devel
> Attached here there is a patch to add new functions to mark, unmark,
> and delete all bookmarks at once.
>
> Menu entries and bindings are updated too, and tests are included.
>
> You may consider this both a suggestion and a way to get an opinion
> from the community.
>
> I felt that some functions where missing, I'm used to type "U" in
> dired to unmark all marks...
>
> Summary (just main functions are described here):
>
> * bookmark-delete-all: With ARG nil asks for confirmation. Deletes all
> bookmarks. Bound to "D" in `bookmark-map'.
> * bookmark-bmenu-mark-all: Operates in the *Bookmark List* buffer.
> Marks all bookmarks. Bound to "M" in `bookmark-bmenu-mode-map'.
> * bookmark-bmenu-unmark-all: Operates in the *Bookmark List* buffer.
> Unmarks all bookmarks. Bound to "U" in `bookmark-bmenu-mode-map'.
> * bookmark-bmenu-delete-all: Operates in the *Bookmark List* buffer.
> Marks all bookmarks for deletion. Bound to "D" in `bookmark-bmenu-mode-
> map'.
Well, I naturally am not in favor of seeing this.
All of that is in Bookmark+, but better, IMO.
You could have taken some of what's there, either
as is or as a starting point.
The Bookmark+ behavior borrows heavily from Dired
(and not just for marking). `U', like `M-DEL',
unmarks all of a given kind of mark, or all marks.
There are commands to mark different kinds of
bookmarks and commands to mark bookmarks that
satisfy various conditions.
`M-m' marks all bookmarks. (`M' is a prefix key
for marking or showing only `man' bookmarks. Most
uppercase letters are prefix keys for acting on
particular kinds of bookmarks. `M' as a suffix
key marks bookmarks of the type indicated by the
prefix key; e.g., `I M' marks Info bookmarks.)
Marking flexibility carries over for deletion.
As in Dired, `D' deletes the marked bookmarks -
it doesn't flag for deletion. `d' and `C-d' flag
individually. As in Dired, there's no need for a
command to flag all, IMO.
https://www.emacswiki.org/emacs/BookmarkPlus#MarkingAndUnmarkingBookmarks
___
Some of the commands involving mark (`>') and flag
(`D') that are bound to keys:
d - Flag bookmark `D' for deletion, then move down
C-d - Flag bookmark `D' for deletion, then move up
x - Delete (visible) bookmarks flagged `D'
D - Delete (visible) bookmarks marked `>'
m - Mark bookmark
u - Unmark bookmark (`C-u': move up one line)
DEL - Unmark previous bookmark (move up, then unmark)
M-m - Mark all bookmarks
% m - Mark all bookmarks whose names match a regexp
U - Unmark all bookmarks (`C-u': interactive query)
t - Toggle marks: unmark the marked and mark the unmarked
A M - Mark autofile bookmarks
# M - Mark autonamed bookmarks
X M - Mark temporary bookmarks
H M - Mark highlighted bookmarks
B M - Mark non-file (i.e. buffer) bookmarks
M-d M-m - Mark Dired bookmarks
F M - Mark file & directory bookmarks (`C-u': local only)
G M - Mark Gnus bookmarks
I M - Mark Info bookmarks
i M - Mark Icicles search-hits bookmarks
N M - Mark non-invokable bookmarks
M-I M-M - Mark image-file bookmarks
K M - Mark desktop bookmarks
M M - Mark `man' page bookmarks (that's `M' twice, not Meta-M)
O M - Mark orphaned local file/dir bookmarks (`C-u': remote also)
R M - Mark region bookmarks
Q M - Mark function bookmarks
M-u M-m - Mark URL bookmarks
V M - Mark variable-list bookmarks
W 3 M - Mark W3M (URL) bookmarks
w M - Mark snippet bookmarks
Y M - Mark bookmark-file bookmarks
Z M - Mark bookmark-list bookmarks
T m % - Mark bookmarks having at least one tag that matches a regexp
T m + - Mark bookmarks having at least one tag in a set (OR)
T m * - Mark bookmarks having all of the tags in a set (AND)
T m ~ + - Mark bookmarks not having any of the tags in a set (NOT OR)
T m ~ * - Mark bookmarks not having all of the tags in a set (NOT AND)
T u % - Unmark bookmarks having a tag that matches a regexp
T u + - Unmark bookmarks having at least one tag in a set (OR)
T u * - Unmark bookmarks having all of the tags in a set (AND)
T u ~ + - Unmark bookmarks not having any tags in a set (NOT OR)
T u ~ * - Unmark bookmarks not having all tags in a set (NOT AND)
> - Toggle showing only marked bookmarks
< - Toggle showing only unmarked bookmarks
- > - Omit the marked bookmarks; un-omit them if after `- S'
j > - Jump to marked bookmarks in other windows
C-h > - Show info about marked bookmarks (`C-u': internal form)
Y > - - Move the marked bookmarks to a bookmark file
Y > + - Copy the marked bookmarks to a bookmark file
Y > 0 - Copy the marked bookmarks to a new bookmark file
M-l - Load marked bookmark-file bookmarks (extra load)
M-R - Relocate the marked bookmarks
a > - Edit annotations of marked bookmarks (`C-u': all)
E - Edit internal Lisp records of marked bookmarks (`C-u': all)
M-X - Toggle temporary/savable status of marked bookmarks
M-d > - Open Dired for marked file & directory bookmarks
M-s a C-s - Isearch the marked bookmarks (`C-u': all)
M-s a C-M-s - Regexp-isearch the marked bookmarks (`C-u': all)
M-s a M-s - Regexp-search the marked file bookmarks (`C-u': all)
M-q - Query-replace the marked file bookmarks
T > l - List tags used in marked bookmarks (`C-u': show tag values)
T > v - Set value of a tag, for each marked bookmark (`C-u': all)
T > C-y - Add tags copied from a bookmark to those marked (`C-u': all)
T > q - Replace tags of marked with copied tags (`C-u': all)
H > + - Set highlighting for marked bookmarks
H > H - Highlight the marked bookmarks
H > U - Unhighlight the marked bookmarks
s > - Sort marked (`>') bookmarks first
s D - Sort flagged (`D') bookmarks first
Here's the `Mark' submenu of the `Bookmark+' menu-bar
menu, followed by its `Bookmarks of Type' submenu for
marking bookmarks of a specific kind:
https://www.emacswiki.org/emacs/BookmarkPlus#MarkMenu
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-23 23:00 Add new functions to mark/unmark/delete all bookmarks Matthew White
2020-07-24 3:15 ` Drew Adams
@ 2020-07-24 5:31 ` Yuri Khan
2020-07-24 15:05 ` Stefan Monnier
2020-07-24 17:07 ` Add new functions to mark/unmark/delete all bookmarks Drew Adams
2020-07-24 19:07 ` Karl Fogel
2 siblings, 2 replies; 52+ messages in thread
From: Yuri Khan @ 2020-07-24 5:31 UTC (permalink / raw)
To: Matthew White; +Cc: Emacs developers
On Fri, 24 Jul 2020 at 06:49, Matthew White <mehw.is.me@inventati.org> wrote:
> Attached here there is a patch to add new functions to mark, unmark,
> and delete all bookmarks at once.
>
> I felt that some functions were missing, I'm used to typing "U" in
> dired to unmark all marks...
I wonder (1) why bookmark-menu is not derived from
tabulated-list-mode, and (2) if maybe tabulated-list-mode would
benefit from a generic implementation of multiple marking, unmarking,
marking-for-deletion, and command application.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-24 3:15 ` Drew Adams
@ 2020-07-24 9:07 ` Matthew White
2020-07-24 15:07 ` Stefan Monnier
2020-07-24 16:43 ` Drew Adams
0 siblings, 2 replies; 52+ messages in thread
From: Matthew White @ 2020-07-24 9:07 UTC (permalink / raw)
To: Drew Adams, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 626 bytes --]
> Well, I naturally am not in favor of seeing this.
>
> All of that is in Bookmark+, but better, IMO.
> You could have taken some of what's there, either
> as is or as a starting point.
Hi Drew Adams,
Thanks for your great work.
> https://www.emacswiki.org/emacs/BookmarkPlus#MarkingAndUnmarkingBookmarks
I assure you that I never looked into the code of Bookmark+, hence
there is no code I borrowed in writing my patch to add some common
features to bookmark.el. I'm sorry if people think things that way.
What about the repository of Bookmark+ on GitHub?
https://github.com/emacsmirror/bookmark-plus
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-24 5:31 ` Yuri Khan
@ 2020-07-24 15:05 ` Stefan Monnier
2020-07-24 16:56 ` Noam Postavsky
2020-07-24 19:00 ` Extend tabulated-list-mode to support marks Yuri Khan
2020-07-24 17:07 ` Add new functions to mark/unmark/delete all bookmarks Drew Adams
1 sibling, 2 replies; 52+ messages in thread
From: Stefan Monnier @ 2020-07-24 15:05 UTC (permalink / raw)
To: Yuri Khan; +Cc: Emacs developers, Matthew White
> I wonder (1) why bookmark-menu is not derived from
> tabulated-list-mode,
AFAIK `tabulated-list-mode` was born much later than `bookmark-menu`.
I think we'd welcome a patch to make it use `tabulated-list-mode`.
> and (2) if maybe tabulated-list-mode would benefit from a generic
> implementation of multiple marking, unmarking, marking-for-deletion,
> and command application.
Maybe "for deletion" is too application-specific (some uses of
`tabulated-list-mode` can't "delete" or at least not conveniently enough
that it's a common use case), but a definite +1 for the rest.
Stefan
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-24 9:07 ` Matthew White
@ 2020-07-24 15:07 ` Stefan Monnier
2020-07-24 15:18 ` Lars Ingebrigtsen
2020-07-24 17:07 ` Drew Adams
2020-07-24 16:43 ` Drew Adams
1 sibling, 2 replies; 52+ messages in thread
From: Stefan Monnier @ 2020-07-24 15:07 UTC (permalink / raw)
To: Matthew White; +Cc: Drew Adams, emacs-devel
> I assure you that I never looked into the code of Bookmark+, hence
> there is no code I borrowed in writing my patch to add some common
> features to bookmark.el. I'm sorry if people think things that way.
His code is covered by the GPL *and* he signed the copyright paperwork,
so I don't think his issue is the reverse: he'd rather that the "new
code" be taken from his to reduce the difference between bookmark.el and
bookmark+.el.
Stefan
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-24 15:07 ` Stefan Monnier
@ 2020-07-24 15:18 ` Lars Ingebrigtsen
2020-07-24 17:03 ` Matthew White
2020-07-24 17:07 ` Drew Adams
2020-07-24 17:07 ` Drew Adams
1 sibling, 2 replies; 52+ messages in thread
From: Lars Ingebrigtsen @ 2020-07-24 15:18 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel, Drew Adams, Matthew White
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> I assure you that I never looked into the code of Bookmark+, hence
>> there is no code I borrowed in writing my patch to add some common
>> features to bookmark.el. I'm sorry if people think things that way.
>
> His code is covered by the GPL *and* he signed the copyright paperwork,
> so I don't think his issue is the reverse: he'd rather that the "new
> code" be taken from his to reduce the difference between bookmark.el and
> bookmark+.el.
And I don't think that's a reasonable expectation for Drew to have --
there's no requirement for people working on bookmark.el to look at the
bookmark+.el code.
Matthew's code looks good to me, and seems like a real improvement to
bookmark.el, but I looked at the copyright assignment papers, and
apparently Matthew's only assigned copyright for wget, and not for
Emacs? Or am I misreading the file?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 52+ messages in thread
* RE: Add new functions to mark/unmark/delete all bookmarks
2020-07-24 9:07 ` Matthew White
2020-07-24 15:07 ` Stefan Monnier
@ 2020-07-24 16:43 ` Drew Adams
1 sibling, 0 replies; 52+ messages in thread
From: Drew Adams @ 2020-07-24 16:43 UTC (permalink / raw)
To: Matthew White, emacs-devel
> I assure you that I never looked into the code of Bookmark+, hence
> there is no code I borrowed in writing my patch to add some common
> features to bookmark.el. I'm sorry if people think things that way.
I didn't mean to suggest that you did.
I'm saying that you can, if you like. ;-)
That's all.
> What about the repository of Bookmark+ on GitHub?
> https://github.com/emacsmirror/bookmark-plus
What about it?
Someone else mirrors the files from Emacs Wiki there,
I believe. They look relatively up-to-date, judging
only by the indicated dates. I can't vouch for them,
otherwise, but I trust the guys who apparently mirrored
them.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-24 15:05 ` Stefan Monnier
@ 2020-07-24 16:56 ` Noam Postavsky
2020-07-24 19:00 ` Extend tabulated-list-mode to support marks Yuri Khan
1 sibling, 0 replies; 52+ messages in thread
From: Noam Postavsky @ 2020-07-24 16:56 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Matthew White, Emacs developers, Yuri Khan
On Fri, 24 Jul 2020 at 11:06, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> > I wonder (1) why bookmark-menu is not derived from
> > tabulated-list-mode,
>
> AFAIK `tabulated-list-mode` was born much later than `bookmark-menu`.
> I think we'd welcome a patch to make it use `tabulated-list-mode`.
See Bug#39293 - "Base bookmark-bmenu-mode on 'tabulated-list-mode'"
https://debbugs.gnu.org/39293
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-24 15:18 ` Lars Ingebrigtsen
@ 2020-07-24 17:03 ` Matthew White
2020-07-24 17:09 ` Stefan Monnier
2020-07-24 17:07 ` Drew Adams
1 sibling, 1 reply; 52+ messages in thread
From: Matthew White @ 2020-07-24 17:03 UTC (permalink / raw)
To: Lars Ingebrigtsen, emacs-devel
> Matthew's code looks good to me, and seems like a real improvement to
> bookmark.el, but I looked at the copyright assignment papers, and
> apparently Matthew's only assigned copyright for wget, and not for
> Emacs? Or am I misreading the file?
Hi Lars Ingebrigtsen,
I still have to assign, this is the first patch I submit which requires
an assignment...
Can you, or some dev, send me the necessary instruction and assignment
form, please?
Cheers!
^ permalink raw reply [flat|nested] 52+ messages in thread
* RE: Add new functions to mark/unmark/delete all bookmarks
2020-07-24 5:31 ` Yuri Khan
2020-07-24 15:05 ` Stefan Monnier
@ 2020-07-24 17:07 ` Drew Adams
1 sibling, 0 replies; 52+ messages in thread
From: Drew Adams @ 2020-07-24 17:07 UTC (permalink / raw)
To: Yuri Khan, Matthew White; +Cc: Emacs developers
> I wonder (1) why bookmark-menu is not derived from
> tabulated-list-mode, and (2) if maybe tabulated-list-mode would
> benefit from a generic implementation of multiple marking, unmarking,
> marking-for-deletion, and command application.
See bug #39293:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=39293
Tabulated-list mode is quite rigid and rudimentary.
The kinds of marking, not to mention more complex
features, that Dired provides are not available
(and may not be feasible currently) with t-l mode.
If someone is eager to propagate t-l mode, start
with something like Ibuffer.
(Just one opinion.)
^ permalink raw reply [flat|nested] 52+ messages in thread
* RE: Add new functions to mark/unmark/delete all bookmarks
2020-07-24 15:07 ` Stefan Monnier
2020-07-24 15:18 ` Lars Ingebrigtsen
@ 2020-07-24 17:07 ` Drew Adams
1 sibling, 0 replies; 52+ messages in thread
From: Drew Adams @ 2020-07-24 17:07 UTC (permalink / raw)
To: Stefan Monnier, Matthew White; +Cc: emacs-devel
> > I assure you that I never looked into the code of Bookmark+, hence
> > there is no code I borrowed in writing my patch to add some common
> > features to bookmark.el. I'm sorry if people think things that way.
>
> His code is covered by the GPL *and* he signed the copyright paperwork,
> so I don't think his issue is the reverse: he'd rather that the "new
> code" be taken from his to reduce the difference between bookmark.el and
> bookmark+.el.
Yes.
To the extent that some of the stuff in Bookmark+
were to be integrated into vanilla bookmark.el, I
wouldn't need to include it anymore in Bookmark+
(except for older Emacs releases). IOW, a plus
for me, in terms of Bookmark+ maintenance.
A more important reason for others is that (IMHO)
the Bookmark+ versions of overlapping features are
generally better than the vanilla versions.
And there are a lot more features, many of which
could be added to vanilla bookmark.el with little
trouble, I expect.
I'm not volunteering to help with any such
integration (I haven't the time, energy, or will),
but I'd be happy to help with understanding the
Bookmark+ code if someone did try to integrate
some of the functionality.
^ permalink raw reply [flat|nested] 52+ messages in thread
* RE: Add new functions to mark/unmark/delete all bookmarks
2020-07-24 15:18 ` Lars Ingebrigtsen
2020-07-24 17:03 ` Matthew White
@ 2020-07-24 17:07 ` Drew Adams
1 sibling, 0 replies; 52+ messages in thread
From: Drew Adams @ 2020-07-24 17:07 UTC (permalink / raw)
To: Lars Ingebrigtsen, Stefan Monnier; +Cc: Matthew White, emacs-devel
> > he'd rather that the "new code" be taken from
> > his to reduce the difference between bookmark.el
> > and bookmark+.el.
>
> And I don't think that's a reasonable expectation
> for Drew to have -- there's no requirement for
> people working on bookmark.el to look at the
> bookmark+.el code.
I have no such expectation, believe me.
I only said that Emacs is welcome to take stuff
from Bookmark+, "either as is or as a starting
point."
I do think that doing so would benefit vanilla
Emacs (as well as potentially help me in terms
of perhaps reducing some Bookmark+ maintenance).
But I don't expect that to happen - not at all.
The features of Bookmark+ - the ideas - are
useful, I think. The implementation is also
reasonable, but the features are the main
thing it has to offer. They might at least
serve as food for thought.
IMO, bookmarks are a pretty general tool. They
can be used for all kinds of things.
The name "bookmark" can be somewhat misleading,
as Emacs bookmarks are just another way to hook
in arbitrary bits of code - like hooks, timers,
etc. That they are often associated with target
"places" is a plus, but not a requirement.
And besides hooking in code they hook in data:
timestamps, number of uses/visits, annotations,
anything at all...
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-24 17:03 ` Matthew White
@ 2020-07-24 17:09 ` Stefan Monnier
2020-07-28 15:54 ` Matthew White
0 siblings, 1 reply; 52+ messages in thread
From: Stefan Monnier @ 2020-07-24 17:09 UTC (permalink / raw)
To: Matthew White; +Cc: Lars Ingebrigtsen, emacs-devel
> Can you, or some dev, send me the necessary instruction and assignment
> form, please?
Please fill the form below and then send it as instructed to the FSF so
they can send you the relevant paperwork to sign,
Stefan
Please email the following information to assign@gnu.org, and we
will send you the assignment form for your past and future changes.
Please use your full legal name (in ASCII characters) as the subject
line of the message.
----------------------------------------------------------------------
REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES
[What is the name of the program or package you're contributing to?]
Emacs
[Did you copy any files or text written by someone else in these changes?
Even if that material is free software, we need to know about it.]
[Do you have an employer who might have a basis to claim to own
your changes? Do you attend a school which might make such a claim?]
[For the copyright registration, what country are you a citizen of?]
[What year were you born?]
[Please write your email address here.]
[Please write your postal address here.]
[Which files have you changed so far, and which new files have you written
so far?]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Extend tabulated-list-mode to support marks
2020-07-24 15:05 ` Stefan Monnier
2020-07-24 16:56 ` Noam Postavsky
@ 2020-07-24 19:00 ` Yuri Khan
2020-07-25 13:46 ` Dirk-Jan C. Binnema
` (2 more replies)
1 sibling, 3 replies; 52+ messages in thread
From: Yuri Khan @ 2020-07-24 19:00 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Emacs developers, Matthew White
On Fri, 24 Jul 2020 at 22:05, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> > and (2) if maybe tabulated-list-mode would benefit from a generic
> > implementation of multiple marking, unmarking, marking-for-deletion,
> > and command application.
>
> Maybe "for deletion" is too application-specific (some uses of
> `tabulated-list-mode` can't "delete" or at least not conveniently enough
> that it's a common use case), but a definite +1 for the rest.
My point is that there are many instances of a generic scenario:
Display a list of items; enable the user to perform operations on a
single item at point or to mark multiple items and operate on them as
a subset. Currently, each mode derived from ‘tabulated-list-mode’ has
to reimplement marking, unmarking, mark preservation over refresh,
iterating over marked items, and fallback to the item at point when no
marks are set. One high-visible derived mode hijacks the “unmark all”
key for “mark upgradable packages”. I think they would benefit from a
common implementation and the unification of the UI stemming from
that.
One possibility would be for a derived mode to set some buffer-local
variables to inform ‘tabulated-list-mode’ which marks it supports.
E.g.:
* Set ‘tabulated-list-delete-function’ to a non-nil function that will
delete the item at point or signal an error. Then the command bound to
‘D’ will invoke that function and remove its visual representation
from the buffer; ‘d’ will mark the item at point for deletion; ‘x’
will go through all the items marked for deletion and actually delete
and remove them; ‘u’ will unmark the item at point; and ‘U’ will
unmark all items.
* Set ‘tabulated-list-marks’ to a string of characters. If that string
contains ‘>’, ‘m’ will mark the item at point with ‘>’. Other marks
listed in the string can be set by the derived mode binding a key to a
function that will call (tabulated-list-set-mark CHAR). ‘u’ and ‘U’
will unmark the item at point or all items, respectively. A helper
function, (tabulated-list-map CHAR FUNC), will execute FUNC on each
item marked CHAR or on the item at point if no items are marked,
optionally removing the item or unmarking it depending on FUNC’s
return value; this can be invoked in any commands bound in the derived
mode keymap.
I might implement something if this general direction is deemed
desirable. Details can be discussed.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-23 23:00 Add new functions to mark/unmark/delete all bookmarks Matthew White
2020-07-24 3:15 ` Drew Adams
2020-07-24 5:31 ` Yuri Khan
@ 2020-07-24 19:07 ` Karl Fogel
2020-07-25 10:46 ` Matthew White
2 siblings, 1 reply; 52+ messages in thread
From: Karl Fogel @ 2020-07-24 19:07 UTC (permalink / raw)
To: Matthew White; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 29847 bytes --]
On 24 Jul 2020, Matthew White wrote:
>Attached here there is a patch to add new functions to mark, unmark,
>and delete all bookmarks at once.
>
>Menu entries and bindings are updated too, and tests are included.
>
>You may consider this both a suggestion and a way to get an opinion
>from the community.
>
>I felt that some functions where missing, I'm used to type "U" in
>dired to unmark all marks...
>
>Summary (just main functions are described here):
>
>* bookmark-delete-all: With ARG nil asks for confirmation. Deletes all
> bookmarks. Bound to "D" in `bookmark-map'.
>* bookmark-bmenu-mark-all: Operates in the *Bookmark List* buffer.
> Marks all bookmarks. Bound to "M" in `bookmark-bmenu-mode-map'.
>* bookmark-bmenu-unmark-all: Operates in the *Bookmark List* buffer.
> Unmarks all bookmarks. Bound to "U" in `bookmark-bmenu-mode-map'.
>* bookmark-bmenu-delete-all: Operates in the *Bookmark List* buffer.
> Marks all bookmarks for deletion. Bound to "D" in `bookmark-bmenu-mode-map'.
Hi, Matthew. Thanks so much for this. I have reviewed the patch against 'master' (since it applies cleanly there, whereas it does not quite apply cleanly to 'emacs-27'). It is very good; I have just some minor comments below.
First, a couple of presentation issues:
1) It helps to include a summary line at the top of your commit message, followed by a blank line. The summary line should be limited to 50 characters or fewer, if possible. (This is all documented in the "Commit messages" section in the 'CONTRIBUTE' file in the top level of the Emacs source tree, by the way). In your case, the Subject header of your email almost does the job -- we can shorten it a bit to fit within 50 characters:
"Add ability to mark/unmark/delete all bookmarks"
2) In the commit message, you don't need to repeat the filename ("* lisp/bookmark.el") each time. You can just write it once, and then underneath it show everything affected in that file. Taking the first three entries in your original log message as an example:
* lisp/bookmark.el (bookmark-delete-all): New function to delete all
bookmarks.
* lisp/bookmark.el (bookmark-bmenu-mark-all): New function to mark all
bookmarks in the bookmark list buffer.
* lisp/bookmark.el (bookmark-bmenu-unmark-all): New function to unmark
all bookmarks in the bookmark list buffer.
...they would instead be written like this:
* lisp/bookmark.el (bookmark-delete-all): New function to delete all
bookmarks.
(bookmark-bmenu-mark-all): New function to mark all bookmarks in the
bookmark list buffer.
(bookmark-bmenu-unmark-all): New function to unmark all bookmarks in
the bookmark list buffer.
Quick digression:
You can use 'git log' to see this style used in past Emacs commits. However, note that there a *few* commits that do it the way you did it, and one of them happens to be quite recent, 0d7d422b767c. So if you did look in the logs, and you happened to see that one, then it would be understandable for you to have taken it as an example :-). Nevertheless, it is an exception and is not the normal style (also, even within 0d7d422b767c's commit message the style is inconsistent, if you look closely). See the 'CONTRIBUTE' file for more details on this -- it in turn points to https://www.gnu.org/prep/standards/html_node/Change-Logs.html, which, I now discover, *also* does not give a clear example of the "multiple changes inside one file" scenario we're talking about here even though it is a very common case.
Sigh :-).
Anyway, look through the Emacs logs and you'll see that the way I demonstrate above is the common way.
Other than those two issues, your commit message is very clear. I was able to understand the intent and manner of the change from reading the commit message.
Below I'll quote the entire log message and patch, so people can know that they're seeing everything in this response, even though I only make a few inline comments.
>* lisp/bookmark.el (bookmark-delete-all): New function to delete all
> bookmarks.
>* lisp/bookmark.el (bookmark-bmenu-mark-all): New function to mark all
> bookmarks in the bookmark list buffer.
>* lisp/bookmark.el (bookmark-bmenu-unmark-all): New function to unmark
> all bookmarks in the bookmark list buffer.
>* lisp/bookmark.el (bookmark-bmenu-delete-all): New function to mark
> for deletion all bookmarks in the bookmark list buffer.
>* lisp/bookmark.el (bookmark-map): Map "D" to `bookmark-delete-all'.
>* lisp/bookmark.el (bookmark-bmenu-mode-map): New mappping for "M" to
> `bookmark-bmenu-mark-all'.
>* lisp/bookmark.el (bookmark-bmenu-mode-map): New mappping for "U" to
> `bookmark-bmenu-unmark-all'.
>* lisp/bookmark.el (bookmark-bmenu-mode-map): New mappping for "D" to
> `bookmark-bmenu-delete-all'.
>* lisp/bookmark.el (bookmark-bmenu-mark-all): New bookmark menu to
> `bookmark-delete-all'.
>* lisp/bookmark.el (easy-menu-define): New bookmark menu to
> `bookmark-bmenu-mark-all'.
>* lisp/bookmark.el (easy-menu-define): New bookmark menu to
> `bookmark-bmenu-unmark-all'.
>* lisp/bookmark.el (easy-menu-define): New bookmark menu to
> `bookmark-bmenu-delete-all'.
>* lisp/bookmark.el (bookmark-bmenu-select): Update docstring to
> include a reference to `bookmark-bmenu-mark-all'.
>* lisp/bookmark.el (bookmark-bmenu-mode): Update docstring. Add/Update
> description: `bookmark-bmenu-mark-all', `bookmark-bmenu-delete-all',
> `bookmark-bmenu-execute-deletions', and `bookmark-bmenu-unmark-all'.
>* test/lisp/bookmark-resources/test-list.bmk: New bookmark file to
> test a list of bookmarks.
>* test/lisp/bookmark-tests.el (bookmark-tests-bookmark-file-list): New
> reference to the bookmark file used for testing a list of bookmarks.
>* test/lisp/bookmark-tests.el (bookmark-tests-bookmark-list-0,
> bookmark-tests-bookmark-list-1, bookmark-tests-bookmark-list-2): New
> cached values for testing a list of bookmark.
>* test/lisp/bookmark-tests.el (bookmark-tests-cache-timestamp-list):
> New variable to set `bookmark-bookmarks-timestamp'.
>* test/lisp/bookmark-tests.el (with-bookmark-test-list): New macro
> environment to test a list of bookmarks.
>* test/lisp/bookmark-tests.el (with-bookmark-test-file-list): New
> macro environment to test a list of bookmarks with example.txt.
>* test/lisp/bookmark-tests.el (with-bookmark-bmenu-test-list): New
> macro environment to test functions about a list of bookmarks from
> `bookmark-bmenu-list'.
>* test/lisp/bookmark-tests.el (bookmark-tests-all-names-list,
> bookmark-tests-get-bookmark-list,
> bookmark-tests-get-bookmark-record-list): New functions to test the
> records of the list of bookmarks.
>* test/lisp/bookmark-tests.el (bookmark-tests-make-record-list): New
> function to test the creation of a record from example.txt with a
> list of bookmarks loaded.
>* test/lisp/bookmark-tests.el (bookmark-tests-delete-all): New
> function to test `bookmark-delete-all'.
>* test/lisp/bookmark-tests.el (bookmark-test-bmenu-mark-all): New
> function to test `bookmark-bmenu-mark-all'.
>* test/lisp/bookmark-tests.el (bookmark-test-bmenu-unmark-all): New
> function to test `bookmark-bmenu-unmark-all'.
>* test/lisp/bookmark-tests.el (bookmark-test-bmenu-delete-all): New
> function to test `bookmark-bmenu-delete-all'.
>---
> lisp/bookmark.el | 79 ++++++++-
> test/lisp/bookmark-resources/test-list.bmk | 20 +++
> test/lisp/bookmark-tests.el | 183 +++++++++++++++++++++
> 3 files changed, 280 insertions(+), 2 deletions(-)
> create mode 100644 test/lisp/bookmark-resources/test-list.bmk
>
>diff --git a/lisp/bookmark.el b/lisp/bookmark.el
>index de7d60f97e..8a9c032930 100644
>--- a/lisp/bookmark.el
>+++ b/lisp/bookmark.el
>@@ -200,6 +200,7 @@ A non-nil value may result in truncated bookmark names."
> (define-key map "f" 'bookmark-insert-location) ;"f"ind
> (define-key map "r" 'bookmark-rename)
> (define-key map "d" 'bookmark-delete)
>+ (define-key map "D" 'bookmark-delete-all)
> (define-key map "l" 'bookmark-load)
> (define-key map "w" 'bookmark-write)
> (define-key map "s" 'bookmark-save)
>@@ -1374,6 +1375,21 @@ probably because we were called from there."
> (bookmark-save)))
>
>
>+;;;###autoload
>+(defun bookmark-delete-all (&optional arg)
>+ "Delete all bookmarks permanently.
>+Doesn't ask for confirmation if ARG is non-nil."
>+ (interactive "P")
>+ (when (or arg (yes-or-no-p "Delete all bookmarks permanently? "))
>+ (bookmark-maybe-load-default-file)
>+ (setq bookmark-alist-modification-count
>+ (+ bookmark-alist-modification-count (length bookmark-alist)))
>+ (setq bookmark-alist nil)
>+ (bookmark-bmenu-surreptitiously-rebuild-list)
>+ (when (bookmark-time-to-save-p)
>+ (bookmark-save))))
>+
>+
Better to call ARG something like NO-CONFIRM, so its name reflects its purpose.
And a suggestion: have the prompt say "Permanently delete all bookmarks? " instead.
Because of subtle assumptions associated with English word order, the current prompt ("Delete all bookmarks permanently? ") implies that the alternative might be to delete the bookmarks non-permanently. E.g., a user might think that if she responds "no", the next thing she'll get is a prompt offering various choices of non-permanent ways to delete them :-). This is not a huge problem, of course -- when she doesn't get any followup prompt, she'd figure out what's going on. But it's better to just ask the question in a way that makes the choice clear in the first place.
> (defun bookmark-time-to-save-p (&optional final-time)
> "Return t if it is time to save bookmarks to disk, nil otherwise.
> Optional argument FINAL-TIME means this is being called when Emacs
>@@ -1600,12 +1616,15 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
> (define-key map "\C-d" 'bookmark-bmenu-delete-backwards)
> (define-key map "x" 'bookmark-bmenu-execute-deletions)
> (define-key map "d" 'bookmark-bmenu-delete)
>+ (define-key map "D" 'bookmark-bmenu-delete-all)
> (define-key map " " 'next-line)
> (define-key map "n" 'next-line)
> (define-key map "p" 'previous-line)
> (define-key map "\177" 'bookmark-bmenu-backup-unmark)
> (define-key map "u" 'bookmark-bmenu-unmark)
>+ (define-key map "U" 'bookmark-bmenu-unmark-all)
> (define-key map "m" 'bookmark-bmenu-mark)
>+ (define-key map "M" 'bookmark-bmenu-mark-all)
> (define-key map "l" 'bookmark-bmenu-load)
> (define-key map "r" 'bookmark-bmenu-rename)
> (define-key map "R" 'bookmark-bmenu-relocate)
>@@ -1627,8 +1646,10 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
> ["Select Marked Bookmarks" bookmark-bmenu-select t]
> "---"
> ["Mark Bookmark" bookmark-bmenu-mark t]
>+ ["Mark all Bookmarks" bookmark-bmenu-mark-all t]
> ["Unmark Bookmark" bookmark-bmenu-unmark t]
> ["Unmark Backwards" bookmark-bmenu-backup-unmark t]
>+ ["Unmark all Bookmarks" bookmark-bmenu-unmark-all t]
> ["Toggle Display of Filenames" bookmark-bmenu-toggle-filenames t]
> ["Display Location of Bookmark" bookmark-bmenu-locate t]
> "---"
>@@ -1636,6 +1657,7 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
> ["Rename Bookmark" bookmark-bmenu-rename t]
> ["Relocate Bookmark's File" bookmark-bmenu-relocate t]
> ["Mark Bookmark for Deletion" bookmark-bmenu-delete t]
>+ ["Mark all Bookmarks for Deletion" bookmark-bmenu-delete-all t]
> ["Delete Marked Bookmarks" bookmark-bmenu-execute-deletions t])
> ("Annotations"
> ["Show Annotation for Current Bookmark" bookmark-bmenu-show-annotation t]
>@@ -1748,6 +1770,7 @@ Letters do not insert themselves; instead, they are commands.
> Bookmark names preceded by a \"*\" have annotations.
> \\<bookmark-bmenu-mode-map>
> \\[bookmark-bmenu-mark] -- mark bookmark to be displayed.
>+\\[bookmark-bmenu-mark-all] -- mark all listed bookmarks to be displayed.
> \\[bookmark-bmenu-select] -- select bookmark of line point is on.
> Also show bookmarks marked using m in other windows.
> \\[bookmark-bmenu-toggle-filenames] -- toggle displaying of filenames (they may obscure long bookmark names).
>@@ -1764,13 +1787,15 @@ Bookmark names preceded by a \"*\" have annotations.
> \\[bookmark-bmenu-relocate] -- relocate this bookmark's file (prompts for new file).
> \\[bookmark-bmenu-delete] -- mark this bookmark to be deleted, and move down.
> \\[bookmark-bmenu-delete-backwards] -- mark this bookmark to be deleted, and move up.
>-\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]'.
>+\\[bookmark-bmenu-delete-all] -- mark all listed bookmarks as to be deleted.
>+\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]' or `\\[bookmark-bmenu-delete-all]'.
> \\[bookmark-bmenu-save] -- save the current bookmark list in the default file.
> With a prefix arg, prompts for a file to save in.
> \\[bookmark-bmenu-load] -- load in a file of bookmarks (prompts for file.)
> \\[bookmark-bmenu-unmark] -- remove all kinds of marks from current line.
> With prefix argument, also move up one line.
> \\[bookmark-bmenu-backup-unmark] -- back up a line and remove marks.
>+\\[bookmark-bmenu-unmark-all] -- remove all kinds of marks from all listed bookmarks.
> \\[bookmark-bmenu-show-annotation] -- show the annotation, if it exists, for the current bookmark
> in another buffer.
> \\[bookmark-bmenu-show-all-annotations] -- show the annotations of all bookmarks in another buffer.
>@@ -1937,9 +1962,24 @@ If the annotation does not exist, do nothing."
> (bookmark-bmenu-ensure-position))))
>
>
>+(defun bookmark-bmenu-mark-all ()
>+ "Mark all listed bookmarks to be displayed by \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-select]."
>+ (interactive)
>+ (save-excursion
>+ (goto-char (point-min))
>+ (bookmark-bmenu-ensure-position)
>+ (with-buffer-modified-unmodified
>+ (let ((inhibit-read-only t))
>+ ;; FIXME: This assumes that the last line is empty.
>+ (while (not (eobp))
>+ (delete-char 1)
>+ (insert ?>)
>+ (forward-line 1))))))
>+
>+
Regarding the "FIXME" comment: that assumption is embedded throughout the bookmark-bmenu-* code, so I think there's no need for the "FIXME" (and indeed the comment may cause confusion, since its presence calls the assumption into question).
It might be good to make a followup change in which we document the last-line-is-empty assumption, but that's a separate change of course.
> (defun bookmark-bmenu-select ()
> "Select this line's bookmark; also display bookmarks marked with `>'.
>-You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] command."
>+You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] or \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark-all] commands."
> (interactive)
> (let ((bmrk (bookmark-bmenu-bookmark))
> (menu (current-buffer))
>@@ -2108,6 +2148,21 @@ Optional BACKUP means move up."
> (bookmark-bmenu-ensure-position))
>
>
Nice -- I appreciate your thoroughness in updating even these secondary aspects of the doc strings.
>+(defun bookmark-bmenu-unmark-all ()
>+ "Cancel all requested operations on all listed bookmarks."
>+ (interactive)
>+ (save-excursion
>+ (goto-char (point-min))
>+ (bookmark-bmenu-ensure-position)
>+ (with-buffer-modified-unmodified
>+ (let ((inhibit-read-only t))
>+ ;; FIXME: This assumes that the last line is empty.
>+ (while (not (eobp))
>+ (delete-char 1)
>+ (insert " ")
>+ (forward-line 1))))))
>+
>+
Same point as earlier about the "FIXME" comment.
> (defun bookmark-bmenu-delete ()
> "Mark bookmark on this line to be deleted.
> To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
>@@ -2133,6 +2188,23 @@ To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\
> (bookmark-bmenu-ensure-position))
>
>
>+(defun bookmark-bmenu-delete-all ()
>+ "Mark all listed bookmarks as to be deleted.
>+To remove all deletion marks, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-unmark-all].
>+To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
>+ (interactive)
>+ (save-excursion
>+ (goto-char (point-min))
>+ (bookmark-bmenu-ensure-position)
>+ (with-buffer-modified-unmodified
>+ (let ((inhibit-read-only t))
>+ ;; FIXME: This assumes that the last line is empty.
>+ (while (not (eobp))
>+ (delete-char 1)
>+ (insert ?D)
>+ (forward-line 1))))))
>+
>+
Same point as earlier about the "FIXME" comment.
> (defun bookmark-bmenu-execute-deletions ()
> "Delete bookmarks flagged `D'."
> (interactive)
>@@ -2292,6 +2364,9 @@ strings returned are not."
> (bindings--define-key map [delete]
> '(menu-item "Delete Bookmark..." bookmark-delete
> :help "Delete a bookmark from the bookmark list"))
>+ (bindings--define-key map [delete-all]
>+ '(menu-item "Delete all Bookmarks..." bookmark-delete-all
>+ :help "Delete all bookmarks from the bookmark list"))
> (bindings--define-key map [rename]
> '(menu-item "Rename Bookmark..." bookmark-rename
> :help "Change the name of a bookmark"))
>diff --git a/test/lisp/bookmark-resources/test-list.bmk b/test/lisp/bookmark-resources/test-list.bmk
>new file mode 100644
>index 0000000000..11a5c3f222
>--- /dev/null
>+++ b/test/lisp/bookmark-resources/test-list.bmk
>@@ -0,0 +1,20 @@
>+;;;; Emacs Bookmark Format Version 1 ;;;; -*- coding: utf-8-emacs -*-
>+;;; This format is meant to be slightly human-readable;
>+;;; nevertheless, you probably don't want to edit it.
>+;;; -*- End Of Bookmark File Format Version Stamp -*-
>+(("name0"
>+ (filename . "/some/file0")
>+ (front-context-string . "abc")
>+ (rear-context-string . "def")
>+ (position . 3))
>+("name1"
>+ (filename . "/some/file1")
>+ (front-context-string . "abc")
>+ (rear-context-string . "def")
>+ (position . 3))
>+("name2"
>+ (filename . "/some/file2")
>+ (front-context-string . "abc")
>+ (rear-context-string . "def")
>+ (position . 3))
>+)
>diff --git a/test/lisp/bookmark-tests.el b/test/lisp/bookmark-tests.el
>index b9c6ff9c54..3790498812 100644
>--- a/test/lisp/bookmark-tests.el
>+++ b/test/lisp/bookmark-tests.el
>@@ -83,6 +83,70 @@ the lexically-bound variable `buffer'."
> ,@body)
> (kill-buffer buffer))))
>
>+(defvar bookmark-tests-bookmark-file-list
>+ (expand-file-name "test-list.bmk" bookmark-tests-data-dir)
>+ "Bookmark file used for testing a list of bookmarks.")
>+
>+;; The values below should match `bookmark-tests-bookmark-file-list'
>+;; content. We cache these values to speed up tests.
>+(eval-and-compile ; needed by `with-bookmark-test-list' macro
>+ (defvar bookmark-tests-bookmark-list-0 '("name0"
>+ (filename . "/some/file0")
>+ (front-context-string . "ghi")
>+ (rear-context-string . "jkl")
>+ (position . 4))
>+ "Cached value used in bookmark-tests.el."))
>+
>+;; The values below should match `bookmark-tests-bookmark-file-list'
>+;; content. We cache these values to speed up tests.
>+(eval-and-compile ; needed by `with-bookmark-test-list' macro
>+ (defvar bookmark-tests-bookmark-list-1 '("name1"
>+ (filename . "/some/file1")
>+ (front-context-string . "mno")
>+ (rear-context-string . "pqr")
>+ (position . 5))
>+ "Cached value used in bookmark-tests.el."))
>+
>+;; The values below should match `bookmark-tests-bookmark-file-list'
>+;; content. We cache these values to speed up tests.
>+(eval-and-compile ; needed by `with-bookmark-test-list' macro
>+ (defvar bookmark-tests-bookmark-list-2 '("name2"
>+ (filename . "/some/file2")
>+ (front-context-string . "stu")
>+ (rear-context-string . "vwx")
>+ (position . 6))
>+ "Cached value used in bookmark-tests.el."))
>+
>+(defvar bookmark-tests-cache-timestamp-list
>+ (cons bookmark-tests-bookmark-file-list
>+ (nth 5 (file-attributes
>+ bookmark-tests-bookmark-file-list)))
>+ "Cached value used in bookmark-tests.el.")
>+
>+(defmacro with-bookmark-test-list (&rest body)
>+ "Create environment for testing bookmark.el and evaluate BODY.
>+Ensure a clean environment for testing, and do not change user
>+data when running tests interactively."
>+ `(with-temp-buffer
>+ (let ((bookmark-alist (quote (,(copy-sequence bookmark-tests-bookmark-list-0)
>+ ,(copy-sequence bookmark-tests-bookmark-list-1)
>+ ,(copy-sequence bookmark-tests-bookmark-list-2))))
>+ (bookmark-default-file bookmark-tests-bookmark-file-list)
>+ (bookmark-bookmarks-timestamp bookmark-tests-cache-timestamp-list)
>+ bookmark-save-flag)
>+ ,@body)))
>+
>+(defmacro with-bookmark-test-file-list (&rest body)
>+ "Create environment for testing bookmark.el and evaluate BODY.
>+Same as `with-bookmark-test-list' but also opens the resource file
>+example.txt in a buffer, which can be accessed by callers through
>+the lexically-bound variable `buffer'."
>+ `(let ((buffer (find-file-noselect bookmark-tests-example-file)))
>+ (unwind-protect
>+ (with-bookmark-test-list
>+ ,@body)
>+ (kill-buffer buffer))))
>+
> (ert-deftest bookmark-tests-all-names ()
> (with-bookmark-test
> (should (equal (bookmark-all-names) '("name")))))
>@@ -95,6 +159,22 @@ the lexically-bound variable `buffer'."
> (with-bookmark-test
> (should (equal (bookmark-get-bookmark-record "name") (cdr bookmark-tests-bookmark)))))
>
>+(ert-deftest bookmark-tests-all-names-list ()
>+ (with-bookmark-test-list
>+ (should (equal (bookmark-all-names) '("name0" "name1" "name2")))))
>+
>+(ert-deftest bookmark-tests-get-bookmark-list ()
>+ (with-bookmark-test-list
>+ (should (equal (bookmark-get-bookmark "name0") bookmark-tests-bookmark-list-0))
>+ (should (equal (bookmark-get-bookmark "name1") bookmark-tests-bookmark-list-1))
>+ (should (equal (bookmark-get-bookmark "name2") bookmark-tests-bookmark-list-2))))
>+
>+(ert-deftest bookmark-tests-get-bookmark-record-list ()
>+ (with-bookmark-test-list
>+ (should (equal (bookmark-get-bookmark-record "name0") (cdr bookmark-tests-bookmark-list-0)))
>+ (should (equal (bookmark-get-bookmark-record "name1") (cdr bookmark-tests-bookmark-list-1)))
>+ (should (equal (bookmark-get-bookmark-record "name2") (cdr bookmark-tests-bookmark-list-2)))))
>+
> (ert-deftest bookmark-tests-record-getters-and-setters-new ()
> (with-temp-buffer
> (let* ((buffer-file-name "test")
>@@ -130,6 +210,19 @@ the lexically-bound variable `buffer'."
> ;; calling twice gives same record
> (should (equal (bookmark-make-record) record))))))
>
>+(ert-deftest bookmark-tests-make-record-list ()
>+ (with-bookmark-test-file-list
>+ (let* ((record `("example.txt" (filename . ,bookmark-tests-example-file)
>+ (front-context-string . "is text file is ")
>+ (rear-context-string)
>+ (position . 3)
>+ (defaults "example.txt"))))
>+ (with-current-buffer buffer
>+ (goto-char 3)
>+ (should (equal (bookmark-make-record) record))
>+ ;; calling twice gives same record
>+ (should (equal (bookmark-make-record) record))))))
>+
> (ert-deftest bookmark-tests-make-record-function ()
> (with-bookmark-test
> (let ((buffer-file-name "test"))
>@@ -267,6 +360,11 @@ the lexically-bound variable `buffer'."
> (bookmark-delete "name")
> (should (equal bookmark-alist nil))))
>
>+(ert-deftest bookmark-tests-delete-all ()
>+ (with-bookmark-test-list
>+ (bookmark-delete-all t)
>+ (should (equal bookmark-alist nil))))
>+
> (defmacro with-bookmark-test-save-load (&rest body)
> "Create environment for testing bookmark.el and evaluate BODY.
> Same as `with-bookmark-test' but also sets a temporary
>@@ -340,6 +438,18 @@ testing `bookmark-bmenu-list'."
> ,@body)
> (kill-buffer bookmark-bmenu-buffer)))))
>
>+(defmacro with-bookmark-bmenu-test-list (&rest body)
>+ "Create environment for testing `bookmark-bmenu-list' and evaluate BODY.
>+Same as `with-bookmark-test-list' but with additions suitable for
>+testing `bookmark-bmenu-list'."
>+ `(with-bookmark-test-list
>+ (let ((bookmark-bmenu-buffer "*Bookmark List - Testing*"))
>+ (unwind-protect
>+ (save-window-excursion
>+ (bookmark-bmenu-list)
>+ ,@body)
>+ (kill-buffer bookmark-bmenu-buffer)))))
>+
> (ert-deftest bookmark-test-bmenu-edit-annotation/show-annotation ()
> (with-bookmark-bmenu-test
> (bookmark-set-annotation "name" "foo")
>@@ -396,6 +506,28 @@ testing `bookmark-bmenu-list'."
> (beginning-of-line)
> (should (looking-at "^>"))))
>
>+(ert-deftest bookmark-test-bmenu-mark-all ()
>+ (with-bookmark-bmenu-test-list
>+ (let ((here (point-max)))
>+ ;; Expect to not move the point
>+ (goto-char here)
>+ (bookmark-bmenu-mark-all)
>+ (should (equal here (point)))
>+ ;; Verify that all bookmarks are marked
>+ (goto-char (point-min))
>+ (bookmark-bmenu-ensure-position)
>+ (should (looking-at "^> "))
>+ (should (equal bookmark-tests-bookmark-list-0
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ (forward-line 1)
>+ (should (looking-at "^> "))
>+ (should (equal bookmark-tests-bookmark-list-1
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ (forward-line 1)
>+ (should (looking-at "^> "))
>+ (should (equal bookmark-tests-bookmark-list-2
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark)))))))
>+
> (ert-deftest bookmark-test-bmenu-any-marks ()
> (with-bookmark-bmenu-test
> (bookmark-bmenu-mark)
>@@ -410,12 +542,63 @@ testing `bookmark-bmenu-list'."
> (beginning-of-line)
> (should (looking-at "^ "))))
>
>+(ert-deftest bookmark-test-bmenu-unmark-all ()
>+ (with-bookmark-bmenu-test-list
>+ (bookmark-bmenu-mark-all)
>+ (let ((here (point-max)))
>+ ;; Expect to not move the point
>+ (goto-char here)
>+ (bookmark-bmenu-unmark-all)
>+ (should (equal here (point)))
>+ ;; Verify that all bookmarks are unmarked
>+ (goto-char (point-min))
>+ (bookmark-bmenu-ensure-position)
>+ (should (looking-at "^ "))
>+ (should (equal bookmark-tests-bookmark-list-0
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ (forward-line 1)
>+ (should (looking-at "^ "))
>+ (should (equal bookmark-tests-bookmark-list-1
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ (forward-line 1)
>+ (should (looking-at "^ "))
>+ (should (equal bookmark-tests-bookmark-list-2
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark)))))))
>+
> (ert-deftest bookmark-test-bmenu-delete ()
> (with-bookmark-bmenu-test
> (bookmark-bmenu-delete)
> (bookmark-bmenu-execute-deletions)
> (should (equal (length bookmark-alist) 0))))
>
>+(ert-deftest bookmark-test-bmenu-delete-all ()
>+ (with-bookmark-bmenu-test-list
>+ ;; Verify that unmarked bookmarks aren't deleted
>+ (bookmark-bmenu-execute-deletions)
>+ (should-not (eq bookmark-alist nil))
>+ (let ((here (point-max)))
>+ ;; Expect to not move the point
>+ (goto-char here)
>+ (bookmark-bmenu-delete-all)
>+ (should (equal here (point)))
>+ ;; Verify that all bookmarks are marked for deletion
>+ (goto-char (point-min))
>+ (bookmark-bmenu-ensure-position)
>+ (should (looking-at "^D "))
>+ (should (equal bookmark-tests-bookmark-list-0
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ (forward-line 1)
>+ (should (looking-at "^D "))
>+ (should (equal bookmark-tests-bookmark-list-1
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ (forward-line 1)
>+ (should (looking-at "^D "))
>+ (should (equal bookmark-tests-bookmark-list-2
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ ;; Verify that all bookmarks are deleted
>+ (bookmark-bmenu-execute-deletions)
>+ (should (eq bookmark-alist nil)))))
>+
> (ert-deftest bookmark-test-bmenu-locate ()
> (let (msg)
> (cl-letf (((symbol-function 'message)
These tests are a huge contribution -- thanks for taking the time to write them.
This patch looks good to me. I've tested it myself and it works as advertised, and the new tests pass 'make check'.
If you have time to revise the patch slightly as per above (if you agree with the comments, of course -- otherwise, let's discuss here) that would be great. I see that Stefan has already sent you the assignment paperwork.
Best regards,
-Karl
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-24 19:07 ` Karl Fogel
@ 2020-07-25 10:46 ` Matthew White
2020-07-25 10:58 ` Eli Zaretskii
` (2 more replies)
0 siblings, 3 replies; 52+ messages in thread
From: Matthew White @ 2020-07-25 10:46 UTC (permalink / raw)
To: Karl Fogel, emacs-devel
[-- Attachment #1.1: Type: text/plain, Size: 5595 bytes --]
> Hi, Matthew. Thanks so much for this. I have reviewed the patch
> against 'master' (since it applies cleanly there, whereas it does not
> quite apply cleanly to 'emacs-27'). It is very good; I have just
> some minor comments below.
Hi Karl. I'm flattered, thanks!
I appreciate so much that you took the time to review my patch, and
I eagerly followed your suggestions!
You will find attached both a patch that applies to master, and one
that applies to emacs-27... They are the same in term of changes.
I'm still waiting a reply to the copyright assignment... But in the
meantime we can freely discuss about the patch...
> 1) It helps to include a summary line at the top of your commit
> message, followed by a blank line. The summary line should be
> limited to 50 characters or fewer, if possible. (This is all
> documented in the "Commit messages" section in the 'CONTRIBUTE' file
> in the top level of the Emacs source tree, by the way). In your
> case, the Subject header of your email almost does the job -- we can
> shorten it a bit to fit within 50 characters:
>
> "Add ability to mark/unmark/delete all bookmarks"
Thanks for pointing that out. Now that I read the 'CONTRIBUTE' file,
I have some questions (by the way, I needed a remainder to read...):
1) I did commit a message with a summary line, as you described...
[ Actually my line was the subject of the thread, yours (above)
is better, hence I switched to your idea, thanks again ;) ]
And indeed I use `git format-patch -1` as suggested in 'CONTRIBUTE'
to produce a patch, but the command "strips" the summary line from
the commit message and uses it as 'Subject:' in the patch, where it
is prefixed with '[PATCH] ' if the option -k isn't also given...
Once the patch is applied via `git am`, the 'Subject:' is put back
in the commit message as summary, without the '[PATCH] ' prefix.
So, I wonder if you used `git am` to apply the patch...? Could you
tip me off, please?
2) With which quotes should I surround the variable/function names in
the commit message? `' or '' or ‘’? I could not figure that out...
> 2) In the commit message, you don't need to repeat the filename ("*
> lisp/bookmark.el") each time. You can just write it once, and then
> underneath it show everything affected in that file. Taking the
> first three entries in your original log message as an example:
>
> * lisp/bookmark.el (bookmark-delete-all): New function to delete
> all bookmarks.
> * lisp/bookmark.el (bookmark-bmenu-mark-all): New function to mark
> all bookmarks in the bookmark list buffer.
> * lisp/bookmark.el (bookmark-bmenu-unmark-all): New function to
> unmark all bookmarks in the bookmark list buffer.
>
> ...they would instead be written like this:
>
> * lisp/bookmark.el (bookmark-delete-all): New function to delete
> all bookmarks.
> (bookmark-bmenu-mark-all): New function to mark all bookmarks in
> the bookmark list buffer.
> (bookmark-bmenu-unmark-all): New function to unmark all
> bookmarks in the bookmark list buffer.
>
> Quick digression:
>
> You can use 'git log' to see this style used in past Emacs commits.
[x] Don't repeat the file name each time in the commit message.
...And yes, I did take a look at the `git log`...
> Better to call ARG something like NO-CONFIRM, so its name reflects
> its purpose.
>
> And a suggestion: have the prompt say "Permanently delete all
> bookmarks? " instead.
In `bookmark-delete-all':
[x] Chage ARG to NO-CONFIRM.
[x] Prompt "Permanently delete all bookmarks? ".
> Because of subtle assumptions associated with English word order, the
> current prompt ("Delete all bookmarks permanently? ") implies that
> the alternative might be to delete the bookmarks non-permanently.
That's a very smart thought... I'm truly impressed!
> Regarding the "FIXME" comment: that assumption is embedded throughout
> the bookmark-bmenu-* code, so I think there's no need for the "FIXME"
> (and indeed the comment may cause confusion, since its presence calls
> the assumption into question).
>
> It might be good to make a followup change in which we document the
> last-line-is-empty assumption, but that's a separate change of course.
[x] Remove the "FIXME" code comments.
> >+ ;; FIXME: This assumes that the last line is empty.
> >+ (while (not (eobp))
> These tests are a huge contribution -- thanks for taking the time to
> write them.
Following your guidelines is very instructing, Karl!
I also:
[x] Change `nameN' and `fileN' into `name-N' and `file-N' (i.e. name0
-> name-0) in all new functions and in the new test-list.bmk file.
[x] Indent new functions to split long lines into shorter ones.
[x] bookmark-tests.el (bookmark-test-bmenu-any-marks-list): New function
to test `bookmark-bmenu-any-marks' with a list of bookmarks.
> This patch looks good to me. I've tested it myself and it works as
> advertised, and the new tests pass 'make check'.
>
> If you have time to revise the patch slightly as per above (if you
> agree with the comments, of course -- otherwise, let's discuss here)
> that would be great. I see that Stefan has already sent you the
> assignment paperwork.
>
> Best regards,
> -Karl
Again, thank you very much Karl for all your effort reviewing my patch,
and especially for your brilliant insights!
I hope to get back soon with good news about the copyright assignment.
Catch you later!
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: master-Add-ability-to-mark-unmark-delete-all-bookmarks.patch --]
[-- Type: text/x-patch, Size: 24349 bytes --]
From 26b3590648720466a0a558e7c85da4eb7126827e Mon Sep 17 00:00:00 2001
From: Matthew White <mehw.is.me@inventati.org>
Date: Thu, 23 Jul 2020 21:14:32 +0000
Subject: [PATCH] Add ability to mark/unmark/delete all bookmarks
* lisp/bookmark.el (bookmark-delete-all): New function to delete all
bookmarks.
(bookmark-bmenu-mark-all): New function to mark all bookmarks in the
bookmark list buffer.
(bookmark-bmenu-unmark-all): New function to unmark all bookmarks in
the bookmark list buffer.
(bookmark-bmenu-delete-all): New function to mark for deletion all
bookmarks in the bookmark list buffer.
(bookmark-map): Map "D" to `bookmark-delete-all'.
(bookmark-bmenu-mode-map): New mappping for "M" to
`bookmark-bmenu-mark-all'.
(bookmark-bmenu-mode-map): New mappping for "U" to
`bookmark-bmenu-unmark-all'.
(bookmark-bmenu-mode-map): New mappping for "D" to
`bookmark-bmenu-delete-all'.
(bookmark-bmenu-mark-all): New bookmark menu to
`bookmark-delete-all'.
(easy-menu-define): New bookmark menu to `bookmark-bmenu-mark-all'.
(easy-menu-define): New bookmark menu to
`bookmark-bmenu-unmark-all'.
(easy-menu-define): New bookmark menu to
`bookmark-bmenu-delete-all'.
(bookmark-bmenu-select): Update docstring to include a reference to
`bookmark-bmenu-mark-all'.
(bookmark-bmenu-mode): Update docstring. Add/Update description:
`bookmark-bmenu-mark-all', `bookmark-bmenu-delete-all',
`bookmark-bmenu-execute-deletions', and `bookmark-bmenu-unmark-all'.
* test/lisp/bookmark-resources/test-list.bmk: New bookmark file to
test a list of bookmarks.
* test/lisp/bookmark-tests.el (bookmark-tests-bookmark-file-list): New
reference to the bookmark file used for testing a list of bookmarks.
(bookmark-tests-bookmark-list-0, bookmark-tests-bookmark-list-1,
bookmark-tests-bookmark-list-2): New cached values for testing a
list of bookmark.
(bookmark-tests-cache-timestamp-list): New variable to set
`bookmark-bookmarks-timestamp'.
(with-bookmark-test-list): New macro environment to test a list of
bookmarks.
(with-bookmark-test-file-list): New macro environment to test a list
of bookmarks with example.txt.
(with-bookmark-bmenu-test-list): New macro environment to test
functions about a list of bookmarks from `bookmark-bmenu-list'.
(bookmark-tests-all-names-list, bookmark-tests-get-bookmark-list,
bookmark-tests-get-bookmark-record-list): New functions to test the
records of the list of bookmarks.
(bookmark-tests-make-record-list): New function to test the creation
of a record from example.txt with a list of bookmarks loaded.
(bookmark-tests-delete-all): New function to test
`bookmark-delete-all'.
(bookmark-test-bmenu-any-marks-list): New function to test
`bookmark-bmenu-any-marks' with a list of bookmarks.
(bookmark-test-bmenu-mark-all): New function to test
`bookmark-bmenu-mark-all'.
(bookmark-test-bmenu-unmark-all): New function to test
`bookmark-bmenu-unmark-all'.
(bookmark-test-bmenu-delete-all): New function to test
`bookmark-bmenu-delete-all'.
---
lisp/bookmark.el | 77 +++++++-
test/lisp/bookmark-resources/test-list.bmk | 20 ++
test/lisp/bookmark-tests.el | 215 +++++++++++++++++++++
3 files changed, 310 insertions(+), 2 deletions(-)
create mode 100644 test/lisp/bookmark-resources/test-list.bmk
diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index de7d60f97e..870c41d5a8 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -200,6 +200,7 @@ A non-nil value may result in truncated bookmark names."
(define-key map "f" 'bookmark-insert-location) ;"f"ind
(define-key map "r" 'bookmark-rename)
(define-key map "d" 'bookmark-delete)
+ (define-key map "D" 'bookmark-delete-all)
(define-key map "l" 'bookmark-load)
(define-key map "w" 'bookmark-write)
(define-key map "s" 'bookmark-save)
@@ -1374,6 +1375,22 @@ probably because we were called from there."
(bookmark-save)))
+;;;###autoload
+(defun bookmark-delete-all (&optional no-confirm)
+ "Permanently delete all bookmarks.
+Doesn't ask for confirmation if NO-CONFIRM is non-nil."
+ (interactive "P")
+ (when (or no-confirm
+ (yes-or-no-p "Permanently delete all bookmarks? "))
+ (bookmark-maybe-load-default-file)
+ (setq bookmark-alist-modification-count
+ (+ bookmark-alist-modification-count (length bookmark-alist)))
+ (setq bookmark-alist nil)
+ (bookmark-bmenu-surreptitiously-rebuild-list)
+ (when (bookmark-time-to-save-p)
+ (bookmark-save))))
+
+
(defun bookmark-time-to-save-p (&optional final-time)
"Return t if it is time to save bookmarks to disk, nil otherwise.
Optional argument FINAL-TIME means this is being called when Emacs
@@ -1600,12 +1617,15 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
(define-key map "\C-d" 'bookmark-bmenu-delete-backwards)
(define-key map "x" 'bookmark-bmenu-execute-deletions)
(define-key map "d" 'bookmark-bmenu-delete)
+ (define-key map "D" 'bookmark-bmenu-delete-all)
(define-key map " " 'next-line)
(define-key map "n" 'next-line)
(define-key map "p" 'previous-line)
(define-key map "\177" 'bookmark-bmenu-backup-unmark)
(define-key map "u" 'bookmark-bmenu-unmark)
+ (define-key map "U" 'bookmark-bmenu-unmark-all)
(define-key map "m" 'bookmark-bmenu-mark)
+ (define-key map "M" 'bookmark-bmenu-mark-all)
(define-key map "l" 'bookmark-bmenu-load)
(define-key map "r" 'bookmark-bmenu-rename)
(define-key map "R" 'bookmark-bmenu-relocate)
@@ -1627,8 +1647,10 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
["Select Marked Bookmarks" bookmark-bmenu-select t]
"---"
["Mark Bookmark" bookmark-bmenu-mark t]
+ ["Mark all Bookmarks" bookmark-bmenu-mark-all t]
["Unmark Bookmark" bookmark-bmenu-unmark t]
["Unmark Backwards" bookmark-bmenu-backup-unmark t]
+ ["Unmark all Bookmarks" bookmark-bmenu-unmark-all t]
["Toggle Display of Filenames" bookmark-bmenu-toggle-filenames t]
["Display Location of Bookmark" bookmark-bmenu-locate t]
"---"
@@ -1636,6 +1658,7 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
["Rename Bookmark" bookmark-bmenu-rename t]
["Relocate Bookmark's File" bookmark-bmenu-relocate t]
["Mark Bookmark for Deletion" bookmark-bmenu-delete t]
+ ["Mark all Bookmarks for Deletion" bookmark-bmenu-delete-all t]
["Delete Marked Bookmarks" bookmark-bmenu-execute-deletions t])
("Annotations"
["Show Annotation for Current Bookmark" bookmark-bmenu-show-annotation t]
@@ -1748,6 +1771,7 @@ Letters do not insert themselves; instead, they are commands.
Bookmark names preceded by a \"*\" have annotations.
\\<bookmark-bmenu-mode-map>
\\[bookmark-bmenu-mark] -- mark bookmark to be displayed.
+\\[bookmark-bmenu-mark-all] -- mark all listed bookmarks to be displayed.
\\[bookmark-bmenu-select] -- select bookmark of line point is on.
Also show bookmarks marked using m in other windows.
\\[bookmark-bmenu-toggle-filenames] -- toggle displaying of filenames (they may obscure long bookmark names).
@@ -1764,13 +1788,15 @@ Bookmark names preceded by a \"*\" have annotations.
\\[bookmark-bmenu-relocate] -- relocate this bookmark's file (prompts for new file).
\\[bookmark-bmenu-delete] -- mark this bookmark to be deleted, and move down.
\\[bookmark-bmenu-delete-backwards] -- mark this bookmark to be deleted, and move up.
-\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]'.
+\\[bookmark-bmenu-delete-all] -- mark all listed bookmarks as to be deleted.
+\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]' or `\\[bookmark-bmenu-delete-all]'.
\\[bookmark-bmenu-save] -- save the current bookmark list in the default file.
With a prefix arg, prompts for a file to save in.
\\[bookmark-bmenu-load] -- load in a file of bookmarks (prompts for file.)
\\[bookmark-bmenu-unmark] -- remove all kinds of marks from current line.
With prefix argument, also move up one line.
\\[bookmark-bmenu-backup-unmark] -- back up a line and remove marks.
+\\[bookmark-bmenu-unmark-all] -- remove all kinds of marks from all listed bookmarks.
\\[bookmark-bmenu-show-annotation] -- show the annotation, if it exists, for the current bookmark
in another buffer.
\\[bookmark-bmenu-show-all-annotations] -- show the annotations of all bookmarks in another buffer.
@@ -1937,9 +1963,23 @@ If the annotation does not exist, do nothing."
(bookmark-bmenu-ensure-position))))
+(defun bookmark-bmenu-mark-all ()
+ "Mark all listed bookmarks to be displayed by \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-select]."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ (while (not (eobp))
+ (delete-char 1)
+ (insert ?>)
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-select ()
"Select this line's bookmark; also display bookmarks marked with `>'.
-You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] command."
+You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] or \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark-all] commands."
(interactive)
(let ((bmrk (bookmark-bmenu-bookmark))
(menu (current-buffer))
@@ -2108,6 +2148,20 @@ Optional BACKUP means move up."
(bookmark-bmenu-ensure-position))
+(defun bookmark-bmenu-unmark-all ()
+ "Cancel all requested operations on all listed bookmarks."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ (while (not (eobp))
+ (delete-char 1)
+ (insert " ")
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-delete ()
"Mark bookmark on this line to be deleted.
To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
@@ -2133,6 +2187,22 @@ To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\
(bookmark-bmenu-ensure-position))
+(defun bookmark-bmenu-delete-all ()
+ "Mark all listed bookmarks as to be deleted.
+To remove all deletion marks, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-unmark-all].
+To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ (while (not (eobp))
+ (delete-char 1)
+ (insert ?D)
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-execute-deletions ()
"Delete bookmarks flagged `D'."
(interactive)
@@ -2292,6 +2362,9 @@ strings returned are not."
(bindings--define-key map [delete]
'(menu-item "Delete Bookmark..." bookmark-delete
:help "Delete a bookmark from the bookmark list"))
+ (bindings--define-key map [delete-all]
+ '(menu-item "Delete all Bookmarks..." bookmark-delete-all
+ :help "Delete all bookmarks from the bookmark list"))
(bindings--define-key map [rename]
'(menu-item "Rename Bookmark..." bookmark-rename
:help "Change the name of a bookmark"))
diff --git a/test/lisp/bookmark-resources/test-list.bmk b/test/lisp/bookmark-resources/test-list.bmk
new file mode 100644
index 0000000000..696d64979b
--- /dev/null
+++ b/test/lisp/bookmark-resources/test-list.bmk
@@ -0,0 +1,20 @@
+;;;; Emacs Bookmark Format Version 1 ;;;; -*- coding: utf-8-emacs -*-
+;;; This format is meant to be slightly human-readable;
+;;; nevertheless, you probably don't want to edit it.
+;;; -*- End Of Bookmark File Format Version Stamp -*-
+(("name-0"
+ (filename . "/some/file-0")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+("name-1"
+ (filename . "/some/file-1")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+("name-2"
+ (filename . "/some/file-2")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+)
diff --git a/test/lisp/bookmark-tests.el b/test/lisp/bookmark-tests.el
index b9c6ff9c54..d5f7f24317 100644
--- a/test/lisp/bookmark-tests.el
+++ b/test/lisp/bookmark-tests.el
@@ -83,6 +83,70 @@ the lexically-bound variable `buffer'."
,@body)
(kill-buffer buffer))))
+(defvar bookmark-tests-bookmark-file-list
+ (expand-file-name "test-list.bmk" bookmark-tests-data-dir)
+ "Bookmark file used for testing a list of bookmarks.")
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-0 '("name-0"
+ (filename . "/some/file-0")
+ (front-context-string . "ghi")
+ (rear-context-string . "jkl")
+ (position . 4))
+ "Cached value used in bookmark-tests.el."))
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-1 '("name-1"
+ (filename . "/some/file-1")
+ (front-context-string . "mno")
+ (rear-context-string . "pqr")
+ (position . 5))
+ "Cached value used in bookmark-tests.el."))
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-2 '("name-2"
+ (filename . "/some/file-2")
+ (front-context-string . "stu")
+ (rear-context-string . "vwx")
+ (position . 6))
+ "Cached value used in bookmark-tests.el."))
+
+(defvar bookmark-tests-cache-timestamp-list
+ (cons bookmark-tests-bookmark-file-list
+ (nth 5 (file-attributes
+ bookmark-tests-bookmark-file-list)))
+ "Cached value used in bookmark-tests.el.")
+
+(defmacro with-bookmark-test-list (&rest body)
+ "Create environment for testing bookmark.el and evaluate BODY.
+Ensure a clean environment for testing, and do not change user
+data when running tests interactively."
+ `(with-temp-buffer
+ (let ((bookmark-alist (quote (,(copy-sequence bookmark-tests-bookmark-list-0)
+ ,(copy-sequence bookmark-tests-bookmark-list-1)
+ ,(copy-sequence bookmark-tests-bookmark-list-2))))
+ (bookmark-default-file bookmark-tests-bookmark-file-list)
+ (bookmark-bookmarks-timestamp bookmark-tests-cache-timestamp-list)
+ bookmark-save-flag)
+ ,@body)))
+
+(defmacro with-bookmark-test-file-list (&rest body)
+ "Create environment for testing bookmark.el and evaluate BODY.
+Same as `with-bookmark-test-list' but also opens the resource file
+example.txt in a buffer, which can be accessed by callers through
+the lexically-bound variable `buffer'."
+ `(let ((buffer (find-file-noselect bookmark-tests-example-file)))
+ (unwind-protect
+ (with-bookmark-test-list
+ ,@body)
+ (kill-buffer buffer))))
+
(ert-deftest bookmark-tests-all-names ()
(with-bookmark-test
(should (equal (bookmark-all-names) '("name")))))
@@ -95,6 +159,30 @@ the lexically-bound variable `buffer'."
(with-bookmark-test
(should (equal (bookmark-get-bookmark-record "name") (cdr bookmark-tests-bookmark)))))
+(ert-deftest bookmark-tests-all-names-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-all-names) '("name-0"
+ "name-1"
+ "name-2")))))
+
+(ert-deftest bookmark-tests-get-bookmark-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-get-bookmark "name-0")
+ bookmark-tests-bookmark-list-0))
+ (should (equal (bookmark-get-bookmark "name-1")
+ bookmark-tests-bookmark-list-1))
+ (should (equal (bookmark-get-bookmark "name-2")
+ bookmark-tests-bookmark-list-2))))
+
+(ert-deftest bookmark-tests-get-bookmark-record-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-get-bookmark-record "name-0")
+ (cdr bookmark-tests-bookmark-list-0)))
+ (should (equal (bookmark-get-bookmark-record "name-1")
+ (cdr bookmark-tests-bookmark-list-1)))
+ (should (equal (bookmark-get-bookmark-record "name-2")
+ (cdr bookmark-tests-bookmark-list-2)))))
+
(ert-deftest bookmark-tests-record-getters-and-setters-new ()
(with-temp-buffer
(let* ((buffer-file-name "test")
@@ -130,6 +218,19 @@ the lexically-bound variable `buffer'."
;; calling twice gives same record
(should (equal (bookmark-make-record) record))))))
+(ert-deftest bookmark-tests-make-record-list ()
+ (with-bookmark-test-file-list
+ (let* ((record `("example.txt" (filename . ,bookmark-tests-example-file)
+ (front-context-string . "is text file is ")
+ (rear-context-string)
+ (position . 3)
+ (defaults "example.txt"))))
+ (with-current-buffer buffer
+ (goto-char 3)
+ (should (equal (bookmark-make-record) record))
+ ;; calling twice gives same record
+ (should (equal (bookmark-make-record) record))))))
+
(ert-deftest bookmark-tests-make-record-function ()
(with-bookmark-test
(let ((buffer-file-name "test"))
@@ -267,6 +368,11 @@ the lexically-bound variable `buffer'."
(bookmark-delete "name")
(should (equal bookmark-alist nil))))
+(ert-deftest bookmark-tests-delete-all ()
+ (with-bookmark-test-list
+ (bookmark-delete-all t)
+ (should (equal bookmark-alist nil))))
+
(defmacro with-bookmark-test-save-load (&rest body)
"Create environment for testing bookmark.el and evaluate BODY.
Same as `with-bookmark-test' but also sets a temporary
@@ -340,6 +446,18 @@ testing `bookmark-bmenu-list'."
,@body)
(kill-buffer bookmark-bmenu-buffer)))))
+(defmacro with-bookmark-bmenu-test-list (&rest body)
+ "Create environment for testing `bookmark-bmenu-list' and evaluate BODY.
+Same as `with-bookmark-test-list' but with additions suitable for
+testing `bookmark-bmenu-list'."
+ `(with-bookmark-test-list
+ (let ((bookmark-bmenu-buffer "*Bookmark List - Testing*"))
+ (unwind-protect
+ (save-window-excursion
+ (bookmark-bmenu-list)
+ ,@body)
+ (kill-buffer bookmark-bmenu-buffer)))))
+
(ert-deftest bookmark-test-bmenu-edit-annotation/show-annotation ()
(with-bookmark-bmenu-test
(bookmark-set-annotation "name" "foo")
@@ -402,6 +520,52 @@ testing `bookmark-bmenu-list'."
(beginning-of-line)
(should (bookmark-bmenu-any-marks))))
+(ert-deftest bookmark-test-bmenu-mark-all ()
+ (with-bookmark-bmenu-test-list
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-mark-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are marked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark)))))))
+
+(ert-deftest bookmark-test-bmenu-any-marks-list ()
+ (with-bookmark-bmenu-test-list
+ ;; Mark just the second item
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (forward-line 1)
+ (bookmark-bmenu-mark)
+ ;; Verify that only the second item is marked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ ;; There should be at least one mark
+ (should (bookmark-bmenu-any-marks))))
+
(ert-deftest bookmark-test-bmenu-unmark ()
(with-bookmark-bmenu-test
(bookmark-bmenu-mark)
@@ -410,12 +574,63 @@ testing `bookmark-bmenu-list'."
(beginning-of-line)
(should (looking-at "^ "))))
+(ert-deftest bookmark-test-bmenu-unmark-all ()
+ (with-bookmark-bmenu-test-list
+ (bookmark-bmenu-mark-all)
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-unmark-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are unmarked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark)))))))
+
(ert-deftest bookmark-test-bmenu-delete ()
(with-bookmark-bmenu-test
(bookmark-bmenu-delete)
(bookmark-bmenu-execute-deletions)
(should (equal (length bookmark-alist) 0))))
+(ert-deftest bookmark-test-bmenu-delete-all ()
+ (with-bookmark-bmenu-test-list
+ ;; Verify that unmarked bookmarks aren't deleted
+ (bookmark-bmenu-execute-deletions)
+ (should-not (eq bookmark-alist nil))
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-delete-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are marked for deletion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ ;; Verify that all bookmarks are deleted
+ (bookmark-bmenu-execute-deletions)
+ (should (eq bookmark-alist nil)))))
+
(ert-deftest bookmark-test-bmenu-locate ()
(let (msg)
(cl-letf (((symbol-function 'message)
--
2.26.2
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: emacs-27-Add-ability-to-mark-unmark-delete-all-bookmarks.patch --]
[-- Type: text/x-patch, Size: 23933 bytes --]
From 3e7dd3655b3e28e47be88f60f3bb5f03f251e9ee Mon Sep 17 00:00:00 2001
From: Matthew White <mehw.is.me@inventati.org>
Date: Thu, 23 Jul 2020 21:14:32 +0000
Subject: [PATCH] Add ability to mark/unmark/delete all bookmarks
* lisp/bookmark.el (bookmark-delete-all): New function to delete all
bookmarks.
(bookmark-bmenu-mark-all): New function to mark all bookmarks in the
bookmark list buffer.
(bookmark-bmenu-unmark-all): New function to unmark all bookmarks in
the bookmark list buffer.
(bookmark-bmenu-delete-all): New function to mark for deletion all
bookmarks in the bookmark list buffer.
(bookmark-map): Map "D" to `bookmark-delete-all'.
(bookmark-bmenu-mode-map): New mappping for "M" to
`bookmark-bmenu-mark-all'.
(bookmark-bmenu-mode-map): New mappping for "U" to
`bookmark-bmenu-unmark-all'.
(bookmark-bmenu-mode-map): New mappping for "D" to
`bookmark-bmenu-delete-all'.
(bookmark-bmenu-mark-all): New bookmark menu to
`bookmark-delete-all'.
(easy-menu-define): New bookmark menu to `bookmark-bmenu-mark-all'.
(easy-menu-define): New bookmark menu to
`bookmark-bmenu-unmark-all'.
(easy-menu-define): New bookmark menu to
`bookmark-bmenu-delete-all'.
(bookmark-bmenu-select): Update docstring to include a reference to
`bookmark-bmenu-mark-all'.
(bookmark-bmenu-mode): Update docstring. Add/Update description:
`bookmark-bmenu-mark-all', `bookmark-bmenu-delete-all',
`bookmark-bmenu-execute-deletions', and `bookmark-bmenu-unmark-all'.
* test/lisp/bookmark-resources/test-list.bmk: New bookmark file to
test a list of bookmarks.
* test/lisp/bookmark-tests.el (bookmark-tests-bookmark-file-list): New
reference to the bookmark file used for testing a list of bookmarks.
(bookmark-tests-bookmark-list-0, bookmark-tests-bookmark-list-1,
bookmark-tests-bookmark-list-2): New cached values for testing a
list of bookmark.
(bookmark-tests-cache-timestamp-list): New variable to set
`bookmark-bookmarks-timestamp'.
(with-bookmark-test-list): New macro environment to test a list of
bookmarks.
(with-bookmark-test-file-list): New macro environment to test a list
of bookmarks with example.txt.
(with-bookmark-bmenu-test-list): New macro environment to test
functions about a list of bookmarks from `bookmark-bmenu-list'.
(bookmark-tests-all-names-list, bookmark-tests-get-bookmark-list,
bookmark-tests-get-bookmark-record-list): New functions to test the
records of the list of bookmarks.
(bookmark-tests-make-record-list): New function to test the creation
of a record from example.txt with a list of bookmarks loaded.
(bookmark-tests-delete-all): New function to test
`bookmark-delete-all'.
(bookmark-test-bmenu-any-marks-list): New function to test
`bookmark-bmenu-any-marks' with a list of bookmarks.
(bookmark-test-bmenu-mark-all): New function to test
`bookmark-bmenu-mark-all'.
(bookmark-test-bmenu-unmark-all): New function to test
`bookmark-bmenu-unmark-all'.
(bookmark-test-bmenu-delete-all): New function to test
`bookmark-bmenu-delete-all'.
---
lisp/bookmark.el | 77 +++++++-
test/lisp/bookmark-resources/test-list.bmk | 20 ++
test/lisp/bookmark-tests.el | 215 +++++++++++++++++++++
3 files changed, 310 insertions(+), 2 deletions(-)
create mode 100644 test/lisp/bookmark-resources/test-list.bmk
diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index e69d9f529c..c45a5103fe 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -200,6 +200,7 @@ A non-nil value may result in truncated bookmark names."
(define-key map "f" 'bookmark-insert-location) ;"f"ind
(define-key map "r" 'bookmark-rename)
(define-key map "d" 'bookmark-delete)
+ (define-key map "D" 'bookmark-delete-all)
(define-key map "l" 'bookmark-load)
(define-key map "w" 'bookmark-write)
(define-key map "s" 'bookmark-save)
@@ -1372,6 +1373,22 @@ probably because we were called from there."
(bookmark-save)))
+;;;###autoload
+(defun bookmark-delete-all (&optional no-confirm)
+ "Permanently delete all bookmarks.
+Doesn't ask for confirmation if NO-CONFIRM is non-nil."
+ (interactive "P")
+ (when (or no-confirm
+ (yes-or-no-p "Permanently delete all bookmarks? "))
+ (bookmark-maybe-load-default-file)
+ (setq bookmark-alist-modification-count
+ (+ bookmark-alist-modification-count (length bookmark-alist)))
+ (setq bookmark-alist nil)
+ (bookmark-bmenu-surreptitiously-rebuild-list)
+ (when (bookmark-time-to-save-p)
+ (bookmark-save))))
+
+
(defun bookmark-time-to-save-p (&optional final-time)
"Return t if it is time to save bookmarks to disk, nil otherwise.
Optional argument FINAL-TIME means this is being called when Emacs
@@ -1598,12 +1615,15 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
(define-key map "\C-d" 'bookmark-bmenu-delete-backwards)
(define-key map "x" 'bookmark-bmenu-execute-deletions)
(define-key map "d" 'bookmark-bmenu-delete)
+ (define-key map "D" 'bookmark-bmenu-delete-all)
(define-key map " " 'next-line)
(define-key map "n" 'next-line)
(define-key map "p" 'previous-line)
(define-key map "\177" 'bookmark-bmenu-backup-unmark)
(define-key map "u" 'bookmark-bmenu-unmark)
+ (define-key map "U" 'bookmark-bmenu-unmark-all)
(define-key map "m" 'bookmark-bmenu-mark)
+ (define-key map "M" 'bookmark-bmenu-mark-all)
(define-key map "l" 'bookmark-bmenu-load)
(define-key map "r" 'bookmark-bmenu-rename)
(define-key map "R" 'bookmark-bmenu-relocate)
@@ -1625,8 +1645,10 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
["Select Marked Bookmarks" bookmark-bmenu-select t]
"---"
["Mark Bookmark" bookmark-bmenu-mark t]
+ ["Mark all Bookmarks" bookmark-bmenu-mark-all t]
["Unmark Bookmark" bookmark-bmenu-unmark t]
["Unmark Backwards" bookmark-bmenu-backup-unmark t]
+ ["Unmark all Bookmarks" bookmark-bmenu-unmark-all t]
["Toggle Display of Filenames" bookmark-bmenu-toggle-filenames t]
["Display Location of Bookmark" bookmark-bmenu-locate t]
"---"
@@ -1634,6 +1656,7 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
["Rename Bookmark" bookmark-bmenu-rename t]
["Relocate Bookmark's File" bookmark-bmenu-relocate t]
["Mark Bookmark for Deletion" bookmark-bmenu-delete t]
+ ["Mark all Bookmarks for Deletion" bookmark-bmenu-delete-all t]
["Delete Marked Bookmarks" bookmark-bmenu-execute-deletions t])
("Annotations"
["Show Annotation for Current Bookmark" bookmark-bmenu-show-annotation t]
@@ -1746,6 +1769,7 @@ Letters do not insert themselves; instead, they are commands.
Bookmark names preceded by a \"*\" have annotations.
\\<bookmark-bmenu-mode-map>
\\[bookmark-bmenu-mark] -- mark bookmark to be displayed.
+\\[bookmark-bmenu-mark-all] -- mark all listed bookmarks to be displayed.
\\[bookmark-bmenu-select] -- select bookmark of line point is on.
Also show bookmarks marked using m in other windows.
\\[bookmark-bmenu-toggle-filenames] -- toggle displaying of filenames (they may obscure long bookmark names).
@@ -1762,13 +1786,15 @@ Bookmark names preceded by a \"*\" have annotations.
\\[bookmark-bmenu-relocate] -- relocate this bookmark's file (prompts for new file).
\\[bookmark-bmenu-delete] -- mark this bookmark to be deleted, and move down.
\\[bookmark-bmenu-delete-backwards] -- mark this bookmark to be deleted, and move up.
-\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]'.
+\\[bookmark-bmenu-delete-all] -- mark all listed bookmarks as to be deleted.
+\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]' or `\\[bookmark-bmenu-delete-all]'.
\\[bookmark-bmenu-save] -- save the current bookmark list in the default file.
With a prefix arg, prompts for a file to save in.
\\[bookmark-bmenu-load] -- load in a file of bookmarks (prompts for file.)
\\[bookmark-bmenu-unmark] -- remove all kinds of marks from current line.
With prefix argument, also move up one line.
\\[bookmark-bmenu-backup-unmark] -- back up a line and remove marks.
+\\[bookmark-bmenu-unmark-all] -- remove all kinds of marks from all listed bookmarks.
\\[bookmark-bmenu-show-annotation] -- show the annotation, if it exists, for the current bookmark
in another buffer.
\\[bookmark-bmenu-show-all-annotations] -- show the annotations of all bookmarks in another buffer.
@@ -1935,9 +1961,23 @@ If the annotation does not exist, do nothing."
(bookmark-bmenu-ensure-position))))
+(defun bookmark-bmenu-mark-all ()
+ "Mark all listed bookmarks to be displayed by \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-select]."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ (while (not (eobp))
+ (delete-char 1)
+ (insert ?>)
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-select ()
"Select this line's bookmark; also display bookmarks marked with `>'.
-You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] command."
+You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] or \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark-all] commands."
(interactive)
(let ((bmrk (bookmark-bmenu-bookmark))
(menu (current-buffer))
@@ -2106,6 +2146,20 @@ Optional BACKUP means move up."
(bookmark-bmenu-ensure-position))
+(defun bookmark-bmenu-unmark-all ()
+ "Cancel all requested operations on all listed bookmarks."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ (while (not (eobp))
+ (delete-char 1)
+ (insert " ")
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-delete ()
"Mark bookmark on this line to be deleted.
To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
@@ -2131,6 +2185,22 @@ To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\
(bookmark-bmenu-ensure-position))
+(defun bookmark-bmenu-delete-all ()
+ "Mark all listed bookmarks as to be deleted.
+To remove all deletion marks, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-unmark-all].
+To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ (while (not (eobp))
+ (delete-char 1)
+ (insert ?D)
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-execute-deletions ()
"Delete bookmarks flagged `D'."
(interactive)
@@ -2290,6 +2360,9 @@ strings returned are not."
(bindings--define-key map [delete]
'(menu-item "Delete Bookmark..." bookmark-delete
:help "Delete a bookmark from the bookmark list"))
+ (bindings--define-key map [delete-all]
+ '(menu-item "Delete all Bookmarks..." bookmark-delete-all
+ :help "Delete all bookmarks from the bookmark list"))
(bindings--define-key map [rename]
'(menu-item "Rename Bookmark..." bookmark-rename
:help "Change the name of a bookmark"))
diff --git a/test/lisp/bookmark-resources/test-list.bmk b/test/lisp/bookmark-resources/test-list.bmk
new file mode 100644
index 0000000000..696d64979b
--- /dev/null
+++ b/test/lisp/bookmark-resources/test-list.bmk
@@ -0,0 +1,20 @@
+;;;; Emacs Bookmark Format Version 1 ;;;; -*- coding: utf-8-emacs -*-
+;;; This format is meant to be slightly human-readable;
+;;; nevertheless, you probably don't want to edit it.
+;;; -*- End Of Bookmark File Format Version Stamp -*-
+(("name-0"
+ (filename . "/some/file-0")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+("name-1"
+ (filename . "/some/file-1")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+("name-2"
+ (filename . "/some/file-2")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+)
diff --git a/test/lisp/bookmark-tests.el b/test/lisp/bookmark-tests.el
index 7e0384b724..726db52943 100644
--- a/test/lisp/bookmark-tests.el
+++ b/test/lisp/bookmark-tests.el
@@ -82,6 +82,70 @@ the lexically-bound variable `buffer'."
,@body)
(kill-buffer buffer))))
+(defvar bookmark-tests-bookmark-file-list
+ (expand-file-name "test-list.bmk" bookmark-tests-data-dir)
+ "Bookmark file used for testing a list of bookmarks.")
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-0 '("name-0"
+ (filename . "/some/file-0")
+ (front-context-string . "ghi")
+ (rear-context-string . "jkl")
+ (position . 4))
+ "Cached value used in bookmark-tests.el."))
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-1 '("name-1"
+ (filename . "/some/file-1")
+ (front-context-string . "mno")
+ (rear-context-string . "pqr")
+ (position . 5))
+ "Cached value used in bookmark-tests.el."))
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-2 '("name-2"
+ (filename . "/some/file-2")
+ (front-context-string . "stu")
+ (rear-context-string . "vwx")
+ (position . 6))
+ "Cached value used in bookmark-tests.el."))
+
+(defvar bookmark-tests-cache-timestamp-list
+ (cons bookmark-tests-bookmark-file-list
+ (nth 5 (file-attributes
+ bookmark-tests-bookmark-file-list)))
+ "Cached value used in bookmark-tests.el.")
+
+(defmacro with-bookmark-test-list (&rest body)
+ "Create environment for testing bookmark.el and evaluate BODY.
+Ensure a clean environment for testing, and do not change user
+data when running tests interactively."
+ `(with-temp-buffer
+ (let ((bookmark-alist (quote (,(copy-sequence bookmark-tests-bookmark-list-0)
+ ,(copy-sequence bookmark-tests-bookmark-list-1)
+ ,(copy-sequence bookmark-tests-bookmark-list-2))))
+ (bookmark-default-file bookmark-tests-bookmark-file-list)
+ (bookmark-bookmarks-timestamp bookmark-tests-cache-timestamp-list)
+ bookmark-save-flag)
+ ,@body)))
+
+(defmacro with-bookmark-test-file-list (&rest body)
+ "Create environment for testing bookmark.el and evaluate BODY.
+Same as `with-bookmark-test-list' but also opens the resource file
+example.txt in a buffer, which can be accessed by callers through
+the lexically-bound variable `buffer'."
+ `(let ((buffer (find-file-noselect bookmark-tests-example-file)))
+ (unwind-protect
+ (with-bookmark-test-list
+ ,@body)
+ (kill-buffer buffer))))
+
(ert-deftest bookmark-tests-all-names ()
(with-bookmark-test
(should (equal (bookmark-all-names) '("name")))))
@@ -94,6 +158,30 @@ the lexically-bound variable `buffer'."
(with-bookmark-test
(should (equal (bookmark-get-bookmark-record "name") (cdr bookmark-tests-bookmark)))))
+(ert-deftest bookmark-tests-all-names-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-all-names) '("name-0"
+ "name-1"
+ "name-2")))))
+
+(ert-deftest bookmark-tests-get-bookmark-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-get-bookmark "name-0")
+ bookmark-tests-bookmark-list-0))
+ (should (equal (bookmark-get-bookmark "name-1")
+ bookmark-tests-bookmark-list-1))
+ (should (equal (bookmark-get-bookmark "name-2")
+ bookmark-tests-bookmark-list-2))))
+
+(ert-deftest bookmark-tests-get-bookmark-record-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-get-bookmark-record "name-0")
+ (cdr bookmark-tests-bookmark-list-0)))
+ (should (equal (bookmark-get-bookmark-record "name-1")
+ (cdr bookmark-tests-bookmark-list-1)))
+ (should (equal (bookmark-get-bookmark-record "name-2")
+ (cdr bookmark-tests-bookmark-list-2)))))
+
(ert-deftest bookmark-tests-record-getters-and-setters-new ()
(with-temp-buffer
(let* ((buffer-file-name "test")
@@ -129,6 +217,19 @@ the lexically-bound variable `buffer'."
;; calling twice gives same record
(should (equal (bookmark-make-record) record))))))
+(ert-deftest bookmark-tests-make-record-list ()
+ (with-bookmark-test-file-list
+ (let* ((record `("example.txt" (filename . ,bookmark-tests-example-file)
+ (front-context-string . "is text file is ")
+ (rear-context-string)
+ (position . 3)
+ (defaults "example.txt"))))
+ (with-current-buffer buffer
+ (goto-char 3)
+ (should (equal (bookmark-make-record) record))
+ ;; calling twice gives same record
+ (should (equal (bookmark-make-record) record))))))
+
(ert-deftest bookmark-tests-make-record-function ()
(with-bookmark-test
(let ((buffer-file-name "test"))
@@ -266,6 +367,11 @@ the lexically-bound variable `buffer'."
(bookmark-delete "name")
(should (equal bookmark-alist nil))))
+(ert-deftest bookmark-tests-delete-all ()
+ (with-bookmark-test-list
+ (bookmark-delete-all t)
+ (should (equal bookmark-alist nil))))
+
(defmacro with-bookmark-test-save-load (&rest body)
"Create environment for testing bookmark.el and evaluate BODY.
Same as `with-bookmark-test' but also sets a temporary
@@ -339,6 +445,18 @@ testing `bookmark-bmenu-list'."
,@body)
(kill-buffer bookmark-bmenu-buffer)))))
+(defmacro with-bookmark-bmenu-test-list (&rest body)
+ "Create environment for testing `bookmark-bmenu-list' and evaluate BODY.
+Same as `with-bookmark-test-list' but with additions suitable for
+testing `bookmark-bmenu-list'."
+ `(with-bookmark-test-list
+ (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")
@@ -362,5 +480,102 @@ testing `bookmark-bmenu-list'."
(should (equal (buffer-name (current-buffer)) bookmark-bmenu-buffer))
(should (looking-at "name"))))
+(ert-deftest bookmark-test-bmenu-mark-all ()
+ (with-bookmark-bmenu-test-list
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-mark-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are marked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark)))))))
+
+(ert-deftest bookmark-test-bmenu-any-marks-list ()
+ (with-bookmark-bmenu-test-list
+ ;; Mark just the second item
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (forward-line 1)
+ (bookmark-bmenu-mark)
+ ;; Verify that only the second item is marked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ ;; There should be at least one mark
+ (should (bookmark-bmenu-any-marks))))
+
+(ert-deftest bookmark-test-bmenu-unmark-all ()
+ (with-bookmark-bmenu-test-list
+ (bookmark-bmenu-mark-all)
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-unmark-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are unmarked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark)))))))
+
+(ert-deftest bookmark-test-bmenu-delete-all ()
+ (with-bookmark-bmenu-test-list
+ ;; Verify that unmarked bookmarks aren't deleted
+ (bookmark-bmenu-execute-deletions)
+ (should-not (eq bookmark-alist nil))
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-delete-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are marked for deletion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ ;; Verify that all bookmarks are deleted
+ (bookmark-bmenu-execute-deletions)
+ (should (eq bookmark-alist nil)))))
+
(provide 'bookmark-tests)
;;; bookmark-tests.el ends here
--
2.26.2
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-25 10:46 ` Matthew White
@ 2020-07-25 10:58 ` Eli Zaretskii
2020-07-25 15:33 ` Michael Albinus
2020-07-25 20:36 ` Karl Fogel
2020-07-29 22:16 ` Karl Fogel
2 siblings, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2020-07-25 10:58 UTC (permalink / raw)
To: Matthew White; +Cc: kfogel, emacs-devel
> Date: Sat, 25 Jul 2020 12:46:18 +0200
> From: Matthew White <mehw.is.me@inventati.org>
>
> 2) With which quotes should I surround the variable/function names in
> the commit message? `' or '' or ‘’? I could not figure that out...
You should quote 'like this' in commit log messages (and also in NEWS
entries and comments).
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Extend tabulated-list-mode to support marks
2020-07-24 19:00 ` Extend tabulated-list-mode to support marks Yuri Khan
@ 2020-07-25 13:46 ` Dirk-Jan C. Binnema
2020-07-25 14:23 ` Stefan Monnier
2020-07-26 12:43 ` Stefan Kangas
2 siblings, 0 replies; 52+ messages in thread
From: Dirk-Jan C. Binnema @ 2020-07-25 13:46 UTC (permalink / raw)
To: emacs-devel
On Friday Jul 24 2020, Yuri Khan wrote:
> On Fri, 24 Jul 2020 at 22:05, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
>> > and (2) if maybe tabulated-list-mode would benefit from a generic
>> > implementation of multiple marking, unmarking, marking-for-deletion,
>> > and command application.
>>
>> Maybe "for deletion" is too application-specific (some uses of
>> `tabulated-list-mode` can't "delete" or at least not conveniently enough
>> that it's a common use case), but a definite +1 for the rest.
>
> My point is that there are many instances of a generic scenario:
> Display a list of items; enable the user to perform operations on a
> single item at point or to mark multiple items and operate on them as
> a subset. Currently, each mode derived from ‘tabulated-list-mode’ has
> to reimplement marking, unmarking, mark preservation over refresh,
> iterating over marked items, and fallback to the item at point when no
> marks are set. One high-visible derived mode hijacks the “unmark all”
> key for “mark upgradable packages”. I think they would benefit from a
> common implementation and the unification of the UI stemming from
> that.
>
>
> One possibility would be for a derived mode to set some buffer-local
> variables to inform ‘tabulated-list-mode’ which marks it supports.
> E.g.:
>
> * Set ‘tabulated-list-delete-function’ to a non-nil function that will
> delete the item at point or signal an error. Then the command bound to
> ‘D’ will invoke that function and remove its visual representation
> from the buffer; ‘d’ will mark the item at point for deletion; ‘x’
> will go through all the items marked for deletion and actually delete
> and remove them; ‘u’ will unmark the item at point; and ‘U’ will
> unmark all items.
>
> * Set ‘tabulated-list-marks’ to a string of characters. If that string
> contains ‘>’, ‘m’ will mark the item at point with ‘>’. Other marks
> listed in the string can be set by the derived mode binding a key to a
> function that will call (tabulated-list-set-mark CHAR). ‘u’ and ‘U’
> will unmark the item at point or all items, respectively. A helper
> function, (tabulated-list-map CHAR FUNC), will execute FUNC on each
> item marked CHAR or on the item at point if no items are marked,
> optionally removing the item or unmarking it depending on FUNC’s
> return value; this can be invoked in any commands bound in the derived
> mode keymap.
>
> I might implement something if this general direction is deemed
> desirable. Details can be discussed.
Something like that would be most welcome; I'm using a custom
'dired-like' implementation for mu4e's header-view, and I need the
marking / unmarking, applying commands, adding/removing entries etc.; if
emacs were to gain some standard facility for that, that would really
help.
Some bonus items would be to allow for a tree-view, with
expand/collapse, and multi-line items.
Kind regards,
Dirk.
--
Dirk-Jan C. Binnema Helsinki, Finland
e:djcb@djcbsoftware.nl w:www.djcbsoftware.nl
gpg: 6987 9CED 1745 9375 0F14 DA98 11DD FEA9 DCC4 A036
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Extend tabulated-list-mode to support marks
2020-07-24 19:00 ` Extend tabulated-list-mode to support marks Yuri Khan
2020-07-25 13:46 ` Dirk-Jan C. Binnema
@ 2020-07-25 14:23 ` Stefan Monnier
2020-07-25 14:32 ` Yuri Khan
2020-07-26 12:43 ` Stefan Kangas
2 siblings, 1 reply; 52+ messages in thread
From: Stefan Monnier @ 2020-07-25 14:23 UTC (permalink / raw)
To: Yuri Khan; +Cc: Emacs developers, Matthew White
>> > and (2) if maybe tabulated-list-mode would benefit from a generic
>> > implementation of multiple marking, unmarking, marking-for-deletion,
>> > and command application.
>>
>> Maybe "for deletion" is too application-specific (some uses of
>> `tabulated-list-mode` can't "delete" or at least not conveniently enough
>> that it's a common use case), but a definite +1 for the rest.
>
> My point is that there are many instances of a generic scenario:
Fully agreed. I was just pointing out that to keep it generic it might
be better to start with limiting it to support "mark/unmark" and "apply
command to marked elements" without presuming that "deletion" might be
such a command (e.g. in package.el I'm not sure "deletion" makes a lot
of sense since most of the entries listed reflect the existence of
something on a remote server that's not under the user's control).
Stefan
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Extend tabulated-list-mode to support marks
2020-07-25 14:23 ` Stefan Monnier
@ 2020-07-25 14:32 ` Yuri Khan
0 siblings, 0 replies; 52+ messages in thread
From: Yuri Khan @ 2020-07-25 14:32 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Emacs developers, Matthew White
On Sat, 25 Jul 2020 at 21:23, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> >> > and (2) if maybe tabulated-list-mode would benefit from a generic
> >> > implementation of multiple marking, unmarking, marking-for-deletion,
> >> > and command application.
> >>
> >> Maybe "for deletion" is too application-specific (some uses of
> >> `tabulated-list-mode` can't "delete" or at least not conveniently enough
> >> that it's a common use case), but a definite +1 for the rest.
> >
> > My point is that there are many instances of a generic scenario:
>
> Fully agreed. I was just pointing out that to keep it generic it might
> be better to start with limiting it to support "mark/unmark" and "apply
> command to marked elements" without presuming that "deletion" might be
> such a command (e.g. in package.el I'm not sure "deletion" makes a lot
> of sense since most of the entries listed reflect the existence of
> something on a remote server that's not under the user's control).
Fair enough. Given a generic marking facility, derived modes can just
treat deletion as one kind of a mark.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-25 10:58 ` Eli Zaretskii
@ 2020-07-25 15:33 ` Michael Albinus
2020-07-25 15:38 ` Eli Zaretskii
0 siblings, 1 reply; 52+ messages in thread
From: Michael Albinus @ 2020-07-25 15:33 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: kfogel, emacs-devel, Matthew White
Eli Zaretskii <eliz@gnu.org> writes:
Hi Eli,
>> 2) With which quotes should I surround the variable/function names in
>> the commit message? `' or '' or ‘’? I could not figure that out...
>
> You should quote 'like this' in commit log messages (and also in NEWS
> entries and comments).
In Lisp comments, objects quoted 'like-this' are not
highlighted. Objects quoted `like-this' are. That's why the latter
quotation seems to be preferred for Lisp comments, IME.
Best regards, Michael.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-25 15:33 ` Michael Albinus
@ 2020-07-25 15:38 ` Eli Zaretskii
2020-07-25 15:56 ` Michael Albinus
0 siblings, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2020-07-25 15:38 UTC (permalink / raw)
To: Michael Albinus; +Cc: kfogel, emacs-devel, mehw.is.me
> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Matthew White <mehw.is.me@inventati.org>, kfogel@red-bean.com,
> emacs-devel@gnu.org
> Date: Sat, 25 Jul 2020 17:33:52 +0200
>
> > You should quote 'like this' in commit log messages (and also in NEWS
> > entries and comments).
>
> In Lisp comments, objects quoted 'like-this' are not
> highlighted. Objects quoted `like-this' are. That's why the latter
> quotation seems to be preferred for Lisp comments, IME.
What is the utility of highlighting quoted symbols in comments? Do
you mean doc strings?
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-25 15:38 ` Eli Zaretskii
@ 2020-07-25 15:56 ` Michael Albinus
2020-07-25 16:30 ` Eli Zaretskii
0 siblings, 1 reply; 52+ messages in thread
From: Michael Albinus @ 2020-07-25 15:56 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: kfogel, emacs-devel, mehw.is.me
Eli Zaretskii <eliz@gnu.org> writes:
>> In Lisp comments, objects quoted 'like-this' are not
>> highlighted. Objects quoted `like-this' are. That's why the latter
>> quotation seems to be preferred for Lisp comments, IME.
>
> What is the utility of highlighting quoted symbols in comments? Do
> you mean doc strings?
No, I mean Lisp comments. I like highlighted symbols there, for discoverability.
Best regards, Michael.
^ permalink raw reply [flat|nested] 52+ messages in thread
* RE: Add new functions to mark/unmark/delete all bookmarks
[not found] ` <<83d04jd2pm.fsf@gnu.org>
@ 2020-07-25 16:07 ` Drew Adams
0 siblings, 0 replies; 52+ messages in thread
From: Drew Adams @ 2020-07-25 16:07 UTC (permalink / raw)
To: Eli Zaretskii, Michael Albinus; +Cc: kfogel, mehw.is.me, emacs-devel
> > In Lisp comments, objects quoted 'like-this' are not
> > highlighted. Objects quoted `like-this' are. That's why the latter
> > quotation seems to be preferred for Lisp comments, IME.
>
> What is the utility of highlighting quoted symbols in comments?
Uh, they stand out. Lisp comments can talk about
specific functions, vars, etc. Greatly improves
readability, IMO.
E.g., Contrast these comments in dired.el, which quote:
;; Note this can't simply be run inside function `dired-ls' as the hook
;; functions probably depend on the dired-subdir-alist to be OK.
;; We make heavy use of MATCH-ANCHORED, since the regexps don't identify the
;; file name itself. We search for Dired defined regexps, and then use the
;; Dired defined function `dired-move-to-filename' before searching for the
;; simple regexp ".+". It is that regexp which matches the file name.
;; Files suffixed with `completion-ignored-extensions'.
;; If a dialog is used, call `read-directory-name' so the
;; dialog code knows we want directories. Some dialogs
;; can only select directories or files when popped up,
;; not both. If no dialog is used, call `read-file-name'
;; because the user may want completion of file names for
;; use in a wildcard pattern.
;; Do not auto-revert when the dired buffer can be currently
;; written by the user as in `wdired-mode'.
;; Use point difference instead of `current-column', because
;; the former works when `dired-hide-details-mode' is enabled.
;; Always revert when `dir-or-list' is a cons. Also revert
;; if `dired-directory' is a cons but `dir-or-list' is not.
with these comments, in the same file, which don't quote:
;; Inherit from font-lock-comment-delimiter-face since with min-colors 8
;; font-lock-comment-face is not colored any more.
;; If the argument was syntactically a directory name not a file name,
;; or if it happens to name a file that is a directory,
;; convert it syntactically to a directory name.
;; The reason for checking initially-was-dirname
;; and not just file-directory-p
;; is that file-directory-p is slow over ftp.
and with this comment, which both does and doesn't quote:
;; If DIR-OR-LIST is a string and there is an existing dired buffer
;; for it, just leave buffer as it is (don't even call dired-revert).
;; This saves time especially for deep trees or with ange-ftp.
;; The user can type `g' easily, and it is more consistent with find-file.
;; But if SWITCHES are given they are probably different from the
;; buffer's old value, so call dired-sort-other, which does
;; revert the buffer.
;; Revert the buffer if DIR-OR-LIST is a cons or `dired-directory'
;; is a cons and DIR-OR-LIST is a string.
;; A pity we can't possibly do "Directory has changed - refresh? "
;; like find-file does.
;; Optional argument MODE is passed to dired-find-buffer-nocreate,
;; see there.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-25 15:56 ` Michael Albinus
@ 2020-07-25 16:30 ` Eli Zaretskii
2020-07-25 16:33 ` Eli Zaretskii
0 siblings, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2020-07-25 16:30 UTC (permalink / raw)
To: Michael Albinus; +Cc: kfogel, emacs-devel, mehw.is.me
> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: mehw.is.me@inventati.org, kfogel@red-bean.com, emacs-devel@gnu.org
> Date: Sat, 25 Jul 2020 17:56:22 +0200
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> In Lisp comments, objects quoted 'like-this' are not
> >> highlighted. Objects quoted `like-this' are. That's why the latter
> >> quotation seems to be preferred for Lisp comments, IME.
> >
> > What is the utility of highlighting quoted symbols in comments? Do
> > you mean doc strings?
>
> No, I mean Lisp comments. I like highlighted symbols there, for discoverability.
Fine with me, I don't mind such a style.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-25 16:30 ` Eli Zaretskii
@ 2020-07-25 16:33 ` Eli Zaretskii
2020-07-25 16:43 ` Michael Albinus
0 siblings, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2020-07-25 16:33 UTC (permalink / raw)
To: michael.albinus; +Cc: kfogel, mehw.is.me, emacs-devel
> Date: Sat, 25 Jul 2020 19:30:23 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: kfogel@red-bean.com, emacs-devel@gnu.org, mehw.is.me@inventati.org
>
> > No, I mean Lisp comments. I like highlighted symbols there, for discoverability.
>
> Fine with me, I don't mind such a style.
To clarify: I don't mind such a style _in_Lisp_comments_.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-25 16:33 ` Eli Zaretskii
@ 2020-07-25 16:43 ` Michael Albinus
2020-07-25 16:55 ` Eli Zaretskii
0 siblings, 1 reply; 52+ messages in thread
From: Michael Albinus @ 2020-07-25 16:43 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: kfogel, mehw.is.me, emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
>> > No, I mean Lisp comments. I like highlighted symbols there, for
>> > discoverability.
>>
>> Fine with me, I don't mind such a style.
>
> To clarify: I don't mind such a style _in_Lisp_comments_.
Sure. But the original question was how-to-quote. And a quoting
`like-this' in Lisp comments seems to be preferred. Until we add
highlighting for quoting 'like-this' in Lisp comments.
(No, I don't vote for this feature. I just think we shall be consistent
in our recommendations.)
Best regards, Michael.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-25 16:43 ` Michael Albinus
@ 2020-07-25 16:55 ` Eli Zaretskii
2020-07-25 18:28 ` Matthew White
0 siblings, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2020-07-25 16:55 UTC (permalink / raw)
To: Michael Albinus; +Cc: kfogel, mehw.is.me, emacs-devel
> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: kfogel@red-bean.com, emacs-devel@gnu.org, mehw.is.me@inventati.org
> Date: Sat, 25 Jul 2020 18:43:41 +0200
>
> > To clarify: I don't mind such a style _in_Lisp_comments_.
>
> Sure. But the original question was how-to-quote. And a quoting
> `like-this' in Lisp comments seems to be preferred. Until we add
> highlighting for quoting 'like-this' in Lisp comments.
The original question was how to quote in *commit messages*. The part
about comments was my addition.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-25 16:55 ` Eli Zaretskii
@ 2020-07-25 18:28 ` Matthew White
2020-07-25 18:54 ` Eli Zaretskii
0 siblings, 1 reply; 52+ messages in thread
From: Matthew White @ 2020-07-25 18:28 UTC (permalink / raw)
To: emacs-devel; +Cc: Eli Zaretskii, Michael Albinus, Drew Adams
[-- Attachment #1: Type: text/plain, Size: 1557 bytes --]
On Sat, 25 Jul 2020 19:55:16 +0300
Eli Zaretskii wrote:
> > From: Michael Albinus
> > Date: Sat, 25 Jul 2020 18:43:41 +0200
> >
> > > To clarify: I don't mind such a style _in_Lisp_comments_.
> >
> > Sure. But the original question was how-to-quote. And a quoting
> > `like-this' in Lisp comments seems to be preferred. Until we add
> > highlighting for quoting 'like-this' in Lisp comments.
>
> The original question was how to quote in *commit messages*. The part
> about comments was my addition.
Thanks to all, Eli Zaretskii, Michael Albinus, and Drew Adams.
Yes, the question, as Eli pointed out, was about quoting (E)LISP
elements (variables, functions, etc.) in *commit messages*... In
Emacs it's so cool the fontification of comments `like-this'. So
I do have the habit of using the same style in commit messages.
What do you suggest? Is it ok to be consistent using Emacs style
in commit messages when typing Emacs' elements?
There is also ‘this’ kind of quoting in Emacs. Is it just `this'
rendered into `this', or should you write ‘this’ to begin with?
Just to rephrase. In Emacs I write elisp elements `like-this'.
Is it the "right" style or should I write ‘like-this’. Then in
*commit messages* I keep writing elisp elements `like-this' [or
should I write ‘like-this’?], but not non-elisp elements, which
I quote 'like-this' or "like-this"...
Forgive me... I don't want to raise confusion. I hope that you
all may find reasoning in my bad writing "style"... ;)
Thanks!
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-25 18:28 ` Matthew White
@ 2020-07-25 18:54 ` Eli Zaretskii
0 siblings, 0 replies; 52+ messages in thread
From: Eli Zaretskii @ 2020-07-25 18:54 UTC (permalink / raw)
To: Matthew White; +Cc: michael.albinus, drew.adams, emacs-devel
> Date: Sat, 25 Jul 2020 20:28:56 +0200
> From: Matthew White <mehw.is.me@inventati.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, Michael Albinus <michael.albinus@gmx.de>,
> Drew Adams <drew.adams@oracle.com>
>
> What do you suggest? Is it ok to be consistent using Emacs style
> in commit messages when typing Emacs' elements?
There's no need to be "consistent" between Lisp comments and commit
log messages. They are different contexts.
In commit log messages please quote 'like this'.
> There is also ‘this’ kind of quoting in Emacs.
This requires non-ASCII characters, so could produce mojibake on some
less capable displays. So please don't use that style in our log
messages.
Once again, we are talking only about commit log messages. Lisp
comments, let alone doc strings, are another matter.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-25 10:46 ` Matthew White
2020-07-25 10:58 ` Eli Zaretskii
@ 2020-07-25 20:36 ` Karl Fogel
2020-07-25 21:14 ` Stefan Monnier
2020-07-29 22:16 ` Karl Fogel
2 siblings, 1 reply; 52+ messages in thread
From: Karl Fogel @ 2020-07-25 20:36 UTC (permalink / raw)
To: Matthew White; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 5348 bytes --]
On 25 Jul 2020, Matthew White wrote:
>> Hi, Matthew. Thanks so much for this. I have reviewed the patch
>> against 'master' (since it applies cleanly there, whereas it does not
>> quite apply cleanly to 'emacs-27'). It is very good; I have just
>> some minor comments below.
>
>Hi Karl. I'm flattered, thanks!
>
>I appreciate so much that you took the time to review my patch, and
>I eagerly followed your suggestions!
>
>You will find attached both a patch that applies to master, and one
>that applies to emacs-27... They are the same in term of changes.
Thanks! I'll test against both branches, then.
>I'm still waiting a reply to the copyright assignment... But in the
>meantime we can freely discuss about the patch...
>
>> 1) It helps to include a summary line at the top of your commit
>> message, followed by a blank line. The summary line should be
>> limited to 50 characters or fewer, if possible. (This is all
>> documented in the "Commit messages" section in the 'CONTRIBUTE' file
>> in the top level of the Emacs source tree, by the way). In your
>> case, the Subject header of your email almost does the job -- we can
>> shorten it a bit to fit within 50 characters:
>>
>> "Add ability to mark/unmark/delete all bookmarks"
>
>Thanks for pointing that out. Now that I read the 'CONTRIBUTE' file,
>I have some questions (by the way, I needed a remainder to read...):
>
>1) I did commit a message with a summary line, as you described...
>
> [ Actually my line was the subject of the thread, yours (above)
> is better, hence I switched to your idea, thanks again ;) ]
>
> And indeed I use `git format-patch -1` as suggested in 'CONTRIBUTE'
> to produce a patch, but the command "strips" the summary line from
> the commit message and uses it as 'Subject:' in the patch, where it
> is prefixed with '[PATCH] ' if the option -k isn't also given...
>
> Once the patch is applied via `git am`, the 'Subject:' is put back
> in the commit message as summary, without the '[PATCH] ' prefix.
>
> So, I wonder if you used `git am` to apply the patch...? Could you
> tip me off, please?
I didn't use 'git am' the previous time, but I will this time.
I wish that the output produced by 'git format-patch' were both a little more self-identifying and more self-explanatory. While I recognized the patch as such output, I forgot that the Subject line therefore would contain the first line of the commit message :-). Had I reviewed the commit message after application, I would have seen that line appear, of course. Anyway, you did the right thing here -- I just need to adjust my own process a bit.
>2) With which quotes should I surround the variable/function names in
> the commit message? `' or '' or ‘’? I could not figure that out...
I saw the followup discussion about this, with the answer being 'like so'.
>[x] Don't repeat the file name each time in the commit message.
Great.
> ...And yes, I did take a look at the `git log`...
Heh -- sorry there just happened to be a misleading commit message in the recent history, yeah.
>> Better to call ARG something like NO-CONFIRM, so its name reflects
>> its purpose.
>>
>> And a suggestion: have the prompt say "Permanently delete all
>> bookmarks? " instead.
>
>In `bookmark-delete-all':
>
>[x] Chage ARG to NO-CONFIRM.
>[x] Prompt "Permanently delete all bookmarks? ".
Thanks.
>> Because of subtle assumptions associated with English word order, the
>> current prompt ("Delete all bookmarks permanently? ") implies that
>> the alternative might be to delete the bookmarks non-permanently.
>
>That's a very smart thought... I'm truly impressed!
Well, I used to be an English teacher :-).
>> Regarding the "FIXME" comment: that assumption is embedded throughout
>> the bookmark-bmenu-* code, so I think there's no need for the "FIXME"
>> (and indeed the comment may cause confusion, since its presence calls
>> the assumption into question).
>>
>> It might be good to make a followup change in which we document the
>> last-line-is-empty assumption, but that's a separate change of course.
>
>[x] Remove the "FIXME" code comments.
Thanks.
>Following your guidelines is very instructing, Karl!
It's a pleasure to review a well-done patch!
>I also:
>
>[x] Change `nameN' and `fileN' into `name-N' and `file-N' (i.e. name0
> -> name-0) in all new functions and in the new test-list.bmk file.
>[x] Indent new functions to split long lines into shorter ones.
>[x] bookmark-tests.el (bookmark-test-bmenu-any-marks-list): New function
> to test `bookmark-bmenu-any-marks' with a list of bookmarks.
*nod* Got it; will take a look at those changes.
>I hope to get back soon with good news about the copyright assignment.
Sounds good.
On both branches, I'll apply the corresponding new changesets, run 'make check', and test manually. I'll also look over the diffs again. I'll follow up here after I've done so.
I'm actually not sure yet which branch we should apply on -- I think 'emacs-27' is the right answer, as per CONTRIBUTE, but if anyone knows better please say so.
(I would make sure to use 'git am' to apply, to preserve attribution and history information, of course.)
Best regards,
-Karl
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-25 20:36 ` Karl Fogel
@ 2020-07-25 21:14 ` Stefan Monnier
2020-07-25 22:40 ` Karl Fogel
0 siblings, 1 reply; 52+ messages in thread
From: Stefan Monnier @ 2020-07-25 21:14 UTC (permalink / raw)
To: Karl Fogel; +Cc: emacs-devel, Matthew White
> I'm actually not sure yet which branch we should apply on -- I think
> 'emacs-27' is the right answer, as per CONTRIBUTE, but if anyone knows
> better please say so.
Definitely not `emacs-27` since that one is in hard-freeze until the
Emacs-27.1 release (i.e. only apply patches there which have been
explicitly allowed by the high command (i.e., Eli) because they fix
known bugs in an obviously safe way; with less stringent conditions when
the patch only changes the docs).
Stefan
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-25 21:14 ` Stefan Monnier
@ 2020-07-25 22:40 ` Karl Fogel
0 siblings, 0 replies; 52+ messages in thread
From: Karl Fogel @ 2020-07-25 22:40 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel, Matthew White
On 25 Jul 2020, Stefan Monnier wrote:
>> I'm actually not sure yet which branch we should apply on -- I think
>> 'emacs-27' is the right answer, as per CONTRIBUTE, but if anyone knows
>> better please say so.
>
>Definitely not `emacs-27` since that one is in hard-freeze until the
>Emacs-27.1 release (i.e. only apply patches there which have been
>explicitly allowed by the high command (i.e., Eli) because they fix
>known bugs in an obviously safe way; with less stringent conditions when
>the patch only changes the docs).
Thanks, Stefan. I'll use 'master' then.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Extend tabulated-list-mode to support marks
2020-07-24 19:00 ` Extend tabulated-list-mode to support marks Yuri Khan
2020-07-25 13:46 ` Dirk-Jan C. Binnema
2020-07-25 14:23 ` Stefan Monnier
@ 2020-07-26 12:43 ` Stefan Kangas
2 siblings, 0 replies; 52+ messages in thread
From: Stefan Kangas @ 2020-07-26 12:43 UTC (permalink / raw)
To: Yuri Khan; +Cc: Matthew White, Stefan Monnier, Emacs developers
Yuri Khan <yuri.v.khan@gmail.com> writes:
> One high-visible derived mode hijacks the “unmark all”
> key for “mark upgradable packages”. I think they would benefit from a
> common implementation and the unification of the UI stemming from
> that.
Indeed, I think that is an unfortunate and surprising default that is
at odds with the rest of Emacs (notably Dired). (I'd be in favour of
changing it, FWIW.)
> I might implement something if this general direction is deemed
> desirable. Details can be discussed.
I've had the same idea in the past and would definitely appreciate
such an addition.
Best regards,
Stefan Kangas
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-24 17:09 ` Stefan Monnier
@ 2020-07-28 15:54 ` Matthew White
2020-07-28 16:14 ` Stefan Monnier
2020-07-28 18:06 ` Eli Zaretskii
0 siblings, 2 replies; 52+ messages in thread
From: Matthew White @ 2020-07-28 15:54 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Lars Ingebrigtsen, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 2042 bytes --]
On Fri, 24 Jul 2020 13:09:18 -0400
Stefan Monnier wrote:
> > Can you, or some dev, send me the necessary instruction and
> > assignment form, please?
>
> Please fill the form below and then send it as instructed to the FSF
> so they can send you the relevant paperwork to sign,
>
>
> Stefan
>
>
> Please email the following information to assign@gnu.org, and we
> will send you the assignment form for your past and future changes.
>
> Please use your full legal name (in ASCII characters) as the subject
> line of the message.
> ----------------------------------------------------------------------
> REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES
>
> [What is the name of the program or package you're contributing to?]
> Emacs
>
> [Did you copy any files or text written by someone else in these
> changes? Even if that material is free software, we need to know
> about it.]
>
>
> [Do you have an employer who might have a basis to claim to own
> your changes? Do you attend a school which might make such a claim?]
>
>
> [For the copyright registration, what country are you a citizen of?]
>
>
> [What year were you born?]
>
>
> [Please write your email address here.]
>
>
> [Please write your postal address here.]
>
>
>
>
>
> [Which files have you changed so far, and which new files have you
> written so far?]
>
Hi Stefan,
I compiled and sent the module on the 2020-07-24, but I still got no
reply... I also checked the spam...
I sent a signed email, as Subject I used my legal name, and the body
starts with
> REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES
>
I wrote each answer below the question in square brackets leaving an
empty line above and one below
> [What is the name of the program or package you're contributing to?]
>
> Emacs
>
> [...]
Do you think they at assign@gnu.org are overwhelmed in this period...
or maybe I made a mistake compiling the form...?
What do you suggest?
Thanks,
Matthew
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-28 15:54 ` Matthew White
@ 2020-07-28 16:14 ` Stefan Monnier
2020-07-28 16:36 ` Matthew White
2020-07-28 18:06 ` Eli Zaretskii
1 sibling, 1 reply; 52+ messages in thread
From: Stefan Monnier @ 2020-07-28 16:14 UTC (permalink / raw)
To: Matthew White; +Cc: Lars Ingebrigtsen, emacs-devel
> I compiled and sent the module on the 2020-07-24, but I still got no
> reply... I also checked the spam...
[ Not sure what you mean by "compiled" and "module" since we're talking
about human-processed text here. ]
> I sent a signed email, as Subject I used my legal name, and the body
> starts with
[...]
> Do you think they at assign@gnu.org are overwhelmed in this period...
> or maybe I made a mistake compiling the form...?
It might be a that they're a bit overwhelmed, tho 4 days to reply to
this seems more than what I've seen in the past.
As for a mistake in filling the form, that doesn't sound likely: the
copyright clerk (usually Donald) would have contacted you asking
about it.
Can you try sending a followup-email asking for the status of your
request (and put me in Cc)?
Stefan
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-28 16:14 ` Stefan Monnier
@ 2020-07-28 16:36 ` Matthew White
0 siblings, 0 replies; 52+ messages in thread
From: Matthew White @ 2020-07-28 16:36 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Lars Ingebrigtsen, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 510 bytes --]
On Tue, 28 Jul 2020 12:14:21 -0400
Stefan Monnier wrote:
> > I compiled and sent the module on the 2020-07-24, but I still got no
> > reply... I also checked the spam...
>
> [ Not sure what you mean by "compiled" and "module" since we're
> talking about human-processed text here. ]
>
Funny... ;) ...maybe it's the effect of writing code...
> Can you try sending a followup-email asking for the status of your
> request (and put me in Cc)?
>
>
> Stefan
>
Done,
Matthew
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-28 15:54 ` Matthew White
2020-07-28 16:14 ` Stefan Monnier
@ 2020-07-28 18:06 ` Eli Zaretskii
2020-07-28 18:15 ` Matthew White
1 sibling, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2020-07-28 18:06 UTC (permalink / raw)
To: Matthew White; +Cc: larsi, monnier, emacs-devel
> Date: Tue, 28 Jul 2020 17:54:24 +0200
> From: Matthew White <mehw.is.me@inventati.org>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, emacs-devel@gnu.org
>
> I compiled and sent the module on the 2020-07-24, but I still got no
> reply... I also checked the spam...
If they don't reply in a week, ping them and CC me.
Thanks.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-28 18:06 ` Eli Zaretskii
@ 2020-07-28 18:15 ` Matthew White
0 siblings, 0 replies; 52+ messages in thread
From: Matthew White @ 2020-07-28 18:15 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: larsi, monnier, emacs-devel
> If they don't reply in a week, ping them and CC me.
>
> Thanks.
I will, thank you.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-25 10:46 ` Matthew White
2020-07-25 10:58 ` Eli Zaretskii
2020-07-25 20:36 ` Karl Fogel
@ 2020-07-29 22:16 ` Karl Fogel
2020-07-31 2:58 ` Matthew White
2 siblings, 1 reply; 52+ messages in thread
From: Karl Fogel @ 2020-07-29 22:16 UTC (permalink / raw)
To: Matthew White; +Cc: emacs-devel
[-- Attachment #1.1: Type: text/plain, Size: 5045 bytes --]
Hi, Matthew. I had a chance to try out your latest patch for 'master'. Note I only tried that one, since this change will go onto 'master' and not onto 'emacs-27'.
When I tested manually, everything works fine: marking all, unmarking all, deleting, everything's good. However, bookmark shows one test failure now in 'make check':
FAILED 2/47 bookmark-test-bmenu-any-marks-list (0.000279 sec)
I haven't had time to investigate further yet, but I wanted to know if you're also seeing the same failure? I applied your patch using 'git am'. Below are HEAD and HEAD^ in my tree, so you can see exactly what code I'm testing with:
> commit 23faf0e8d4daac5cc9c199efe97fa36c9d4691b6
> Author: Matthew White <mehw.is.me@inventati.org>
> AuthorDate: Thu Jul 23 21:14:32 2020 +0000
> Commit: Karl Fogel <kfogel@red-bean.com>
> CommitDate: Tue Jul 28 21:10:45 2020 -0500
>
> Add ability to mark/unmark/delete all bookmarks
>
> * lisp/bookmark.el (bookmark-delete-all): New function to delete all
> bookmarks.
> (bookmark-bmenu-mark-all): New function to mark all bookmarks in the
> bookmark list buffer.
> (bookmark-bmenu-unmark-all): New function to unmark all bookmarks in
> the bookmark list buffer.
> (bookmark-bmenu-delete-all): New function to mark for deletion all
> bookmarks in the bookmark list buffer.
> (bookmark-map): Map "D" to `bookmark-delete-all'.
> (bookmark-bmenu-mode-map): New mappping for "M" to
> `bookmark-bmenu-mark-all'.
> (bookmark-bmenu-mode-map): New mappping for "U" to
> `bookmark-bmenu-unmark-all'.
> (bookmark-bmenu-mode-map): New mappping for "D" to
> `bookmark-bmenu-delete-all'.
> (bookmark-bmenu-mark-all): New bookmark menu to
> `bookmark-delete-all'.
> (easy-menu-define): New bookmark menu to `bookmark-bmenu-mark-all'.
> (easy-menu-define): New bookmark menu to
> `bookmark-bmenu-unmark-all'.
> (easy-menu-define): New bookmark menu to
> `bookmark-bmenu-delete-all'.
> (bookmark-bmenu-select): Update docstring to include a reference to
> `bookmark-bmenu-mark-all'.
> (bookmark-bmenu-mode): Update docstring. Add/Update description:
> `bookmark-bmenu-mark-all', `bookmark-bmenu-delete-all',
> `bookmark-bmenu-execute-deletions', and `bookmark-bmenu-unmark-all'.
> * test/lisp/bookmark-resources/test-list.bmk: New bookmark file to
> test a list of bookmarks.
> * test/lisp/bookmark-tests.el (bookmark-tests-bookmark-file-list): New
> reference to the bookmark file used for testing a list of bookmarks.
> (bookmark-tests-bookmark-list-0, bookmark-tests-bookmark-list-1,
> bookmark-tests-bookmark-list-2): New cached values for testing a
> list of bookmark.
> (bookmark-tests-cache-timestamp-list): New variable to set
> `bookmark-bookmarks-timestamp'.
> (with-bookmark-test-list): New macro environment to test a list of
> bookmarks.
> (with-bookmark-test-file-list): New macro environment to test a list
> of bookmarks with example.txt.
> (with-bookmark-bmenu-test-list): New macro environment to test
> functions about a list of bookmarks from `bookmark-bmenu-list'.
> (bookmark-tests-all-names-list, bookmark-tests-get-bookmark-list,
> bookmark-tests-get-bookmark-record-list): New functions to test the
> records of the list of bookmarks.
> (bookmark-tests-make-record-list): New function to test the creation
> of a record from example.txt with a list of bookmarks loaded.
> (bookmark-tests-delete-all): New function to test
> `bookmark-delete-all'.
> (bookmark-test-bmenu-any-marks-list): New function to test
> `bookmark-bmenu-any-marks' with a list of bookmarks.
> (bookmark-test-bmenu-mark-all): New function to test
> `bookmark-bmenu-mark-all'.
> (bookmark-test-bmenu-unmark-all): New function to test
> `bookmark-bmenu-unmark-all'.
> (bookmark-test-bmenu-delete-all): New function to test
> `bookmark-bmenu-delete-all'.
>
> M lisp/bookmark.el
> A test/lisp/bookmark-resources/test-list.bmk
> M test/lisp/bookmark-tests.el
>
> commit eb9e065c00c5590967255fa3abf51db966a5df72
> Author: Juri Linkov <juri@linkov.net>
> AuthorDate: Wed Jul 29 03:47:29 2020 +0300
> Commit: Juri Linkov <juri@linkov.net>
> CommitDate: Wed Jul 29 03:47:29 2020 +0300
>
> * lisp/vc/vc-git.el (vc-git-log-view-mode): Font-lock AuthorDate (bug#40248)
>
> Highlight "AuthorDate" in log-view-font-lock-keywords
> when [format] pretty = fuller.
>
> M lisp/vc/vc-git.el
Attached is the full 'test/lisp/bookmark-tests.log' file after running 'make check'.
Best regards,
-Karl
[-- Attachment #1.2: test/lisp/bookmark-tests.log (after 'make check' has completed) --]
[-- Type: text/plain, Size: 5092 bytes --]
Running 47 tests (2020-07-29 17:06:19-0500, selector `(not (or (tag :expensive-test) (tag :unstable)))')
passed 1/47 bookmark-test-bmenu-any-marks (0.000287 sec)
Test bookmark-test-bmenu-any-marks-list backtrace:
signal(ert-test-failed (((should (looking-at "^> ")) :form (looking-
ert-fail(((should (looking-at "^> ")) :form (looking-at "^> ") :valu
#f(compiled-function () #<bytecode -0x1e9c933e7640da13>)()
ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
ert-run-test(#s(ert-test :name bookmark-test-bmenu-any-marks-list :d
ert-run-or-rerun-test(#s(ert--stats :selector (not ...) :tests [...
ert-run-tests((not (or (tag :expensive-test) (tag :unstable))) #f(co
ert-run-tests-batch((not (or (tag :expensive-test) (tag :unstable)))
ert-run-tests-batch-and-exit((not (or (tag :expensive-test) (tag :un
eval((ert-run-tests-batch-and-exit '(not (or (tag :expensive-test) (
command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/bookmark-tests" "--e
command-line()
normal-top-level()
Test bookmark-test-bmenu-any-marks-list condition:
(ert-test-failed
((should
(looking-at "^> "))
:form
(looking-at "^> ")
:value nil))
FAILED 2/47 bookmark-test-bmenu-any-marks-list (0.000279 sec)
passed 3/47 bookmark-test-bmenu-bookmark (0.000219 sec)
Deleting bookmarks... \
Deleting bookmarks...done
passed 4/47 bookmark-test-bmenu-delete (0.000237 sec)
Deleting bookmarks... \
Deleting bookmarks...done
Deleting bookmarks... \
Deleting bookmarks...done
passed 5/47 bookmark-test-bmenu-delete-all (0.000508 sec)
passed 6/47 bookmark-test-bmenu-edit-annotation/show-annotation (0.000286 sec)
passed 7/47 bookmark-test-bmenu-filter-alist-by-regexp (0.000197 sec)
passed 8/47 bookmark-test-bmenu-hide-filenames (0.000171 sec)
passed 9/47 bookmark-test-bmenu-locate (0.000181 sec)
passed 10/47 bookmark-test-bmenu-mark (0.000159 sec)
passed 11/47 bookmark-test-bmenu-mark-all (0.000237 sec)
Annotation updated for "name"
passed 12/47 bookmark-test-bmenu-send-edited-annotation (0.000480 sec)
Annotation updated for "name"
passed 13/47 bookmark-test-bmenu-send-edited-annotation/restore-focus (0.000472 sec)
passed 14/47 bookmark-test-bmenu-show-filenames (0.000161 sec)
passed 15/47 bookmark-test-bmenu-toggle-filenames (0.000174 sec)
passed 16/47 bookmark-test-bmenu-toggle-filenames/show (0.000155 sec)
passed 17/47 bookmark-test-bmenu-unmark (0.000171 sec)
passed 18/47 bookmark-test-bmenu-unmark-all (0.000254 sec)
passed 19/47 bookmark-tests-all-names (0.000065 sec)
passed 20/47 bookmark-tests-all-names-list (0.000062 sec)
passed 21/47 bookmark-tests-default-annotation-text (0.000058 sec)
passed 22/47 bookmark-tests-delete (0.000063 sec)
passed 23/47 bookmark-tests-delete-all (0.000060 sec)
Annotation updated for "name"
passed 24/47 bookmark-tests-edit-annotation (0.000198 sec)
passed 25/47 bookmark-tests-get-bookmark (0.000051 sec)
passed 26/47 bookmark-tests-get-bookmark-list (0.000052 sec)
passed 27/47 bookmark-tests-get-bookmark-record (0.000047 sec)
passed 28/47 bookmark-tests-get-bookmark-record-list (0.000049 sec)
Mark set
passed 29/47 bookmark-tests-insert (0.054729 sec)
passed 30/47 bookmark-tests-insert-annotation (0.000110 sec)
passed 31/47 bookmark-tests-insert-location (0.000063 sec)
passed 32/47 bookmark-tests-jump (0.037029 sec)
passed 33/47 bookmark-tests-kill-line (0.000073 sec)
passed 34/47 bookmark-tests-load (0.108157 sec)
passed 35/47 bookmark-tests-location (0.000074 sec)
passed 36/47 bookmark-tests-make-record (0.033908 sec)
passed 37/47 bookmark-tests-make-record-function (0.008674 sec)
passed 38/47 bookmark-tests-make-record-list (0.036535 sec)
passed 39/47 bookmark-tests-maybe-historicize-string (0.000048 sec)
passed 40/47 bookmark-tests-maybe-rename (0.000046 sec)
passed 41/47 bookmark-tests-record-getters-and-setters-new (0.000076 sec)
passed 42/47 bookmark-tests-rename (0.000080 sec)
Saving bookmarks to file /tmp/bookmark-tests-nWOh5y... \
Saving bookmarks to file /tmp/bookmark-tests-nWOh5y...done
passed 43/47 bookmark-tests-save (0.001546 sec)
Saving bookmarks to file /tmp/bookmark-tests-ta0qZH... \
Saving bookmarks to file /tmp/bookmark-tests-ta0qZH...done
passed 44/47 bookmark-tests-save/non-ascii-annotation (0.044920 sec)
Saving bookmarks to file /tmp/bookmark-tests-Y7mZuJ... \
Saving bookmarks to file /tmp/bookmark-tests-Y7mZuJ...done
passed 45/47 bookmark-tests-save/non-ascii-bookmark-name (0.001665 sec)
passed 46/47 bookmark-tests-set (0.036700 sec)
Annotation updated for "foo"
passed 47/47 bookmark-tests-set/bookmark-use-annotations-t (0.036375 sec)
Ran 47 tests, 46 results as expected, 1 unexpected (2020-07-29 17:06:20-0500, 0.482341 sec)
1 unexpected results:
FAILED bookmark-test-bmenu-any-marks-list
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-29 22:16 ` Karl Fogel
@ 2020-07-31 2:58 ` Matthew White
2020-07-31 6:01 ` Karl Fogel
2020-08-02 22:13 ` Karl Fogel
0 siblings, 2 replies; 52+ messages in thread
From: Matthew White @ 2020-07-31 2:58 UTC (permalink / raw)
To: Karl Fogel; +Cc: emacs-devel
[-- Attachment #1.1: Type: text/plain, Size: 1655 bytes --]
On Wed, 29 Jul 2020 17:16:09 -0500
Karl Fogel wrote:
> Hi, Matthew. I had a chance to try out your latest patch for
> 'master'. Note I only tried that one, since this change will go onto
> 'master' and not onto 'emacs-27'.
>
> When I tested manually, everything works fine: marking all, unmarking
> all, deleting, everything's good. However, bookmark shows one test
> failure now in 'make check':
>
> FAILED 2/47 bookmark-test-bmenu-any-marks-list (0.000279 sec)
>
> I haven't had time to investigate further yet, but I wanted to know
> if you're also seeing the same failure? I applied your patch using
> 'git am'.
>
> ...
>
> Attached is the full 'test/lisp/bookmark-tests.log' file after
> running 'make check'.
>
> Best regards,
> -Karl
>
Hi, Karl. You got me...! Thanks for taking the time to test the patch
again. I admit, I was lazy and didn't test the amended patch with the
new function `bookmark-test-bmenu-any-marks-list' I added... and this
is the result...
> Test bookmark-test-bmenu-any-marks-list condition:
> (ert-test-failed
> ((should
> (looking-at "^> "))
> :form
> (looking-at "^> ")
> :value nil))
> FAILED 2/47 bookmark-test-bmenu-any-marks-list (0.000279 sec)
Yes, I have the same failure as yours...
There was one extra 'looking-at "^> "' (changed to 'looking-at "^ "').
I attach revised patches (master and emacs-27, with the latter just in
case someone wants to test it on emacs-27).
PS: I finally got the PDF copyright assignment... I signed and emailed
it back... now I'm waiting for the FSF signature.
Cheers!
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: fixed-master-Add-ability-to-mark-unmark-delete-all-bookmarks.patch --]
[-- Type: text/x-patch, Size: 24349 bytes --]
From 48cd4b7de01600d54d2db9deac5d16d1518a2ec5 Mon Sep 17 00:00:00 2001
From: Matthew White <mehw.is.me@inventati.org>
Date: Thu, 23 Jul 2020 21:14:32 +0000
Subject: [PATCH] Add ability to mark/unmark/delete all bookmarks
* lisp/bookmark.el (bookmark-delete-all): New function to delete all
bookmarks.
(bookmark-bmenu-mark-all): New function to mark all bookmarks in the
bookmark list buffer.
(bookmark-bmenu-unmark-all): New function to unmark all bookmarks in
the bookmark list buffer.
(bookmark-bmenu-delete-all): New function to mark for deletion all
bookmarks in the bookmark list buffer.
(bookmark-map): Map "D" to `bookmark-delete-all'.
(bookmark-bmenu-mode-map): New mappping for "M" to
`bookmark-bmenu-mark-all'.
(bookmark-bmenu-mode-map): New mappping for "U" to
`bookmark-bmenu-unmark-all'.
(bookmark-bmenu-mode-map): New mappping for "D" to
`bookmark-bmenu-delete-all'.
(bookmark-bmenu-mark-all): New bookmark menu to
`bookmark-delete-all'.
(easy-menu-define): New bookmark menu to `bookmark-bmenu-mark-all'.
(easy-menu-define): New bookmark menu to
`bookmark-bmenu-unmark-all'.
(easy-menu-define): New bookmark menu to
`bookmark-bmenu-delete-all'.
(bookmark-bmenu-select): Update docstring to include a reference to
`bookmark-bmenu-mark-all'.
(bookmark-bmenu-mode): Update docstring. Add/Update description:
`bookmark-bmenu-mark-all', `bookmark-bmenu-delete-all',
`bookmark-bmenu-execute-deletions', and `bookmark-bmenu-unmark-all'.
* test/lisp/bookmark-resources/test-list.bmk: New bookmark file to
test a list of bookmarks.
* test/lisp/bookmark-tests.el (bookmark-tests-bookmark-file-list): New
reference to the bookmark file used for testing a list of bookmarks.
(bookmark-tests-bookmark-list-0, bookmark-tests-bookmark-list-1,
bookmark-tests-bookmark-list-2): New cached values for testing a
list of bookmark.
(bookmark-tests-cache-timestamp-list): New variable to set
`bookmark-bookmarks-timestamp'.
(with-bookmark-test-list): New macro environment to test a list of
bookmarks.
(with-bookmark-test-file-list): New macro environment to test a list
of bookmarks with example.txt.
(with-bookmark-bmenu-test-list): New macro environment to test
functions about a list of bookmarks from `bookmark-bmenu-list'.
(bookmark-tests-all-names-list, bookmark-tests-get-bookmark-list,
bookmark-tests-get-bookmark-record-list): New functions to test the
records of the list of bookmarks.
(bookmark-tests-make-record-list): New function to test the creation
of a record from example.txt with a list of bookmarks loaded.
(bookmark-tests-delete-all): New function to test
`bookmark-delete-all'.
(bookmark-test-bmenu-any-marks-list): New function to test
`bookmark-bmenu-any-marks' with a list of bookmarks.
(bookmark-test-bmenu-mark-all): New function to test
`bookmark-bmenu-mark-all'.
(bookmark-test-bmenu-unmark-all): New function to test
`bookmark-bmenu-unmark-all'.
(bookmark-test-bmenu-delete-all): New function to test
`bookmark-bmenu-delete-all'.
---
lisp/bookmark.el | 77 +++++++-
test/lisp/bookmark-resources/test-list.bmk | 20 ++
test/lisp/bookmark-tests.el | 215 +++++++++++++++++++++
3 files changed, 310 insertions(+), 2 deletions(-)
create mode 100644 test/lisp/bookmark-resources/test-list.bmk
diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index de7d60f97e..870c41d5a8 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -200,6 +200,7 @@ A non-nil value may result in truncated bookmark names."
(define-key map "f" 'bookmark-insert-location) ;"f"ind
(define-key map "r" 'bookmark-rename)
(define-key map "d" 'bookmark-delete)
+ (define-key map "D" 'bookmark-delete-all)
(define-key map "l" 'bookmark-load)
(define-key map "w" 'bookmark-write)
(define-key map "s" 'bookmark-save)
@@ -1374,6 +1375,22 @@ probably because we were called from there."
(bookmark-save)))
+;;;###autoload
+(defun bookmark-delete-all (&optional no-confirm)
+ "Permanently delete all bookmarks.
+Doesn't ask for confirmation if NO-CONFIRM is non-nil."
+ (interactive "P")
+ (when (or no-confirm
+ (yes-or-no-p "Permanently delete all bookmarks? "))
+ (bookmark-maybe-load-default-file)
+ (setq bookmark-alist-modification-count
+ (+ bookmark-alist-modification-count (length bookmark-alist)))
+ (setq bookmark-alist nil)
+ (bookmark-bmenu-surreptitiously-rebuild-list)
+ (when (bookmark-time-to-save-p)
+ (bookmark-save))))
+
+
(defun bookmark-time-to-save-p (&optional final-time)
"Return t if it is time to save bookmarks to disk, nil otherwise.
Optional argument FINAL-TIME means this is being called when Emacs
@@ -1600,12 +1617,15 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
(define-key map "\C-d" 'bookmark-bmenu-delete-backwards)
(define-key map "x" 'bookmark-bmenu-execute-deletions)
(define-key map "d" 'bookmark-bmenu-delete)
+ (define-key map "D" 'bookmark-bmenu-delete-all)
(define-key map " " 'next-line)
(define-key map "n" 'next-line)
(define-key map "p" 'previous-line)
(define-key map "\177" 'bookmark-bmenu-backup-unmark)
(define-key map "u" 'bookmark-bmenu-unmark)
+ (define-key map "U" 'bookmark-bmenu-unmark-all)
(define-key map "m" 'bookmark-bmenu-mark)
+ (define-key map "M" 'bookmark-bmenu-mark-all)
(define-key map "l" 'bookmark-bmenu-load)
(define-key map "r" 'bookmark-bmenu-rename)
(define-key map "R" 'bookmark-bmenu-relocate)
@@ -1627,8 +1647,10 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
["Select Marked Bookmarks" bookmark-bmenu-select t]
"---"
["Mark Bookmark" bookmark-bmenu-mark t]
+ ["Mark all Bookmarks" bookmark-bmenu-mark-all t]
["Unmark Bookmark" bookmark-bmenu-unmark t]
["Unmark Backwards" bookmark-bmenu-backup-unmark t]
+ ["Unmark all Bookmarks" bookmark-bmenu-unmark-all t]
["Toggle Display of Filenames" bookmark-bmenu-toggle-filenames t]
["Display Location of Bookmark" bookmark-bmenu-locate t]
"---"
@@ -1636,6 +1658,7 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
["Rename Bookmark" bookmark-bmenu-rename t]
["Relocate Bookmark's File" bookmark-bmenu-relocate t]
["Mark Bookmark for Deletion" bookmark-bmenu-delete t]
+ ["Mark all Bookmarks for Deletion" bookmark-bmenu-delete-all t]
["Delete Marked Bookmarks" bookmark-bmenu-execute-deletions t])
("Annotations"
["Show Annotation for Current Bookmark" bookmark-bmenu-show-annotation t]
@@ -1748,6 +1771,7 @@ Letters do not insert themselves; instead, they are commands.
Bookmark names preceded by a \"*\" have annotations.
\\<bookmark-bmenu-mode-map>
\\[bookmark-bmenu-mark] -- mark bookmark to be displayed.
+\\[bookmark-bmenu-mark-all] -- mark all listed bookmarks to be displayed.
\\[bookmark-bmenu-select] -- select bookmark of line point is on.
Also show bookmarks marked using m in other windows.
\\[bookmark-bmenu-toggle-filenames] -- toggle displaying of filenames (they may obscure long bookmark names).
@@ -1764,13 +1788,15 @@ Bookmark names preceded by a \"*\" have annotations.
\\[bookmark-bmenu-relocate] -- relocate this bookmark's file (prompts for new file).
\\[bookmark-bmenu-delete] -- mark this bookmark to be deleted, and move down.
\\[bookmark-bmenu-delete-backwards] -- mark this bookmark to be deleted, and move up.
-\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]'.
+\\[bookmark-bmenu-delete-all] -- mark all listed bookmarks as to be deleted.
+\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]' or `\\[bookmark-bmenu-delete-all]'.
\\[bookmark-bmenu-save] -- save the current bookmark list in the default file.
With a prefix arg, prompts for a file to save in.
\\[bookmark-bmenu-load] -- load in a file of bookmarks (prompts for file.)
\\[bookmark-bmenu-unmark] -- remove all kinds of marks from current line.
With prefix argument, also move up one line.
\\[bookmark-bmenu-backup-unmark] -- back up a line and remove marks.
+\\[bookmark-bmenu-unmark-all] -- remove all kinds of marks from all listed bookmarks.
\\[bookmark-bmenu-show-annotation] -- show the annotation, if it exists, for the current bookmark
in another buffer.
\\[bookmark-bmenu-show-all-annotations] -- show the annotations of all bookmarks in another buffer.
@@ -1937,9 +1963,23 @@ If the annotation does not exist, do nothing."
(bookmark-bmenu-ensure-position))))
+(defun bookmark-bmenu-mark-all ()
+ "Mark all listed bookmarks to be displayed by \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-select]."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ (while (not (eobp))
+ (delete-char 1)
+ (insert ?>)
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-select ()
"Select this line's bookmark; also display bookmarks marked with `>'.
-You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] command."
+You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] or \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark-all] commands."
(interactive)
(let ((bmrk (bookmark-bmenu-bookmark))
(menu (current-buffer))
@@ -2108,6 +2148,20 @@ Optional BACKUP means move up."
(bookmark-bmenu-ensure-position))
+(defun bookmark-bmenu-unmark-all ()
+ "Cancel all requested operations on all listed bookmarks."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ (while (not (eobp))
+ (delete-char 1)
+ (insert " ")
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-delete ()
"Mark bookmark on this line to be deleted.
To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
@@ -2133,6 +2187,22 @@ To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\
(bookmark-bmenu-ensure-position))
+(defun bookmark-bmenu-delete-all ()
+ "Mark all listed bookmarks as to be deleted.
+To remove all deletion marks, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-unmark-all].
+To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ (while (not (eobp))
+ (delete-char 1)
+ (insert ?D)
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-execute-deletions ()
"Delete bookmarks flagged `D'."
(interactive)
@@ -2292,6 +2362,9 @@ strings returned are not."
(bindings--define-key map [delete]
'(menu-item "Delete Bookmark..." bookmark-delete
:help "Delete a bookmark from the bookmark list"))
+ (bindings--define-key map [delete-all]
+ '(menu-item "Delete all Bookmarks..." bookmark-delete-all
+ :help "Delete all bookmarks from the bookmark list"))
(bindings--define-key map [rename]
'(menu-item "Rename Bookmark..." bookmark-rename
:help "Change the name of a bookmark"))
diff --git a/test/lisp/bookmark-resources/test-list.bmk b/test/lisp/bookmark-resources/test-list.bmk
new file mode 100644
index 0000000000..696d64979b
--- /dev/null
+++ b/test/lisp/bookmark-resources/test-list.bmk
@@ -0,0 +1,20 @@
+;;;; Emacs Bookmark Format Version 1 ;;;; -*- coding: utf-8-emacs -*-
+;;; This format is meant to be slightly human-readable;
+;;; nevertheless, you probably don't want to edit it.
+;;; -*- End Of Bookmark File Format Version Stamp -*-
+(("name-0"
+ (filename . "/some/file-0")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+("name-1"
+ (filename . "/some/file-1")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+("name-2"
+ (filename . "/some/file-2")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+)
diff --git a/test/lisp/bookmark-tests.el b/test/lisp/bookmark-tests.el
index b9c6ff9c54..c5959e46d8 100644
--- a/test/lisp/bookmark-tests.el
+++ b/test/lisp/bookmark-tests.el
@@ -83,6 +83,70 @@ the lexically-bound variable `buffer'."
,@body)
(kill-buffer buffer))))
+(defvar bookmark-tests-bookmark-file-list
+ (expand-file-name "test-list.bmk" bookmark-tests-data-dir)
+ "Bookmark file used for testing a list of bookmarks.")
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-0 '("name-0"
+ (filename . "/some/file-0")
+ (front-context-string . "ghi")
+ (rear-context-string . "jkl")
+ (position . 4))
+ "Cached value used in bookmark-tests.el."))
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-1 '("name-1"
+ (filename . "/some/file-1")
+ (front-context-string . "mno")
+ (rear-context-string . "pqr")
+ (position . 5))
+ "Cached value used in bookmark-tests.el."))
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-2 '("name-2"
+ (filename . "/some/file-2")
+ (front-context-string . "stu")
+ (rear-context-string . "vwx")
+ (position . 6))
+ "Cached value used in bookmark-tests.el."))
+
+(defvar bookmark-tests-cache-timestamp-list
+ (cons bookmark-tests-bookmark-file-list
+ (nth 5 (file-attributes
+ bookmark-tests-bookmark-file-list)))
+ "Cached value used in bookmark-tests.el.")
+
+(defmacro with-bookmark-test-list (&rest body)
+ "Create environment for testing bookmark.el and evaluate BODY.
+Ensure a clean environment for testing, and do not change user
+data when running tests interactively."
+ `(with-temp-buffer
+ (let ((bookmark-alist (quote (,(copy-sequence bookmark-tests-bookmark-list-0)
+ ,(copy-sequence bookmark-tests-bookmark-list-1)
+ ,(copy-sequence bookmark-tests-bookmark-list-2))))
+ (bookmark-default-file bookmark-tests-bookmark-file-list)
+ (bookmark-bookmarks-timestamp bookmark-tests-cache-timestamp-list)
+ bookmark-save-flag)
+ ,@body)))
+
+(defmacro with-bookmark-test-file-list (&rest body)
+ "Create environment for testing bookmark.el and evaluate BODY.
+Same as `with-bookmark-test-list' but also opens the resource file
+example.txt in a buffer, which can be accessed by callers through
+the lexically-bound variable `buffer'."
+ `(let ((buffer (find-file-noselect bookmark-tests-example-file)))
+ (unwind-protect
+ (with-bookmark-test-list
+ ,@body)
+ (kill-buffer buffer))))
+
(ert-deftest bookmark-tests-all-names ()
(with-bookmark-test
(should (equal (bookmark-all-names) '("name")))))
@@ -95,6 +159,30 @@ the lexically-bound variable `buffer'."
(with-bookmark-test
(should (equal (bookmark-get-bookmark-record "name") (cdr bookmark-tests-bookmark)))))
+(ert-deftest bookmark-tests-all-names-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-all-names) '("name-0"
+ "name-1"
+ "name-2")))))
+
+(ert-deftest bookmark-tests-get-bookmark-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-get-bookmark "name-0")
+ bookmark-tests-bookmark-list-0))
+ (should (equal (bookmark-get-bookmark "name-1")
+ bookmark-tests-bookmark-list-1))
+ (should (equal (bookmark-get-bookmark "name-2")
+ bookmark-tests-bookmark-list-2))))
+
+(ert-deftest bookmark-tests-get-bookmark-record-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-get-bookmark-record "name-0")
+ (cdr bookmark-tests-bookmark-list-0)))
+ (should (equal (bookmark-get-bookmark-record "name-1")
+ (cdr bookmark-tests-bookmark-list-1)))
+ (should (equal (bookmark-get-bookmark-record "name-2")
+ (cdr bookmark-tests-bookmark-list-2)))))
+
(ert-deftest bookmark-tests-record-getters-and-setters-new ()
(with-temp-buffer
(let* ((buffer-file-name "test")
@@ -130,6 +218,19 @@ the lexically-bound variable `buffer'."
;; calling twice gives same record
(should (equal (bookmark-make-record) record))))))
+(ert-deftest bookmark-tests-make-record-list ()
+ (with-bookmark-test-file-list
+ (let* ((record `("example.txt" (filename . ,bookmark-tests-example-file)
+ (front-context-string . "is text file is ")
+ (rear-context-string)
+ (position . 3)
+ (defaults "example.txt"))))
+ (with-current-buffer buffer
+ (goto-char 3)
+ (should (equal (bookmark-make-record) record))
+ ;; calling twice gives same record
+ (should (equal (bookmark-make-record) record))))))
+
(ert-deftest bookmark-tests-make-record-function ()
(with-bookmark-test
(let ((buffer-file-name "test"))
@@ -267,6 +368,11 @@ the lexically-bound variable `buffer'."
(bookmark-delete "name")
(should (equal bookmark-alist nil))))
+(ert-deftest bookmark-tests-delete-all ()
+ (with-bookmark-test-list
+ (bookmark-delete-all t)
+ (should (equal bookmark-alist nil))))
+
(defmacro with-bookmark-test-save-load (&rest body)
"Create environment for testing bookmark.el and evaluate BODY.
Same as `with-bookmark-test' but also sets a temporary
@@ -340,6 +446,18 @@ testing `bookmark-bmenu-list'."
,@body)
(kill-buffer bookmark-bmenu-buffer)))))
+(defmacro with-bookmark-bmenu-test-list (&rest body)
+ "Create environment for testing `bookmark-bmenu-list' and evaluate BODY.
+Same as `with-bookmark-test-list' but with additions suitable for
+testing `bookmark-bmenu-list'."
+ `(with-bookmark-test-list
+ (let ((bookmark-bmenu-buffer "*Bookmark List - Testing*"))
+ (unwind-protect
+ (save-window-excursion
+ (bookmark-bmenu-list)
+ ,@body)
+ (kill-buffer bookmark-bmenu-buffer)))))
+
(ert-deftest bookmark-test-bmenu-edit-annotation/show-annotation ()
(with-bookmark-bmenu-test
(bookmark-set-annotation "name" "foo")
@@ -402,6 +520,52 @@ testing `bookmark-bmenu-list'."
(beginning-of-line)
(should (bookmark-bmenu-any-marks))))
+(ert-deftest bookmark-test-bmenu-mark-all ()
+ (with-bookmark-bmenu-test-list
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-mark-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are marked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark)))))))
+
+(ert-deftest bookmark-test-bmenu-any-marks-list ()
+ (with-bookmark-bmenu-test-list
+ ;; Mark just the second item
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (forward-line 1)
+ (bookmark-bmenu-mark)
+ ;; Verify that only the second item is marked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ ;; There should be at least one mark
+ (should (bookmark-bmenu-any-marks))))
+
(ert-deftest bookmark-test-bmenu-unmark ()
(with-bookmark-bmenu-test
(bookmark-bmenu-mark)
@@ -410,12 +574,63 @@ testing `bookmark-bmenu-list'."
(beginning-of-line)
(should (looking-at "^ "))))
+(ert-deftest bookmark-test-bmenu-unmark-all ()
+ (with-bookmark-bmenu-test-list
+ (bookmark-bmenu-mark-all)
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-unmark-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are unmarked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark)))))))
+
(ert-deftest bookmark-test-bmenu-delete ()
(with-bookmark-bmenu-test
(bookmark-bmenu-delete)
(bookmark-bmenu-execute-deletions)
(should (equal (length bookmark-alist) 0))))
+(ert-deftest bookmark-test-bmenu-delete-all ()
+ (with-bookmark-bmenu-test-list
+ ;; Verify that unmarked bookmarks aren't deleted
+ (bookmark-bmenu-execute-deletions)
+ (should-not (eq bookmark-alist nil))
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-delete-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are marked for deletion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ ;; Verify that all bookmarks are deleted
+ (bookmark-bmenu-execute-deletions)
+ (should (eq bookmark-alist nil)))))
+
(ert-deftest bookmark-test-bmenu-locate ()
(let (msg)
(cl-letf (((symbol-function 'message)
--
2.26.2
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: fixed-emacs-27-Add-ability-to-mark-unmark-delete-all-bookmarks.patch --]
[-- Type: text/x-patch, Size: 23933 bytes --]
From 0fc0c9236d6df820e04387f95b34f549e7b104b0 Mon Sep 17 00:00:00 2001
From: Matthew White <mehw.is.me@inventati.org>
Date: Thu, 23 Jul 2020 21:14:32 +0000
Subject: [PATCH] Add ability to mark/unmark/delete all bookmarks
* lisp/bookmark.el (bookmark-delete-all): New function to delete all
bookmarks.
(bookmark-bmenu-mark-all): New function to mark all bookmarks in the
bookmark list buffer.
(bookmark-bmenu-unmark-all): New function to unmark all bookmarks in
the bookmark list buffer.
(bookmark-bmenu-delete-all): New function to mark for deletion all
bookmarks in the bookmark list buffer.
(bookmark-map): Map "D" to `bookmark-delete-all'.
(bookmark-bmenu-mode-map): New mappping for "M" to
`bookmark-bmenu-mark-all'.
(bookmark-bmenu-mode-map): New mappping for "U" to
`bookmark-bmenu-unmark-all'.
(bookmark-bmenu-mode-map): New mappping for "D" to
`bookmark-bmenu-delete-all'.
(bookmark-bmenu-mark-all): New bookmark menu to
`bookmark-delete-all'.
(easy-menu-define): New bookmark menu to `bookmark-bmenu-mark-all'.
(easy-menu-define): New bookmark menu to
`bookmark-bmenu-unmark-all'.
(easy-menu-define): New bookmark menu to
`bookmark-bmenu-delete-all'.
(bookmark-bmenu-select): Update docstring to include a reference to
`bookmark-bmenu-mark-all'.
(bookmark-bmenu-mode): Update docstring. Add/Update description:
`bookmark-bmenu-mark-all', `bookmark-bmenu-delete-all',
`bookmark-bmenu-execute-deletions', and `bookmark-bmenu-unmark-all'.
* test/lisp/bookmark-resources/test-list.bmk: New bookmark file to
test a list of bookmarks.
* test/lisp/bookmark-tests.el (bookmark-tests-bookmark-file-list): New
reference to the bookmark file used for testing a list of bookmarks.
(bookmark-tests-bookmark-list-0, bookmark-tests-bookmark-list-1,
bookmark-tests-bookmark-list-2): New cached values for testing a
list of bookmark.
(bookmark-tests-cache-timestamp-list): New variable to set
`bookmark-bookmarks-timestamp'.
(with-bookmark-test-list): New macro environment to test a list of
bookmarks.
(with-bookmark-test-file-list): New macro environment to test a list
of bookmarks with example.txt.
(with-bookmark-bmenu-test-list): New macro environment to test
functions about a list of bookmarks from `bookmark-bmenu-list'.
(bookmark-tests-all-names-list, bookmark-tests-get-bookmark-list,
bookmark-tests-get-bookmark-record-list): New functions to test the
records of the list of bookmarks.
(bookmark-tests-make-record-list): New function to test the creation
of a record from example.txt with a list of bookmarks loaded.
(bookmark-tests-delete-all): New function to test
`bookmark-delete-all'.
(bookmark-test-bmenu-any-marks-list): New function to test
`bookmark-bmenu-any-marks' with a list of bookmarks.
(bookmark-test-bmenu-mark-all): New function to test
`bookmark-bmenu-mark-all'.
(bookmark-test-bmenu-unmark-all): New function to test
`bookmark-bmenu-unmark-all'.
(bookmark-test-bmenu-delete-all): New function to test
`bookmark-bmenu-delete-all'.
---
lisp/bookmark.el | 77 +++++++-
test/lisp/bookmark-resources/test-list.bmk | 20 ++
test/lisp/bookmark-tests.el | 215 +++++++++++++++++++++
3 files changed, 310 insertions(+), 2 deletions(-)
create mode 100644 test/lisp/bookmark-resources/test-list.bmk
diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index e69d9f529c..c45a5103fe 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -200,6 +200,7 @@ A non-nil value may result in truncated bookmark names."
(define-key map "f" 'bookmark-insert-location) ;"f"ind
(define-key map "r" 'bookmark-rename)
(define-key map "d" 'bookmark-delete)
+ (define-key map "D" 'bookmark-delete-all)
(define-key map "l" 'bookmark-load)
(define-key map "w" 'bookmark-write)
(define-key map "s" 'bookmark-save)
@@ -1372,6 +1373,22 @@ probably because we were called from there."
(bookmark-save)))
+;;;###autoload
+(defun bookmark-delete-all (&optional no-confirm)
+ "Permanently delete all bookmarks.
+Doesn't ask for confirmation if NO-CONFIRM is non-nil."
+ (interactive "P")
+ (when (or no-confirm
+ (yes-or-no-p "Permanently delete all bookmarks? "))
+ (bookmark-maybe-load-default-file)
+ (setq bookmark-alist-modification-count
+ (+ bookmark-alist-modification-count (length bookmark-alist)))
+ (setq bookmark-alist nil)
+ (bookmark-bmenu-surreptitiously-rebuild-list)
+ (when (bookmark-time-to-save-p)
+ (bookmark-save))))
+
+
(defun bookmark-time-to-save-p (&optional final-time)
"Return t if it is time to save bookmarks to disk, nil otherwise.
Optional argument FINAL-TIME means this is being called when Emacs
@@ -1598,12 +1615,15 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
(define-key map "\C-d" 'bookmark-bmenu-delete-backwards)
(define-key map "x" 'bookmark-bmenu-execute-deletions)
(define-key map "d" 'bookmark-bmenu-delete)
+ (define-key map "D" 'bookmark-bmenu-delete-all)
(define-key map " " 'next-line)
(define-key map "n" 'next-line)
(define-key map "p" 'previous-line)
(define-key map "\177" 'bookmark-bmenu-backup-unmark)
(define-key map "u" 'bookmark-bmenu-unmark)
+ (define-key map "U" 'bookmark-bmenu-unmark-all)
(define-key map "m" 'bookmark-bmenu-mark)
+ (define-key map "M" 'bookmark-bmenu-mark-all)
(define-key map "l" 'bookmark-bmenu-load)
(define-key map "r" 'bookmark-bmenu-rename)
(define-key map "R" 'bookmark-bmenu-relocate)
@@ -1625,8 +1645,10 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
["Select Marked Bookmarks" bookmark-bmenu-select t]
"---"
["Mark Bookmark" bookmark-bmenu-mark t]
+ ["Mark all Bookmarks" bookmark-bmenu-mark-all t]
["Unmark Bookmark" bookmark-bmenu-unmark t]
["Unmark Backwards" bookmark-bmenu-backup-unmark t]
+ ["Unmark all Bookmarks" bookmark-bmenu-unmark-all t]
["Toggle Display of Filenames" bookmark-bmenu-toggle-filenames t]
["Display Location of Bookmark" bookmark-bmenu-locate t]
"---"
@@ -1634,6 +1656,7 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
["Rename Bookmark" bookmark-bmenu-rename t]
["Relocate Bookmark's File" bookmark-bmenu-relocate t]
["Mark Bookmark for Deletion" bookmark-bmenu-delete t]
+ ["Mark all Bookmarks for Deletion" bookmark-bmenu-delete-all t]
["Delete Marked Bookmarks" bookmark-bmenu-execute-deletions t])
("Annotations"
["Show Annotation for Current Bookmark" bookmark-bmenu-show-annotation t]
@@ -1746,6 +1769,7 @@ Letters do not insert themselves; instead, they are commands.
Bookmark names preceded by a \"*\" have annotations.
\\<bookmark-bmenu-mode-map>
\\[bookmark-bmenu-mark] -- mark bookmark to be displayed.
+\\[bookmark-bmenu-mark-all] -- mark all listed bookmarks to be displayed.
\\[bookmark-bmenu-select] -- select bookmark of line point is on.
Also show bookmarks marked using m in other windows.
\\[bookmark-bmenu-toggle-filenames] -- toggle displaying of filenames (they may obscure long bookmark names).
@@ -1762,13 +1786,15 @@ Bookmark names preceded by a \"*\" have annotations.
\\[bookmark-bmenu-relocate] -- relocate this bookmark's file (prompts for new file).
\\[bookmark-bmenu-delete] -- mark this bookmark to be deleted, and move down.
\\[bookmark-bmenu-delete-backwards] -- mark this bookmark to be deleted, and move up.
-\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]'.
+\\[bookmark-bmenu-delete-all] -- mark all listed bookmarks as to be deleted.
+\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]' or `\\[bookmark-bmenu-delete-all]'.
\\[bookmark-bmenu-save] -- save the current bookmark list in the default file.
With a prefix arg, prompts for a file to save in.
\\[bookmark-bmenu-load] -- load in a file of bookmarks (prompts for file.)
\\[bookmark-bmenu-unmark] -- remove all kinds of marks from current line.
With prefix argument, also move up one line.
\\[bookmark-bmenu-backup-unmark] -- back up a line and remove marks.
+\\[bookmark-bmenu-unmark-all] -- remove all kinds of marks from all listed bookmarks.
\\[bookmark-bmenu-show-annotation] -- show the annotation, if it exists, for the current bookmark
in another buffer.
\\[bookmark-bmenu-show-all-annotations] -- show the annotations of all bookmarks in another buffer.
@@ -1935,9 +1961,23 @@ If the annotation does not exist, do nothing."
(bookmark-bmenu-ensure-position))))
+(defun bookmark-bmenu-mark-all ()
+ "Mark all listed bookmarks to be displayed by \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-select]."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ (while (not (eobp))
+ (delete-char 1)
+ (insert ?>)
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-select ()
"Select this line's bookmark; also display bookmarks marked with `>'.
-You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] command."
+You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] or \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark-all] commands."
(interactive)
(let ((bmrk (bookmark-bmenu-bookmark))
(menu (current-buffer))
@@ -2106,6 +2146,20 @@ Optional BACKUP means move up."
(bookmark-bmenu-ensure-position))
+(defun bookmark-bmenu-unmark-all ()
+ "Cancel all requested operations on all listed bookmarks."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ (while (not (eobp))
+ (delete-char 1)
+ (insert " ")
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-delete ()
"Mark bookmark on this line to be deleted.
To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
@@ -2131,6 +2185,22 @@ To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\
(bookmark-bmenu-ensure-position))
+(defun bookmark-bmenu-delete-all ()
+ "Mark all listed bookmarks as to be deleted.
+To remove all deletion marks, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-unmark-all].
+To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ (while (not (eobp))
+ (delete-char 1)
+ (insert ?D)
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-execute-deletions ()
"Delete bookmarks flagged `D'."
(interactive)
@@ -2290,6 +2360,9 @@ strings returned are not."
(bindings--define-key map [delete]
'(menu-item "Delete Bookmark..." bookmark-delete
:help "Delete a bookmark from the bookmark list"))
+ (bindings--define-key map [delete-all]
+ '(menu-item "Delete all Bookmarks..." bookmark-delete-all
+ :help "Delete all bookmarks from the bookmark list"))
(bindings--define-key map [rename]
'(menu-item "Rename Bookmark..." bookmark-rename
:help "Change the name of a bookmark"))
diff --git a/test/lisp/bookmark-resources/test-list.bmk b/test/lisp/bookmark-resources/test-list.bmk
new file mode 100644
index 0000000000..696d64979b
--- /dev/null
+++ b/test/lisp/bookmark-resources/test-list.bmk
@@ -0,0 +1,20 @@
+;;;; Emacs Bookmark Format Version 1 ;;;; -*- coding: utf-8-emacs -*-
+;;; This format is meant to be slightly human-readable;
+;;; nevertheless, you probably don't want to edit it.
+;;; -*- End Of Bookmark File Format Version Stamp -*-
+(("name-0"
+ (filename . "/some/file-0")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+("name-1"
+ (filename . "/some/file-1")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+("name-2"
+ (filename . "/some/file-2")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+)
diff --git a/test/lisp/bookmark-tests.el b/test/lisp/bookmark-tests.el
index 7e0384b724..fbc440853e 100644
--- a/test/lisp/bookmark-tests.el
+++ b/test/lisp/bookmark-tests.el
@@ -82,6 +82,70 @@ the lexically-bound variable `buffer'."
,@body)
(kill-buffer buffer))))
+(defvar bookmark-tests-bookmark-file-list
+ (expand-file-name "test-list.bmk" bookmark-tests-data-dir)
+ "Bookmark file used for testing a list of bookmarks.")
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-0 '("name-0"
+ (filename . "/some/file-0")
+ (front-context-string . "ghi")
+ (rear-context-string . "jkl")
+ (position . 4))
+ "Cached value used in bookmark-tests.el."))
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-1 '("name-1"
+ (filename . "/some/file-1")
+ (front-context-string . "mno")
+ (rear-context-string . "pqr")
+ (position . 5))
+ "Cached value used in bookmark-tests.el."))
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-2 '("name-2"
+ (filename . "/some/file-2")
+ (front-context-string . "stu")
+ (rear-context-string . "vwx")
+ (position . 6))
+ "Cached value used in bookmark-tests.el."))
+
+(defvar bookmark-tests-cache-timestamp-list
+ (cons bookmark-tests-bookmark-file-list
+ (nth 5 (file-attributes
+ bookmark-tests-bookmark-file-list)))
+ "Cached value used in bookmark-tests.el.")
+
+(defmacro with-bookmark-test-list (&rest body)
+ "Create environment for testing bookmark.el and evaluate BODY.
+Ensure a clean environment for testing, and do not change user
+data when running tests interactively."
+ `(with-temp-buffer
+ (let ((bookmark-alist (quote (,(copy-sequence bookmark-tests-bookmark-list-0)
+ ,(copy-sequence bookmark-tests-bookmark-list-1)
+ ,(copy-sequence bookmark-tests-bookmark-list-2))))
+ (bookmark-default-file bookmark-tests-bookmark-file-list)
+ (bookmark-bookmarks-timestamp bookmark-tests-cache-timestamp-list)
+ bookmark-save-flag)
+ ,@body)))
+
+(defmacro with-bookmark-test-file-list (&rest body)
+ "Create environment for testing bookmark.el and evaluate BODY.
+Same as `with-bookmark-test-list' but also opens the resource file
+example.txt in a buffer, which can be accessed by callers through
+the lexically-bound variable `buffer'."
+ `(let ((buffer (find-file-noselect bookmark-tests-example-file)))
+ (unwind-protect
+ (with-bookmark-test-list
+ ,@body)
+ (kill-buffer buffer))))
+
(ert-deftest bookmark-tests-all-names ()
(with-bookmark-test
(should (equal (bookmark-all-names) '("name")))))
@@ -94,6 +158,30 @@ the lexically-bound variable `buffer'."
(with-bookmark-test
(should (equal (bookmark-get-bookmark-record "name") (cdr bookmark-tests-bookmark)))))
+(ert-deftest bookmark-tests-all-names-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-all-names) '("name-0"
+ "name-1"
+ "name-2")))))
+
+(ert-deftest bookmark-tests-get-bookmark-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-get-bookmark "name-0")
+ bookmark-tests-bookmark-list-0))
+ (should (equal (bookmark-get-bookmark "name-1")
+ bookmark-tests-bookmark-list-1))
+ (should (equal (bookmark-get-bookmark "name-2")
+ bookmark-tests-bookmark-list-2))))
+
+(ert-deftest bookmark-tests-get-bookmark-record-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-get-bookmark-record "name-0")
+ (cdr bookmark-tests-bookmark-list-0)))
+ (should (equal (bookmark-get-bookmark-record "name-1")
+ (cdr bookmark-tests-bookmark-list-1)))
+ (should (equal (bookmark-get-bookmark-record "name-2")
+ (cdr bookmark-tests-bookmark-list-2)))))
+
(ert-deftest bookmark-tests-record-getters-and-setters-new ()
(with-temp-buffer
(let* ((buffer-file-name "test")
@@ -129,6 +217,19 @@ the lexically-bound variable `buffer'."
;; calling twice gives same record
(should (equal (bookmark-make-record) record))))))
+(ert-deftest bookmark-tests-make-record-list ()
+ (with-bookmark-test-file-list
+ (let* ((record `("example.txt" (filename . ,bookmark-tests-example-file)
+ (front-context-string . "is text file is ")
+ (rear-context-string)
+ (position . 3)
+ (defaults "example.txt"))))
+ (with-current-buffer buffer
+ (goto-char 3)
+ (should (equal (bookmark-make-record) record))
+ ;; calling twice gives same record
+ (should (equal (bookmark-make-record) record))))))
+
(ert-deftest bookmark-tests-make-record-function ()
(with-bookmark-test
(let ((buffer-file-name "test"))
@@ -266,6 +367,11 @@ the lexically-bound variable `buffer'."
(bookmark-delete "name")
(should (equal bookmark-alist nil))))
+(ert-deftest bookmark-tests-delete-all ()
+ (with-bookmark-test-list
+ (bookmark-delete-all t)
+ (should (equal bookmark-alist nil))))
+
(defmacro with-bookmark-test-save-load (&rest body)
"Create environment for testing bookmark.el and evaluate BODY.
Same as `with-bookmark-test' but also sets a temporary
@@ -339,6 +445,18 @@ testing `bookmark-bmenu-list'."
,@body)
(kill-buffer bookmark-bmenu-buffer)))))
+(defmacro with-bookmark-bmenu-test-list (&rest body)
+ "Create environment for testing `bookmark-bmenu-list' and evaluate BODY.
+Same as `with-bookmark-test-list' but with additions suitable for
+testing `bookmark-bmenu-list'."
+ `(with-bookmark-test-list
+ (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")
@@ -362,5 +480,102 @@ testing `bookmark-bmenu-list'."
(should (equal (buffer-name (current-buffer)) bookmark-bmenu-buffer))
(should (looking-at "name"))))
+(ert-deftest bookmark-test-bmenu-mark-all ()
+ (with-bookmark-bmenu-test-list
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-mark-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are marked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark)))))))
+
+(ert-deftest bookmark-test-bmenu-any-marks-list ()
+ (with-bookmark-bmenu-test-list
+ ;; Mark just the second item
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (forward-line 1)
+ (bookmark-bmenu-mark)
+ ;; Verify that only the second item is marked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ ;; There should be at least one mark
+ (should (bookmark-bmenu-any-marks))))
+
+(ert-deftest bookmark-test-bmenu-unmark-all ()
+ (with-bookmark-bmenu-test-list
+ (bookmark-bmenu-mark-all)
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-unmark-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are unmarked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark)))))))
+
+(ert-deftest bookmark-test-bmenu-delete-all ()
+ (with-bookmark-bmenu-test-list
+ ;; Verify that unmarked bookmarks aren't deleted
+ (bookmark-bmenu-execute-deletions)
+ (should-not (eq bookmark-alist nil))
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-delete-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are marked for deletion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ ;; Verify that all bookmarks are deleted
+ (bookmark-bmenu-execute-deletions)
+ (should (eq bookmark-alist nil)))))
+
(provide 'bookmark-tests)
;;; bookmark-tests.el ends here
--
2.26.2
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-31 2:58 ` Matthew White
@ 2020-07-31 6:01 ` Karl Fogel
2020-08-02 22:13 ` Karl Fogel
1 sibling, 0 replies; 52+ messages in thread
From: Karl Fogel @ 2020-07-31 6:01 UTC (permalink / raw)
To: Matthew White; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 577 bytes --]
On 31 Jul 2020, Matthew White wrote:
>There was one extra 'looking-at "^> "' (changed to 'looking-at "^ "').
>
>I attach revised patches (master and emacs-27, with the latter just in
>case someone wants to test it on emacs-27).
Got it! I will try again with the new the 'master' patch (both manual testing and 'make check'). I'll also do a top-to-bottom review again of the diff, just so we can be sure.
More soon.
>PS: I finally got the PDF copyright assignment... I signed and emailed
>it back... now I'm waiting for the FSF signature.
Excellent.
Best regards,
-Karl
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-07-31 2:58 ` Matthew White
2020-07-31 6:01 ` Karl Fogel
@ 2020-08-02 22:13 ` Karl Fogel
2020-08-06 17:59 ` Matthew White
1 sibling, 1 reply; 52+ messages in thread
From: Karl Fogel @ 2020-08-02 22:13 UTC (permalink / raw)
To: Matthew White; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 2005 bytes --]
I wrote:
>I will try again with the new the 'master' patch (both manual testing
>and 'make check'). I'll also do a top-to-bottom review again of the
>diff, just so we can be sure.
Hi, Matthew. I have had a chance to test the new changes against the 'master' branch, both manually and with 'make check'. Everything looks good.
I also re-reviewed the diff. Actually, I diffed the *new* diff against your original diff from July 24th, since I'd already reviewed that one, and then I just reviewed the meta-diff :-). Everything seems fine. I see that in the tests you add some hyphens to bookmark names, e.g., "name0" to "name-0" (no problem). You also started using the existing `bookmark-bmenu-any-marks' in the tests -- good thinking; I had forgotten that that function existed.
There is one very minor thing that I should have spotted before. It is so minor that there is no need to post a new patch -- I can just add a fixup commit after applying your commit. In the new function `bookmark-delete-all', the doc string says:
"Permanently delete all bookmarks.
Doesn't ask for confirmation if NO-CONFIRM is non-nil."
A more Emacs-y way to write this would be:
"Permanently delete all bookmarks.
If optional argument NO-CONFIRM is non-nil, don't ask for confirmation."
There are actually several changes there:
- Say that the argument is optional.
- Put the condition before the consequence.
- Use an active verb rather than a semi-stative one ("don't" instead
of "doesn't"). You did this in the first sentence ("delete all
bookmarks"), just not in the second. For more on doc strings, see
https://www.gnu.org/software/emacs/manual/html_node/elisp/Documentation-Tips.html .
Again, there is no need to redo the patch (unless you feel like it). We can take care of it in a follow-up commit.
Please let us know when your paperwork is all done. I'm looking forward to having this change in Emacs.
Best regards,
-Karl
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-08-02 22:13 ` Karl Fogel
@ 2020-08-06 17:59 ` Matthew White
2020-08-07 1:10 ` Karl Fogel
0 siblings, 1 reply; 52+ messages in thread
From: Matthew White @ 2020-08-06 17:59 UTC (permalink / raw)
To: Karl Fogel; +Cc: emacs-devel
[-- Attachment #1.1: Type: text/plain, Size: 2153 bytes --]
On Sun, 02 Aug 2020 17:13:50 -0500
Karl Fogel wrote:
> Hi, Matthew. I have had a chance to test the new changes against the
> 'master' branch, both manually and with 'make check'. Everything
> looks good.
>
> I also re-reviewed the diff. Actually, I diffed the *new* diff
> against your original diff from July 24th, since I'd already reviewed
> that one, and then I just reviewed the meta-diff :-). Everything
> seems fine. I see that in the tests you add some hyphens to bookmark
> names, e.g., "name0" to "name-0" (no problem). You also started
> using the existing `bookmark-bmenu-any-marks' in the tests -- good
> thinking; I had forgotten that that function existed.
>
> There is one very minor thing that I should have spotted before. It
> is so minor that there is no need to post a new patch -- I can just
> add a fixup commit after applying your commit. In the new function
> `bookmark-delete-all', the doc string says:
>
> "Permanently delete all bookmarks.
> Doesn't ask for confirmation if NO-CONFIRM is non-nil."
>
> A more Emacs-y way to write this would be:
>
> "Permanently delete all bookmarks.
> If optional argument NO-CONFIRM is non-nil, don't ask for
> confirmation."
Hi Karl, you keep surprising me. Thanks!
> Again, there is no need to redo the patch (unless you feel like it).
> We can take care of it in a follow-up commit.
>
> Please let us know when your paperwork is all done. I'm looking
> forward to having this change in Emacs.
>
> Best regards,
> -Karl
Karl, I waited to answer you in the hope of getting the paperwork...
Still nothing here... I don't know if the FSF already registered my
copyright assignment request, they have my signature, though.
About the patch, your help is priceless, Karl. I have no trouble to
attach an amended patch, what pressures me is to give you the right
credits! In these days I was thinking how you make me look smarter
than I really am...
Do as you feel, Karl. The attached patch is my attempt to give you
credit, but you are free to do a follow-up commit, if you prefer.
Best regards,
Matthew
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: master-20200806-Add-ability-to-mark-unmark-delete-all-bookmarks.patch --]
[-- Type: text/x-patch, Size: 24466 bytes --]
From 716c427928f0b1c9a124608869f848556edc2e12 Mon Sep 17 00:00:00 2001
From: Matthew White <mehw.is.me@inventati.org>
Date: Thu, 23 Jul 2020 21:14:32 +0000
Subject: [PATCH] Add ability to mark/unmark/delete all bookmarks
Special thanks to Karl Fogel, for the priceless help given to review
and correct this commit.
* lisp/bookmark.el (bookmark-delete-all): New function to delete all
bookmarks.
(bookmark-bmenu-mark-all): New function to mark all bookmarks in the
bookmark list buffer.
(bookmark-bmenu-unmark-all): New function to unmark all bookmarks in
the bookmark list buffer.
(bookmark-bmenu-delete-all): New function to mark for deletion all
bookmarks in the bookmark list buffer.
(bookmark-map): Map "D" to `bookmark-delete-all'.
(bookmark-bmenu-mode-map): New mappping for "M" to
`bookmark-bmenu-mark-all'.
(bookmark-bmenu-mode-map): New mappping for "U" to
`bookmark-bmenu-unmark-all'.
(bookmark-bmenu-mode-map): New mappping for "D" to
`bookmark-bmenu-delete-all'.
(bookmark-bmenu-mark-all): New bookmark menu to
`bookmark-delete-all'.
(easy-menu-define): New bookmark menu to `bookmark-bmenu-mark-all'.
(easy-menu-define): New bookmark menu to
`bookmark-bmenu-unmark-all'.
(easy-menu-define): New bookmark menu to
`bookmark-bmenu-delete-all'.
(bookmark-bmenu-select): Update docstring to include a reference to
`bookmark-bmenu-mark-all'.
(bookmark-bmenu-mode): Update docstring. Add/Update description:
`bookmark-bmenu-mark-all', `bookmark-bmenu-delete-all',
`bookmark-bmenu-execute-deletions', and `bookmark-bmenu-unmark-all'.
* test/lisp/bookmark-resources/test-list.bmk: New bookmark file to
test a list of bookmarks.
* test/lisp/bookmark-tests.el (bookmark-tests-bookmark-file-list): New
reference to the bookmark file used for testing a list of bookmarks.
(bookmark-tests-bookmark-list-0, bookmark-tests-bookmark-list-1,
bookmark-tests-bookmark-list-2): New cached values for testing a
list of bookmark.
(bookmark-tests-cache-timestamp-list): New variable to set
`bookmark-bookmarks-timestamp'.
(with-bookmark-test-list): New macro environment to test a list of
bookmarks.
(with-bookmark-test-file-list): New macro environment to test a list
of bookmarks with example.txt.
(with-bookmark-bmenu-test-list): New macro environment to test
functions about a list of bookmarks from `bookmark-bmenu-list'.
(bookmark-tests-all-names-list, bookmark-tests-get-bookmark-list,
bookmark-tests-get-bookmark-record-list): New functions to test the
records of the list of bookmarks.
(bookmark-tests-make-record-list): New function to test the creation
of a record from example.txt with a list of bookmarks loaded.
(bookmark-tests-delete-all): New function to test
`bookmark-delete-all'.
(bookmark-test-bmenu-any-marks-list): New function to test
`bookmark-bmenu-any-marks' with a list of bookmarks.
(bookmark-test-bmenu-mark-all): New function to test
`bookmark-bmenu-mark-all'.
(bookmark-test-bmenu-unmark-all): New function to test
`bookmark-bmenu-unmark-all'.
(bookmark-test-bmenu-delete-all): New function to test
`bookmark-bmenu-delete-all'.
---
lisp/bookmark.el | 78 +++++++-
test/lisp/bookmark-resources/test-list.bmk | 20 ++
test/lisp/bookmark-tests.el | 215 +++++++++++++++++++++
3 files changed, 311 insertions(+), 2 deletions(-)
create mode 100644 test/lisp/bookmark-resources/test-list.bmk
diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index de7d60f97e..24264d7d38 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -200,6 +200,7 @@ A non-nil value may result in truncated bookmark names."
(define-key map "f" 'bookmark-insert-location) ;"f"ind
(define-key map "r" 'bookmark-rename)
(define-key map "d" 'bookmark-delete)
+ (define-key map "D" 'bookmark-delete-all)
(define-key map "l" 'bookmark-load)
(define-key map "w" 'bookmark-write)
(define-key map "s" 'bookmark-save)
@@ -1374,6 +1375,23 @@ probably because we were called from there."
(bookmark-save)))
+;;;###autoload
+(defun bookmark-delete-all (&optional no-confirm)
+ "Permanently delete all bookmarks.
+If optional argument NO-CONFIRM is non-nil, don't ask for
+confirmation."
+ (interactive "P")
+ (when (or no-confirm
+ (yes-or-no-p "Permanently delete all bookmarks? "))
+ (bookmark-maybe-load-default-file)
+ (setq bookmark-alist-modification-count
+ (+ bookmark-alist-modification-count (length bookmark-alist)))
+ (setq bookmark-alist nil)
+ (bookmark-bmenu-surreptitiously-rebuild-list)
+ (when (bookmark-time-to-save-p)
+ (bookmark-save))))
+
+
(defun bookmark-time-to-save-p (&optional final-time)
"Return t if it is time to save bookmarks to disk, nil otherwise.
Optional argument FINAL-TIME means this is being called when Emacs
@@ -1600,12 +1618,15 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
(define-key map "\C-d" 'bookmark-bmenu-delete-backwards)
(define-key map "x" 'bookmark-bmenu-execute-deletions)
(define-key map "d" 'bookmark-bmenu-delete)
+ (define-key map "D" 'bookmark-bmenu-delete-all)
(define-key map " " 'next-line)
(define-key map "n" 'next-line)
(define-key map "p" 'previous-line)
(define-key map "\177" 'bookmark-bmenu-backup-unmark)
(define-key map "u" 'bookmark-bmenu-unmark)
+ (define-key map "U" 'bookmark-bmenu-unmark-all)
(define-key map "m" 'bookmark-bmenu-mark)
+ (define-key map "M" 'bookmark-bmenu-mark-all)
(define-key map "l" 'bookmark-bmenu-load)
(define-key map "r" 'bookmark-bmenu-rename)
(define-key map "R" 'bookmark-bmenu-relocate)
@@ -1627,8 +1648,10 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
["Select Marked Bookmarks" bookmark-bmenu-select t]
"---"
["Mark Bookmark" bookmark-bmenu-mark t]
+ ["Mark all Bookmarks" bookmark-bmenu-mark-all t]
["Unmark Bookmark" bookmark-bmenu-unmark t]
["Unmark Backwards" bookmark-bmenu-backup-unmark t]
+ ["Unmark all Bookmarks" bookmark-bmenu-unmark-all t]
["Toggle Display of Filenames" bookmark-bmenu-toggle-filenames t]
["Display Location of Bookmark" bookmark-bmenu-locate t]
"---"
@@ -1636,6 +1659,7 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
["Rename Bookmark" bookmark-bmenu-rename t]
["Relocate Bookmark's File" bookmark-bmenu-relocate t]
["Mark Bookmark for Deletion" bookmark-bmenu-delete t]
+ ["Mark all Bookmarks for Deletion" bookmark-bmenu-delete-all t]
["Delete Marked Bookmarks" bookmark-bmenu-execute-deletions t])
("Annotations"
["Show Annotation for Current Bookmark" bookmark-bmenu-show-annotation t]
@@ -1748,6 +1772,7 @@ Letters do not insert themselves; instead, they are commands.
Bookmark names preceded by a \"*\" have annotations.
\\<bookmark-bmenu-mode-map>
\\[bookmark-bmenu-mark] -- mark bookmark to be displayed.
+\\[bookmark-bmenu-mark-all] -- mark all listed bookmarks to be displayed.
\\[bookmark-bmenu-select] -- select bookmark of line point is on.
Also show bookmarks marked using m in other windows.
\\[bookmark-bmenu-toggle-filenames] -- toggle displaying of filenames (they may obscure long bookmark names).
@@ -1764,13 +1789,15 @@ Bookmark names preceded by a \"*\" have annotations.
\\[bookmark-bmenu-relocate] -- relocate this bookmark's file (prompts for new file).
\\[bookmark-bmenu-delete] -- mark this bookmark to be deleted, and move down.
\\[bookmark-bmenu-delete-backwards] -- mark this bookmark to be deleted, and move up.
-\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]'.
+\\[bookmark-bmenu-delete-all] -- mark all listed bookmarks as to be deleted.
+\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]' or `\\[bookmark-bmenu-delete-all]'.
\\[bookmark-bmenu-save] -- save the current bookmark list in the default file.
With a prefix arg, prompts for a file to save in.
\\[bookmark-bmenu-load] -- load in a file of bookmarks (prompts for file.)
\\[bookmark-bmenu-unmark] -- remove all kinds of marks from current line.
With prefix argument, also move up one line.
\\[bookmark-bmenu-backup-unmark] -- back up a line and remove marks.
+\\[bookmark-bmenu-unmark-all] -- remove all kinds of marks from all listed bookmarks.
\\[bookmark-bmenu-show-annotation] -- show the annotation, if it exists, for the current bookmark
in another buffer.
\\[bookmark-bmenu-show-all-annotations] -- show the annotations of all bookmarks in another buffer.
@@ -1937,9 +1964,23 @@ If the annotation does not exist, do nothing."
(bookmark-bmenu-ensure-position))))
+(defun bookmark-bmenu-mark-all ()
+ "Mark all listed bookmarks to be displayed by \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-select]."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ (while (not (eobp))
+ (delete-char 1)
+ (insert ?>)
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-select ()
"Select this line's bookmark; also display bookmarks marked with `>'.
-You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] command."
+You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] or \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark-all] commands."
(interactive)
(let ((bmrk (bookmark-bmenu-bookmark))
(menu (current-buffer))
@@ -2108,6 +2149,20 @@ Optional BACKUP means move up."
(bookmark-bmenu-ensure-position))
+(defun bookmark-bmenu-unmark-all ()
+ "Cancel all requested operations on all listed bookmarks."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ (while (not (eobp))
+ (delete-char 1)
+ (insert " ")
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-delete ()
"Mark bookmark on this line to be deleted.
To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
@@ -2133,6 +2188,22 @@ To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\
(bookmark-bmenu-ensure-position))
+(defun bookmark-bmenu-delete-all ()
+ "Mark all listed bookmarks as to be deleted.
+To remove all deletion marks, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-unmark-all].
+To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
+ (interactive)
+ (save-excursion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (with-buffer-modified-unmodified
+ (let ((inhibit-read-only t))
+ (while (not (eobp))
+ (delete-char 1)
+ (insert ?D)
+ (forward-line 1))))))
+
+
(defun bookmark-bmenu-execute-deletions ()
"Delete bookmarks flagged `D'."
(interactive)
@@ -2292,6 +2363,9 @@ strings returned are not."
(bindings--define-key map [delete]
'(menu-item "Delete Bookmark..." bookmark-delete
:help "Delete a bookmark from the bookmark list"))
+ (bindings--define-key map [delete-all]
+ '(menu-item "Delete all Bookmarks..." bookmark-delete-all
+ :help "Delete all bookmarks from the bookmark list"))
(bindings--define-key map [rename]
'(menu-item "Rename Bookmark..." bookmark-rename
:help "Change the name of a bookmark"))
diff --git a/test/lisp/bookmark-resources/test-list.bmk b/test/lisp/bookmark-resources/test-list.bmk
new file mode 100644
index 0000000000..696d64979b
--- /dev/null
+++ b/test/lisp/bookmark-resources/test-list.bmk
@@ -0,0 +1,20 @@
+;;;; Emacs Bookmark Format Version 1 ;;;; -*- coding: utf-8-emacs -*-
+;;; This format is meant to be slightly human-readable;
+;;; nevertheless, you probably don't want to edit it.
+;;; -*- End Of Bookmark File Format Version Stamp -*-
+(("name-0"
+ (filename . "/some/file-0")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+("name-1"
+ (filename . "/some/file-1")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+("name-2"
+ (filename . "/some/file-2")
+ (front-context-string . "abc")
+ (rear-context-string . "def")
+ (position . 3))
+)
diff --git a/test/lisp/bookmark-tests.el b/test/lisp/bookmark-tests.el
index b9c6ff9c54..c5959e46d8 100644
--- a/test/lisp/bookmark-tests.el
+++ b/test/lisp/bookmark-tests.el
@@ -83,6 +83,70 @@ the lexically-bound variable `buffer'."
,@body)
(kill-buffer buffer))))
+(defvar bookmark-tests-bookmark-file-list
+ (expand-file-name "test-list.bmk" bookmark-tests-data-dir)
+ "Bookmark file used for testing a list of bookmarks.")
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-0 '("name-0"
+ (filename . "/some/file-0")
+ (front-context-string . "ghi")
+ (rear-context-string . "jkl")
+ (position . 4))
+ "Cached value used in bookmark-tests.el."))
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-1 '("name-1"
+ (filename . "/some/file-1")
+ (front-context-string . "mno")
+ (rear-context-string . "pqr")
+ (position . 5))
+ "Cached value used in bookmark-tests.el."))
+
+;; The values below should match `bookmark-tests-bookmark-file-list'
+;; content. We cache these values to speed up tests.
+(eval-and-compile ; needed by `with-bookmark-test-list' macro
+ (defvar bookmark-tests-bookmark-list-2 '("name-2"
+ (filename . "/some/file-2")
+ (front-context-string . "stu")
+ (rear-context-string . "vwx")
+ (position . 6))
+ "Cached value used in bookmark-tests.el."))
+
+(defvar bookmark-tests-cache-timestamp-list
+ (cons bookmark-tests-bookmark-file-list
+ (nth 5 (file-attributes
+ bookmark-tests-bookmark-file-list)))
+ "Cached value used in bookmark-tests.el.")
+
+(defmacro with-bookmark-test-list (&rest body)
+ "Create environment for testing bookmark.el and evaluate BODY.
+Ensure a clean environment for testing, and do not change user
+data when running tests interactively."
+ `(with-temp-buffer
+ (let ((bookmark-alist (quote (,(copy-sequence bookmark-tests-bookmark-list-0)
+ ,(copy-sequence bookmark-tests-bookmark-list-1)
+ ,(copy-sequence bookmark-tests-bookmark-list-2))))
+ (bookmark-default-file bookmark-tests-bookmark-file-list)
+ (bookmark-bookmarks-timestamp bookmark-tests-cache-timestamp-list)
+ bookmark-save-flag)
+ ,@body)))
+
+(defmacro with-bookmark-test-file-list (&rest body)
+ "Create environment for testing bookmark.el and evaluate BODY.
+Same as `with-bookmark-test-list' but also opens the resource file
+example.txt in a buffer, which can be accessed by callers through
+the lexically-bound variable `buffer'."
+ `(let ((buffer (find-file-noselect bookmark-tests-example-file)))
+ (unwind-protect
+ (with-bookmark-test-list
+ ,@body)
+ (kill-buffer buffer))))
+
(ert-deftest bookmark-tests-all-names ()
(with-bookmark-test
(should (equal (bookmark-all-names) '("name")))))
@@ -95,6 +159,30 @@ the lexically-bound variable `buffer'."
(with-bookmark-test
(should (equal (bookmark-get-bookmark-record "name") (cdr bookmark-tests-bookmark)))))
+(ert-deftest bookmark-tests-all-names-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-all-names) '("name-0"
+ "name-1"
+ "name-2")))))
+
+(ert-deftest bookmark-tests-get-bookmark-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-get-bookmark "name-0")
+ bookmark-tests-bookmark-list-0))
+ (should (equal (bookmark-get-bookmark "name-1")
+ bookmark-tests-bookmark-list-1))
+ (should (equal (bookmark-get-bookmark "name-2")
+ bookmark-tests-bookmark-list-2))))
+
+(ert-deftest bookmark-tests-get-bookmark-record-list ()
+ (with-bookmark-test-list
+ (should (equal (bookmark-get-bookmark-record "name-0")
+ (cdr bookmark-tests-bookmark-list-0)))
+ (should (equal (bookmark-get-bookmark-record "name-1")
+ (cdr bookmark-tests-bookmark-list-1)))
+ (should (equal (bookmark-get-bookmark-record "name-2")
+ (cdr bookmark-tests-bookmark-list-2)))))
+
(ert-deftest bookmark-tests-record-getters-and-setters-new ()
(with-temp-buffer
(let* ((buffer-file-name "test")
@@ -130,6 +218,19 @@ the lexically-bound variable `buffer'."
;; calling twice gives same record
(should (equal (bookmark-make-record) record))))))
+(ert-deftest bookmark-tests-make-record-list ()
+ (with-bookmark-test-file-list
+ (let* ((record `("example.txt" (filename . ,bookmark-tests-example-file)
+ (front-context-string . "is text file is ")
+ (rear-context-string)
+ (position . 3)
+ (defaults "example.txt"))))
+ (with-current-buffer buffer
+ (goto-char 3)
+ (should (equal (bookmark-make-record) record))
+ ;; calling twice gives same record
+ (should (equal (bookmark-make-record) record))))))
+
(ert-deftest bookmark-tests-make-record-function ()
(with-bookmark-test
(let ((buffer-file-name "test"))
@@ -267,6 +368,11 @@ the lexically-bound variable `buffer'."
(bookmark-delete "name")
(should (equal bookmark-alist nil))))
+(ert-deftest bookmark-tests-delete-all ()
+ (with-bookmark-test-list
+ (bookmark-delete-all t)
+ (should (equal bookmark-alist nil))))
+
(defmacro with-bookmark-test-save-load (&rest body)
"Create environment for testing bookmark.el and evaluate BODY.
Same as `with-bookmark-test' but also sets a temporary
@@ -340,6 +446,18 @@ testing `bookmark-bmenu-list'."
,@body)
(kill-buffer bookmark-bmenu-buffer)))))
+(defmacro with-bookmark-bmenu-test-list (&rest body)
+ "Create environment for testing `bookmark-bmenu-list' and evaluate BODY.
+Same as `with-bookmark-test-list' but with additions suitable for
+testing `bookmark-bmenu-list'."
+ `(with-bookmark-test-list
+ (let ((bookmark-bmenu-buffer "*Bookmark List - Testing*"))
+ (unwind-protect
+ (save-window-excursion
+ (bookmark-bmenu-list)
+ ,@body)
+ (kill-buffer bookmark-bmenu-buffer)))))
+
(ert-deftest bookmark-test-bmenu-edit-annotation/show-annotation ()
(with-bookmark-bmenu-test
(bookmark-set-annotation "name" "foo")
@@ -402,6 +520,52 @@ testing `bookmark-bmenu-list'."
(beginning-of-line)
(should (bookmark-bmenu-any-marks))))
+(ert-deftest bookmark-test-bmenu-mark-all ()
+ (with-bookmark-bmenu-test-list
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-mark-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are marked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark)))))))
+
+(ert-deftest bookmark-test-bmenu-any-marks-list ()
+ (with-bookmark-bmenu-test-list
+ ;; Mark just the second item
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (forward-line 1)
+ (bookmark-bmenu-mark)
+ ;; Verify that only the second item is marked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^> "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ ;; There should be at least one mark
+ (should (bookmark-bmenu-any-marks))))
+
(ert-deftest bookmark-test-bmenu-unmark ()
(with-bookmark-bmenu-test
(bookmark-bmenu-mark)
@@ -410,12 +574,63 @@ testing `bookmark-bmenu-list'."
(beginning-of-line)
(should (looking-at "^ "))))
+(ert-deftest bookmark-test-bmenu-unmark-all ()
+ (with-bookmark-bmenu-test-list
+ (bookmark-bmenu-mark-all)
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-unmark-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are unmarked
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^ "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark)))))))
+
(ert-deftest bookmark-test-bmenu-delete ()
(with-bookmark-bmenu-test
(bookmark-bmenu-delete)
(bookmark-bmenu-execute-deletions)
(should (equal (length bookmark-alist) 0))))
+(ert-deftest bookmark-test-bmenu-delete-all ()
+ (with-bookmark-bmenu-test-list
+ ;; Verify that unmarked bookmarks aren't deleted
+ (bookmark-bmenu-execute-deletions)
+ (should-not (eq bookmark-alist nil))
+ (let ((here (point-max)))
+ ;; Expect to not move the point
+ (goto-char here)
+ (bookmark-bmenu-delete-all)
+ (should (equal here (point)))
+ ;; Verify that all bookmarks are marked for deletion
+ (goto-char (point-min))
+ (bookmark-bmenu-ensure-position)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-0
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-1
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ (forward-line 1)
+ (should (looking-at "^D "))
+ (should (equal bookmark-tests-bookmark-list-2
+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
+ ;; Verify that all bookmarks are deleted
+ (bookmark-bmenu-execute-deletions)
+ (should (eq bookmark-alist nil)))))
+
(ert-deftest bookmark-test-bmenu-locate ()
(let (msg)
(cl-letf (((symbol-function 'message)
--
2.26.2
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-08-06 17:59 ` Matthew White
@ 2020-08-07 1:10 ` Karl Fogel
2020-08-07 8:33 ` Matthew White
[not found] ` <jwveeojyxh1.fsf-monnier+emacs@gnu.org>
0 siblings, 2 replies; 52+ messages in thread
From: Karl Fogel @ 2020-08-07 1:10 UTC (permalink / raw)
To: Matthew White; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 25965 bytes --]
>Karl, I waited to answer you in the hope of getting the paperwork...
>Still nothing here... I don't know if the FSF already registered my
>copyright assignment request, they have my signature, though.
I'll ask about it in a separate thread, no problem. Maybe it's all done and we just didn't get the news yet.
>About the patch, your help is priceless, Karl. I have no trouble to
>attach an amended patch, what pressures me is to give you the right
>credits! In these days I was thinking how you make me look smarter
>than I really am...
>
>Do as you feel, Karl. The attached patch is my attempt to give you
>credit, but you are free to do a follow-up commit, if you prefer.
Aw, Matthew, thanks, but you did all the hard work here! If you're okay with it, when I apply the commit I'll just change your commit message to include a simple phrase like "Thanks to Karl Fogel for pre-commit review" or something like that.
Best regards,
-Karl
>From 716c427928f0b1c9a124608869f848556edc2e12 Mon Sep 17 00:00:00 2001
>From: Matthew White <mehw.is.me@inventati.org>
>Date: Thu, 23 Jul 2020 21:14:32 +0000
>Subject: [PATCH] Add ability to mark/unmark/delete all bookmarks
>
>Special thanks to Karl Fogel, for the priceless help given to review
>and correct this commit.
>
>* lisp/bookmark.el (bookmark-delete-all): New function to delete all
> bookmarks.
> (bookmark-bmenu-mark-all): New function to mark all bookmarks in the
> bookmark list buffer.
> (bookmark-bmenu-unmark-all): New function to unmark all bookmarks in
> the bookmark list buffer.
> (bookmark-bmenu-delete-all): New function to mark for deletion all
> bookmarks in the bookmark list buffer.
> (bookmark-map): Map "D" to `bookmark-delete-all'.
> (bookmark-bmenu-mode-map): New mappping for "M" to
> `bookmark-bmenu-mark-all'.
> (bookmark-bmenu-mode-map): New mappping for "U" to
> `bookmark-bmenu-unmark-all'.
> (bookmark-bmenu-mode-map): New mappping for "D" to
> `bookmark-bmenu-delete-all'.
> (bookmark-bmenu-mark-all): New bookmark menu to
> `bookmark-delete-all'.
> (easy-menu-define): New bookmark menu to `bookmark-bmenu-mark-all'.
> (easy-menu-define): New bookmark menu to
> `bookmark-bmenu-unmark-all'.
> (easy-menu-define): New bookmark menu to
> `bookmark-bmenu-delete-all'.
> (bookmark-bmenu-select): Update docstring to include a reference to
> `bookmark-bmenu-mark-all'.
> (bookmark-bmenu-mode): Update docstring. Add/Update description:
> `bookmark-bmenu-mark-all', `bookmark-bmenu-delete-all',
> `bookmark-bmenu-execute-deletions', and `bookmark-bmenu-unmark-all'.
>* test/lisp/bookmark-resources/test-list.bmk: New bookmark file to
> test a list of bookmarks.
>* test/lisp/bookmark-tests.el (bookmark-tests-bookmark-file-list): New
> reference to the bookmark file used for testing a list of bookmarks.
> (bookmark-tests-bookmark-list-0, bookmark-tests-bookmark-list-1,
> bookmark-tests-bookmark-list-2): New cached values for testing a
> list of bookmark.
> (bookmark-tests-cache-timestamp-list): New variable to set
> `bookmark-bookmarks-timestamp'.
> (with-bookmark-test-list): New macro environment to test a list of
> bookmarks.
> (with-bookmark-test-file-list): New macro environment to test a list
> of bookmarks with example.txt.
> (with-bookmark-bmenu-test-list): New macro environment to test
> functions about a list of bookmarks from `bookmark-bmenu-list'.
> (bookmark-tests-all-names-list, bookmark-tests-get-bookmark-list,
> bookmark-tests-get-bookmark-record-list): New functions to test the
> records of the list of bookmarks.
> (bookmark-tests-make-record-list): New function to test the creation
> of a record from example.txt with a list of bookmarks loaded.
> (bookmark-tests-delete-all): New function to test
> `bookmark-delete-all'.
> (bookmark-test-bmenu-any-marks-list): New function to test
> `bookmark-bmenu-any-marks' with a list of bookmarks.
> (bookmark-test-bmenu-mark-all): New function to test
> `bookmark-bmenu-mark-all'.
> (bookmark-test-bmenu-unmark-all): New function to test
> `bookmark-bmenu-unmark-all'.
> (bookmark-test-bmenu-delete-all): New function to test
> `bookmark-bmenu-delete-all'.
>---
> lisp/bookmark.el | 78 +++++++-
> test/lisp/bookmark-resources/test-list.bmk | 20 ++
> test/lisp/bookmark-tests.el | 215 +++++++++++++++++++++
> 3 files changed, 311 insertions(+), 2 deletions(-)
> create mode 100644 test/lisp/bookmark-resources/test-list.bmk
>
>diff --git a/lisp/bookmark.el b/lisp/bookmark.el
>index de7d60f97e..24264d7d38 100644
>--- a/lisp/bookmark.el
>+++ b/lisp/bookmark.el
>@@ -200,6 +200,7 @@ A non-nil value may result in truncated bookmark names."
> (define-key map "f" 'bookmark-insert-location) ;"f"ind
> (define-key map "r" 'bookmark-rename)
> (define-key map "d" 'bookmark-delete)
>+ (define-key map "D" 'bookmark-delete-all)
> (define-key map "l" 'bookmark-load)
> (define-key map "w" 'bookmark-write)
> (define-key map "s" 'bookmark-save)
>@@ -1374,6 +1375,23 @@ probably because we were called from there."
> (bookmark-save)))
>
>
>+;;;###autoload
>+(defun bookmark-delete-all (&optional no-confirm)
>+ "Permanently delete all bookmarks.
>+If optional argument NO-CONFIRM is non-nil, don't ask for
>+confirmation."
>+ (interactive "P")
>+ (when (or no-confirm
>+ (yes-or-no-p "Permanently delete all bookmarks? "))
>+ (bookmark-maybe-load-default-file)
>+ (setq bookmark-alist-modification-count
>+ (+ bookmark-alist-modification-count (length bookmark-alist)))
>+ (setq bookmark-alist nil)
>+ (bookmark-bmenu-surreptitiously-rebuild-list)
>+ (when (bookmark-time-to-save-p)
>+ (bookmark-save))))
>+
>+
> (defun bookmark-time-to-save-p (&optional final-time)
> "Return t if it is time to save bookmarks to disk, nil otherwise.
> Optional argument FINAL-TIME means this is being called when Emacs
>@@ -1600,12 +1618,15 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
> (define-key map "\C-d" 'bookmark-bmenu-delete-backwards)
> (define-key map "x" 'bookmark-bmenu-execute-deletions)
> (define-key map "d" 'bookmark-bmenu-delete)
>+ (define-key map "D" 'bookmark-bmenu-delete-all)
> (define-key map " " 'next-line)
> (define-key map "n" 'next-line)
> (define-key map "p" 'previous-line)
> (define-key map "\177" 'bookmark-bmenu-backup-unmark)
> (define-key map "u" 'bookmark-bmenu-unmark)
>+ (define-key map "U" 'bookmark-bmenu-unmark-all)
> (define-key map "m" 'bookmark-bmenu-mark)
>+ (define-key map "M" 'bookmark-bmenu-mark-all)
> (define-key map "l" 'bookmark-bmenu-load)
> (define-key map "r" 'bookmark-bmenu-rename)
> (define-key map "R" 'bookmark-bmenu-relocate)
>@@ -1627,8 +1648,10 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
> ["Select Marked Bookmarks" bookmark-bmenu-select t]
> "---"
> ["Mark Bookmark" bookmark-bmenu-mark t]
>+ ["Mark all Bookmarks" bookmark-bmenu-mark-all t]
> ["Unmark Bookmark" bookmark-bmenu-unmark t]
> ["Unmark Backwards" bookmark-bmenu-backup-unmark t]
>+ ["Unmark all Bookmarks" bookmark-bmenu-unmark-all t]
> ["Toggle Display of Filenames" bookmark-bmenu-toggle-filenames t]
> ["Display Location of Bookmark" bookmark-bmenu-locate t]
> "---"
>@@ -1636,6 +1659,7 @@ unique numeric suffixes \"<2>\", \"<3>\", etc."
> ["Rename Bookmark" bookmark-bmenu-rename t]
> ["Relocate Bookmark's File" bookmark-bmenu-relocate t]
> ["Mark Bookmark for Deletion" bookmark-bmenu-delete t]
>+ ["Mark all Bookmarks for Deletion" bookmark-bmenu-delete-all t]
> ["Delete Marked Bookmarks" bookmark-bmenu-execute-deletions t])
> ("Annotations"
> ["Show Annotation for Current Bookmark" bookmark-bmenu-show-annotation t]
>@@ -1748,6 +1772,7 @@ Letters do not insert themselves; instead, they are commands.
> Bookmark names preceded by a \"*\" have annotations.
> \\<bookmark-bmenu-mode-map>
> \\[bookmark-bmenu-mark] -- mark bookmark to be displayed.
>+\\[bookmark-bmenu-mark-all] -- mark all listed bookmarks to be displayed.
> \\[bookmark-bmenu-select] -- select bookmark of line point is on.
> Also show bookmarks marked using m in other windows.
> \\[bookmark-bmenu-toggle-filenames] -- toggle displaying of filenames (they may obscure long bookmark names).
>@@ -1764,13 +1789,15 @@ Bookmark names preceded by a \"*\" have annotations.
> \\[bookmark-bmenu-relocate] -- relocate this bookmark's file (prompts for new file).
> \\[bookmark-bmenu-delete] -- mark this bookmark to be deleted, and move down.
> \\[bookmark-bmenu-delete-backwards] -- mark this bookmark to be deleted, and move up.
>-\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]'.
>+\\[bookmark-bmenu-delete-all] -- mark all listed bookmarks as to be deleted.
>+\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]' or `\\[bookmark-bmenu-delete-all]'.
> \\[bookmark-bmenu-save] -- save the current bookmark list in the default file.
> With a prefix arg, prompts for a file to save in.
> \\[bookmark-bmenu-load] -- load in a file of bookmarks (prompts for file.)
> \\[bookmark-bmenu-unmark] -- remove all kinds of marks from current line.
> With prefix argument, also move up one line.
> \\[bookmark-bmenu-backup-unmark] -- back up a line and remove marks.
>+\\[bookmark-bmenu-unmark-all] -- remove all kinds of marks from all listed bookmarks.
> \\[bookmark-bmenu-show-annotation] -- show the annotation, if it exists, for the current bookmark
> in another buffer.
> \\[bookmark-bmenu-show-all-annotations] -- show the annotations of all bookmarks in another buffer.
>@@ -1937,9 +1964,23 @@ If the annotation does not exist, do nothing."
> (bookmark-bmenu-ensure-position))))
>
>
>+(defun bookmark-bmenu-mark-all ()
>+ "Mark all listed bookmarks to be displayed by \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-select]."
>+ (interactive)
>+ (save-excursion
>+ (goto-char (point-min))
>+ (bookmark-bmenu-ensure-position)
>+ (with-buffer-modified-unmodified
>+ (let ((inhibit-read-only t))
>+ (while (not (eobp))
>+ (delete-char 1)
>+ (insert ?>)
>+ (forward-line 1))))))
>+
>+
> (defun bookmark-bmenu-select ()
> "Select this line's bookmark; also display bookmarks marked with `>'.
>-You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] command."
>+You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] or \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark-all] commands."
> (interactive)
> (let ((bmrk (bookmark-bmenu-bookmark))
> (menu (current-buffer))
>@@ -2108,6 +2149,20 @@ Optional BACKUP means move up."
> (bookmark-bmenu-ensure-position))
>
>
>+(defun bookmark-bmenu-unmark-all ()
>+ "Cancel all requested operations on all listed bookmarks."
>+ (interactive)
>+ (save-excursion
>+ (goto-char (point-min))
>+ (bookmark-bmenu-ensure-position)
>+ (with-buffer-modified-unmodified
>+ (let ((inhibit-read-only t))
>+ (while (not (eobp))
>+ (delete-char 1)
>+ (insert " ")
>+ (forward-line 1))))))
>+
>+
> (defun bookmark-bmenu-delete ()
> "Mark bookmark on this line to be deleted.
> To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
>@@ -2133,6 +2188,22 @@ To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\
> (bookmark-bmenu-ensure-position))
>
>
>+(defun bookmark-bmenu-delete-all ()
>+ "Mark all listed bookmarks as to be deleted.
>+To remove all deletion marks, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-unmark-all].
>+To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
>+ (interactive)
>+ (save-excursion
>+ (goto-char (point-min))
>+ (bookmark-bmenu-ensure-position)
>+ (with-buffer-modified-unmodified
>+ (let ((inhibit-read-only t))
>+ (while (not (eobp))
>+ (delete-char 1)
>+ (insert ?D)
>+ (forward-line 1))))))
>+
>+
> (defun bookmark-bmenu-execute-deletions ()
> "Delete bookmarks flagged `D'."
> (interactive)
>@@ -2292,6 +2363,9 @@ strings returned are not."
> (bindings--define-key map [delete]
> '(menu-item "Delete Bookmark..." bookmark-delete
> :help "Delete a bookmark from the bookmark list"))
>+ (bindings--define-key map [delete-all]
>+ '(menu-item "Delete all Bookmarks..." bookmark-delete-all
>+ :help "Delete all bookmarks from the bookmark list"))
> (bindings--define-key map [rename]
> '(menu-item "Rename Bookmark..." bookmark-rename
> :help "Change the name of a bookmark"))
>diff --git a/test/lisp/bookmark-resources/test-list.bmk b/test/lisp/bookmark-resources/test-list.bmk
>new file mode 100644
>index 0000000000..696d64979b
>--- /dev/null
>+++ b/test/lisp/bookmark-resources/test-list.bmk
>@@ -0,0 +1,20 @@
>+;;;; Emacs Bookmark Format Version 1 ;;;; -*- coding: utf-8-emacs -*-
>+;;; This format is meant to be slightly human-readable;
>+;;; nevertheless, you probably don't want to edit it.
>+;;; -*- End Of Bookmark File Format Version Stamp -*-
>+(("name-0"
>+ (filename . "/some/file-0")
>+ (front-context-string . "abc")
>+ (rear-context-string . "def")
>+ (position . 3))
>+("name-1"
>+ (filename . "/some/file-1")
>+ (front-context-string . "abc")
>+ (rear-context-string . "def")
>+ (position . 3))
>+("name-2"
>+ (filename . "/some/file-2")
>+ (front-context-string . "abc")
>+ (rear-context-string . "def")
>+ (position . 3))
>+)
>diff --git a/test/lisp/bookmark-tests.el b/test/lisp/bookmark-tests.el
>index b9c6ff9c54..c5959e46d8 100644
>--- a/test/lisp/bookmark-tests.el
>+++ b/test/lisp/bookmark-tests.el
>@@ -83,6 +83,70 @@ the lexically-bound variable `buffer'."
> ,@body)
> (kill-buffer buffer))))
>
>+(defvar bookmark-tests-bookmark-file-list
>+ (expand-file-name "test-list.bmk" bookmark-tests-data-dir)
>+ "Bookmark file used for testing a list of bookmarks.")
>+
>+;; The values below should match `bookmark-tests-bookmark-file-list'
>+;; content. We cache these values to speed up tests.
>+(eval-and-compile ; needed by `with-bookmark-test-list' macro
>+ (defvar bookmark-tests-bookmark-list-0 '("name-0"
>+ (filename . "/some/file-0")
>+ (front-context-string . "ghi")
>+ (rear-context-string . "jkl")
>+ (position . 4))
>+ "Cached value used in bookmark-tests.el."))
>+
>+;; The values below should match `bookmark-tests-bookmark-file-list'
>+;; content. We cache these values to speed up tests.
>+(eval-and-compile ; needed by `with-bookmark-test-list' macro
>+ (defvar bookmark-tests-bookmark-list-1 '("name-1"
>+ (filename . "/some/file-1")
>+ (front-context-string . "mno")
>+ (rear-context-string . "pqr")
>+ (position . 5))
>+ "Cached value used in bookmark-tests.el."))
>+
>+;; The values below should match `bookmark-tests-bookmark-file-list'
>+;; content. We cache these values to speed up tests.
>+(eval-and-compile ; needed by `with-bookmark-test-list' macro
>+ (defvar bookmark-tests-bookmark-list-2 '("name-2"
>+ (filename . "/some/file-2")
>+ (front-context-string . "stu")
>+ (rear-context-string . "vwx")
>+ (position . 6))
>+ "Cached value used in bookmark-tests.el."))
>+
>+(defvar bookmark-tests-cache-timestamp-list
>+ (cons bookmark-tests-bookmark-file-list
>+ (nth 5 (file-attributes
>+ bookmark-tests-bookmark-file-list)))
>+ "Cached value used in bookmark-tests.el.")
>+
>+(defmacro with-bookmark-test-list (&rest body)
>+ "Create environment for testing bookmark.el and evaluate BODY.
>+Ensure a clean environment for testing, and do not change user
>+data when running tests interactively."
>+ `(with-temp-buffer
>+ (let ((bookmark-alist (quote (,(copy-sequence bookmark-tests-bookmark-list-0)
>+ ,(copy-sequence bookmark-tests-bookmark-list-1)
>+ ,(copy-sequence bookmark-tests-bookmark-list-2))))
>+ (bookmark-default-file bookmark-tests-bookmark-file-list)
>+ (bookmark-bookmarks-timestamp bookmark-tests-cache-timestamp-list)
>+ bookmark-save-flag)
>+ ,@body)))
>+
>+(defmacro with-bookmark-test-file-list (&rest body)
>+ "Create environment for testing bookmark.el and evaluate BODY.
>+Same as `with-bookmark-test-list' but also opens the resource file
>+example.txt in a buffer, which can be accessed by callers through
>+the lexically-bound variable `buffer'."
>+ `(let ((buffer (find-file-noselect bookmark-tests-example-file)))
>+ (unwind-protect
>+ (with-bookmark-test-list
>+ ,@body)
>+ (kill-buffer buffer))))
>+
> (ert-deftest bookmark-tests-all-names ()
> (with-bookmark-test
> (should (equal (bookmark-all-names) '("name")))))
>@@ -95,6 +159,30 @@ the lexically-bound variable `buffer'."
> (with-bookmark-test
> (should (equal (bookmark-get-bookmark-record "name") (cdr bookmark-tests-bookmark)))))
>
>+(ert-deftest bookmark-tests-all-names-list ()
>+ (with-bookmark-test-list
>+ (should (equal (bookmark-all-names) '("name-0"
>+ "name-1"
>+ "name-2")))))
>+
>+(ert-deftest bookmark-tests-get-bookmark-list ()
>+ (with-bookmark-test-list
>+ (should (equal (bookmark-get-bookmark "name-0")
>+ bookmark-tests-bookmark-list-0))
>+ (should (equal (bookmark-get-bookmark "name-1")
>+ bookmark-tests-bookmark-list-1))
>+ (should (equal (bookmark-get-bookmark "name-2")
>+ bookmark-tests-bookmark-list-2))))
>+
>+(ert-deftest bookmark-tests-get-bookmark-record-list ()
>+ (with-bookmark-test-list
>+ (should (equal (bookmark-get-bookmark-record "name-0")
>+ (cdr bookmark-tests-bookmark-list-0)))
>+ (should (equal (bookmark-get-bookmark-record "name-1")
>+ (cdr bookmark-tests-bookmark-list-1)))
>+ (should (equal (bookmark-get-bookmark-record "name-2")
>+ (cdr bookmark-tests-bookmark-list-2)))))
>+
> (ert-deftest bookmark-tests-record-getters-and-setters-new ()
> (with-temp-buffer
> (let* ((buffer-file-name "test")
>@@ -130,6 +218,19 @@ the lexically-bound variable `buffer'."
> ;; calling twice gives same record
> (should (equal (bookmark-make-record) record))))))
>
>+(ert-deftest bookmark-tests-make-record-list ()
>+ (with-bookmark-test-file-list
>+ (let* ((record `("example.txt" (filename . ,bookmark-tests-example-file)
>+ (front-context-string . "is text file is ")
>+ (rear-context-string)
>+ (position . 3)
>+ (defaults "example.txt"))))
>+ (with-current-buffer buffer
>+ (goto-char 3)
>+ (should (equal (bookmark-make-record) record))
>+ ;; calling twice gives same record
>+ (should (equal (bookmark-make-record) record))))))
>+
> (ert-deftest bookmark-tests-make-record-function ()
> (with-bookmark-test
> (let ((buffer-file-name "test"))
>@@ -267,6 +368,11 @@ the lexically-bound variable `buffer'."
> (bookmark-delete "name")
> (should (equal bookmark-alist nil))))
>
>+(ert-deftest bookmark-tests-delete-all ()
>+ (with-bookmark-test-list
>+ (bookmark-delete-all t)
>+ (should (equal bookmark-alist nil))))
>+
> (defmacro with-bookmark-test-save-load (&rest body)
> "Create environment for testing bookmark.el and evaluate BODY.
> Same as `with-bookmark-test' but also sets a temporary
>@@ -340,6 +446,18 @@ testing `bookmark-bmenu-list'."
> ,@body)
> (kill-buffer bookmark-bmenu-buffer)))))
>
>+(defmacro with-bookmark-bmenu-test-list (&rest body)
>+ "Create environment for testing `bookmark-bmenu-list' and evaluate BODY.
>+Same as `with-bookmark-test-list' but with additions suitable for
>+testing `bookmark-bmenu-list'."
>+ `(with-bookmark-test-list
>+ (let ((bookmark-bmenu-buffer "*Bookmark List - Testing*"))
>+ (unwind-protect
>+ (save-window-excursion
>+ (bookmark-bmenu-list)
>+ ,@body)
>+ (kill-buffer bookmark-bmenu-buffer)))))
>+
> (ert-deftest bookmark-test-bmenu-edit-annotation/show-annotation ()
> (with-bookmark-bmenu-test
> (bookmark-set-annotation "name" "foo")
>@@ -402,6 +520,52 @@ testing `bookmark-bmenu-list'."
> (beginning-of-line)
> (should (bookmark-bmenu-any-marks))))
>
>+(ert-deftest bookmark-test-bmenu-mark-all ()
>+ (with-bookmark-bmenu-test-list
>+ (let ((here (point-max)))
>+ ;; Expect to not move the point
>+ (goto-char here)
>+ (bookmark-bmenu-mark-all)
>+ (should (equal here (point)))
>+ ;; Verify that all bookmarks are marked
>+ (goto-char (point-min))
>+ (bookmark-bmenu-ensure-position)
>+ (should (looking-at "^> "))
>+ (should (equal bookmark-tests-bookmark-list-0
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ (forward-line 1)
>+ (should (looking-at "^> "))
>+ (should (equal bookmark-tests-bookmark-list-1
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ (forward-line 1)
>+ (should (looking-at "^> "))
>+ (should (equal bookmark-tests-bookmark-list-2
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark)))))))
>+
>+(ert-deftest bookmark-test-bmenu-any-marks-list ()
>+ (with-bookmark-bmenu-test-list
>+ ;; Mark just the second item
>+ (goto-char (point-min))
>+ (bookmark-bmenu-ensure-position)
>+ (forward-line 1)
>+ (bookmark-bmenu-mark)
>+ ;; Verify that only the second item is marked
>+ (goto-char (point-min))
>+ (bookmark-bmenu-ensure-position)
>+ (should (looking-at "^ "))
>+ (should (equal bookmark-tests-bookmark-list-0
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ (forward-line 1)
>+ (should (looking-at "^> "))
>+ (should (equal bookmark-tests-bookmark-list-1
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ (forward-line 1)
>+ (should (looking-at "^ "))
>+ (should (equal bookmark-tests-bookmark-list-2
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ ;; There should be at least one mark
>+ (should (bookmark-bmenu-any-marks))))
>+
> (ert-deftest bookmark-test-bmenu-unmark ()
> (with-bookmark-bmenu-test
> (bookmark-bmenu-mark)
>@@ -410,12 +574,63 @@ testing `bookmark-bmenu-list'."
> (beginning-of-line)
> (should (looking-at "^ "))))
>
>+(ert-deftest bookmark-test-bmenu-unmark-all ()
>+ (with-bookmark-bmenu-test-list
>+ (bookmark-bmenu-mark-all)
>+ (let ((here (point-max)))
>+ ;; Expect to not move the point
>+ (goto-char here)
>+ (bookmark-bmenu-unmark-all)
>+ (should (equal here (point)))
>+ ;; Verify that all bookmarks are unmarked
>+ (goto-char (point-min))
>+ (bookmark-bmenu-ensure-position)
>+ (should (looking-at "^ "))
>+ (should (equal bookmark-tests-bookmark-list-0
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ (forward-line 1)
>+ (should (looking-at "^ "))
>+ (should (equal bookmark-tests-bookmark-list-1
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ (forward-line 1)
>+ (should (looking-at "^ "))
>+ (should (equal bookmark-tests-bookmark-list-2
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark)))))))
>+
> (ert-deftest bookmark-test-bmenu-delete ()
> (with-bookmark-bmenu-test
> (bookmark-bmenu-delete)
> (bookmark-bmenu-execute-deletions)
> (should (equal (length bookmark-alist) 0))))
>
>+(ert-deftest bookmark-test-bmenu-delete-all ()
>+ (with-bookmark-bmenu-test-list
>+ ;; Verify that unmarked bookmarks aren't deleted
>+ (bookmark-bmenu-execute-deletions)
>+ (should-not (eq bookmark-alist nil))
>+ (let ((here (point-max)))
>+ ;; Expect to not move the point
>+ (goto-char here)
>+ (bookmark-bmenu-delete-all)
>+ (should (equal here (point)))
>+ ;; Verify that all bookmarks are marked for deletion
>+ (goto-char (point-min))
>+ (bookmark-bmenu-ensure-position)
>+ (should (looking-at "^D "))
>+ (should (equal bookmark-tests-bookmark-list-0
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ (forward-line 1)
>+ (should (looking-at "^D "))
>+ (should (equal bookmark-tests-bookmark-list-1
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ (forward-line 1)
>+ (should (looking-at "^D "))
>+ (should (equal bookmark-tests-bookmark-list-2
>+ (bookmark-get-bookmark (bookmark-bmenu-bookmark))))
>+ ;; Verify that all bookmarks are deleted
>+ (bookmark-bmenu-execute-deletions)
>+ (should (eq bookmark-alist nil)))))
>+
> (ert-deftest bookmark-test-bmenu-locate ()
> (let (msg)
> (cl-letf (((symbol-function 'message)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-08-07 1:10 ` Karl Fogel
@ 2020-08-07 8:33 ` Matthew White
[not found] ` <jwveeojyxh1.fsf-monnier+emacs@gnu.org>
1 sibling, 0 replies; 52+ messages in thread
From: Matthew White @ 2020-08-07 8:33 UTC (permalink / raw)
To: Karl Fogel; +Cc: Lars Ingebrigtsen, Eli Zaretskii, Stefan Monnier, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 1276 bytes --]
On Thu, 06 Aug 2020 20:10:43 -0500
Karl Fogel wrote:
> >Karl, I waited to answer you in the hope of getting the paperwork...
> >Still nothing here... I don't know if the FSF already registered my
> >copyright assignment request, they have my signature, though.
>
> I'll ask about it in a separate thread, no problem. Maybe it's all
> done and we just didn't get the news yet.
Thank you! I got the CC of the email you sent.
> >About the patch, your help is priceless, Karl. I have no trouble to
> >attach an amended patch, what pressures me is to give you the right
> >credits! In these days I was thinking how you make me look smarter
> >than I really am...
> >
> >Do as you feel, Karl. The attached patch is my attempt to give you
> >credit, but you are free to do a follow-up commit, if you prefer.
>
> Aw, Matthew, thanks, but you did all the hard work here! If you're
> okay with it, when I apply the commit I'll just change your commit
> message to include a simple phrase like "Thanks to Karl Fogel for
> pre-commit review" or something like that.
>
> Best regards,
> -Karl
I'm comfortable with your judgment, feel free to amend the commit.
At this point, we wait and see about the paperwork...
Best regards,
-Matthew
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
[not found] ` <87a6z715do.fsf@red-bean.com>
@ 2020-08-07 17:05 ` Matthew White
2020-08-07 17:18 ` Eli Zaretskii
2020-08-09 20:19 ` Karl Fogel
0 siblings, 2 replies; 52+ messages in thread
From: Matthew White @ 2020-08-07 17:05 UTC (permalink / raw)
To: Karl Fogel; +Cc: Lars Ingebrigtsen, Eli Zaretskii, Stefan Monnier, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]
On Thu, 06 Aug 2020 22:50:27 -0500
Karl Fogel wrote:
> On 06 Aug 2020, Stefan Monnier wrote:
> >>>Karl, I waited to answer you in the hope of getting the
> >>>paperwork... Still nothing here... I don't know if the FSF
> >>>already registered my copyright assignment request, they have my
> >>>signature, though.
> >>
> >> I'll ask about it in a separate thread, no problem. Maybe it's
> >> all done and we just didn't get the news yet.
> >
> >FWIW, I only see your name for `wget` for which you apparently signed
> >4 years ago. So, it looks like your paperwork is still "in progress"
> >(no idea at which stage it is; you might like to ask
> >`assign@gnu.org`).
>
> Thanks for watching this thread, Stefan.
>
> I'll send a inquiry mail to assign@gnu.org and CC Matthew.
>
> Best regards,
> -Karl
The copyright assignment should be complete now! I believe the commit
could be merged now...
Thank you all for making my work worthy!
Best regards,
-Matthew
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-08-07 17:05 ` Matthew White
@ 2020-08-07 17:18 ` Eli Zaretskii
2020-08-08 8:23 ` Matthew White
2020-08-09 20:19 ` Karl Fogel
1 sibling, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2020-08-07 17:18 UTC (permalink / raw)
To: Matthew White; +Cc: kfogel, larsi, monnier, emacs-devel
> Date: Fri, 7 Aug 2020 19:05:51 +0200
> From: Matthew White <mehw.is.me@inventati.org>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Eli Zaretskii <eliz@gnu.org>,
> Lars Ingebrigtsen <larsi@gnus.org>, emacs-devel@gnu.org
>
> The copyright assignment should be complete now! I believe the commit
> could be merged now...
I still don't see your assignment on file, so let's wait for a couple
of days for it to appear.
Thanks.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-08-07 17:18 ` Eli Zaretskii
@ 2020-08-08 8:23 ` Matthew White
0 siblings, 0 replies; 52+ messages in thread
From: Matthew White @ 2020-08-08 8:23 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: kfogel, larsi, monnier, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 695 bytes --]
On Fri, 07 Aug 2020 20:18:18 +0300
Eli Zaretskii wrote:
> > Date: Fri, 7 Aug 2020 19:05:51 +0200
> > From: Matthew White
> > Cc: Stefan Monnier, Eli Zaretskii, Lars Ingebrigtsen
> >
> > The copyright assignment should be complete now! I believe the
> > commit could be merged now...
>
> I still don't see your assignment on file, so let's wait for a couple
> of days for it to appear.
>
> Thanks.
Could you check for [gnu.org #976762] too, please. Stefan wrote me
about it... it was an assignment I made in 2015 for company-mode.
I don't know if the company-mode assignment applies to Emacs too.
The new assignment is [gnu.org #1578599].
Best regards,
-Matthew
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-08-07 17:05 ` Matthew White
2020-08-07 17:18 ` Eli Zaretskii
@ 2020-08-09 20:19 ` Karl Fogel
2020-08-10 5:41 ` Matthew White
1 sibling, 1 reply; 52+ messages in thread
From: Karl Fogel @ 2020-08-09 20:19 UTC (permalink / raw)
To: Matthew White
Cc: Lars Ingebrigtsen, Eli Zaretskii, Stefan Monnier, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 548 bytes --]
On 07 Aug 2020, Matthew White wrote:
>The copyright assignment should be complete now! I believe the commit
>could be merged now...
>
>Thank you all for making my work worthy!
Confirmed -- I heard from the FSF copyright clerk that Matthew's assignment paperwork is complete.
Accordingly, I retested the change against the latest sources (just to be sure) and then pushed it:
http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=450644e9f7923c30ef3d885638e69d3263bfa3a8
Thank you for the excellent contribution, Matthew!
Best regards,
-Karl
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Add new functions to mark/unmark/delete all bookmarks
2020-08-09 20:19 ` Karl Fogel
@ 2020-08-10 5:41 ` Matthew White
0 siblings, 0 replies; 52+ messages in thread
From: Matthew White @ 2020-08-10 5:41 UTC (permalink / raw)
To: Karl Fogel; +Cc: Lars Ingebrigtsen, Eli Zaretskii, Stefan Monnier, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 744 bytes --]
On Sun, 09 Aug 2020 15:19:07 -0500
Karl Fogel wrote:
> On 07 Aug 2020, Matthew White wrote:
> >The copyright assignment should be complete now! I believe the commit
> >could be merged now...
> >
> >Thank you all for making my work worthy!
>
> Confirmed -- I heard from the FSF copyright clerk that Matthew's
> assignment paperwork is complete.
>
> Accordingly, I retested the change against the latest sources (just
> to be sure) and then pushed it:
>
> http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=450644e9f7923c30ef3d885638e69d3263bfa3a8
>
> Thank you for the excellent contribution, Matthew!
>
> Best regards,
> -Karl
Hi Karl,
Thanks for reviewing and polishing the commit.
Cheers,
-Matthew
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
end of thread, other threads:[~2020-08-10 5:41 UTC | newest]
Thread overview: 52+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-23 23:00 Add new functions to mark/unmark/delete all bookmarks Matthew White
2020-07-24 3:15 ` Drew Adams
2020-07-24 9:07 ` Matthew White
2020-07-24 15:07 ` Stefan Monnier
2020-07-24 15:18 ` Lars Ingebrigtsen
2020-07-24 17:03 ` Matthew White
2020-07-24 17:09 ` Stefan Monnier
2020-07-28 15:54 ` Matthew White
2020-07-28 16:14 ` Stefan Monnier
2020-07-28 16:36 ` Matthew White
2020-07-28 18:06 ` Eli Zaretskii
2020-07-28 18:15 ` Matthew White
2020-07-24 17:07 ` Drew Adams
2020-07-24 17:07 ` Drew Adams
2020-07-24 16:43 ` Drew Adams
2020-07-24 5:31 ` Yuri Khan
2020-07-24 15:05 ` Stefan Monnier
2020-07-24 16:56 ` Noam Postavsky
2020-07-24 19:00 ` Extend tabulated-list-mode to support marks Yuri Khan
2020-07-25 13:46 ` Dirk-Jan C. Binnema
2020-07-25 14:23 ` Stefan Monnier
2020-07-25 14:32 ` Yuri Khan
2020-07-26 12:43 ` Stefan Kangas
2020-07-24 17:07 ` Add new functions to mark/unmark/delete all bookmarks Drew Adams
2020-07-24 19:07 ` Karl Fogel
2020-07-25 10:46 ` Matthew White
2020-07-25 10:58 ` Eli Zaretskii
2020-07-25 15:33 ` Michael Albinus
2020-07-25 15:38 ` Eli Zaretskii
2020-07-25 15:56 ` Michael Albinus
2020-07-25 16:30 ` Eli Zaretskii
2020-07-25 16:33 ` Eli Zaretskii
2020-07-25 16:43 ` Michael Albinus
2020-07-25 16:55 ` Eli Zaretskii
2020-07-25 18:28 ` Matthew White
2020-07-25 18:54 ` Eli Zaretskii
2020-07-25 20:36 ` Karl Fogel
2020-07-25 21:14 ` Stefan Monnier
2020-07-25 22:40 ` Karl Fogel
2020-07-29 22:16 ` Karl Fogel
2020-07-31 2:58 ` Matthew White
2020-07-31 6:01 ` Karl Fogel
2020-08-02 22:13 ` Karl Fogel
2020-08-06 17:59 ` Matthew White
2020-08-07 1:10 ` Karl Fogel
2020-08-07 8:33 ` Matthew White
[not found] ` <jwveeojyxh1.fsf-monnier+emacs@gnu.org>
[not found] ` <87a6z715do.fsf@red-bean.com>
2020-08-07 17:05 ` Matthew White
2020-08-07 17:18 ` Eli Zaretskii
2020-08-08 8:23 ` Matthew White
2020-08-09 20:19 ` Karl Fogel
2020-08-10 5:41 ` Matthew White
[not found] <<20200724005105.11f85d5f@pineapple>
[not found] ` <<87pn8ku3y9.fsf@red-bean.com>
[not found] ` <<20200725124618.49a073b1@pineapple>
[not found] ` <<83lfj7dfp2.fsf@gnu.org>
[not found] ` <<87tuxvpq1r.fsf@gmx.de>
[not found] ` <<83d04jd2pm.fsf@gnu.org>
2020-07-25 16:07 ` Drew Adams
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).