GNU bug report logs - #10998
Allow movements in bookmark-bmenu-search.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 10998 in the body.
You can then email your comments to 10998 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Allow movements in bookmark-bmenu-search.
Date: Mon, 12 Mar 2012 07:08:04 +0100

Hi,
there is no cursor in the prompt of bookmark-bmenu-search, and it is not
possible to move and edit it actually, this patch allow this.

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/bookmark.el b/lisp/bookmark.el
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -169,6 +169,11 @@
   :group 'bookmark
   :version "22.1")
 
+(defface bookmark-cursor
+  '((t (:foreground "green")))
+  "Face for cursor color in `bookmark-bmenu-search' prompt."
+  :group 'bookmark)
+
 
 ;;; No user-serviceable parts beyond this point.
 
@@ -2008,20 +2013,57 @@
 ;; Store keyboard input for incremental search.
 (defvar bookmark-search-pattern)
 
+(defun bookmark-set-cursor-in-prompt (str pos)
+  "Add a cursor in string STR at index position POS."
+  (setq pos (min index (1- (length tmp-list))))
+  (when (not (string= str ""))
+    (let* ((real-index (- (1- (length tmp-list)) pos))
+           (cur-str (substring str real-index (1+ real-index))))
+      (concat (substring str 0 real-index)
+              (propertize cur-str 'display
+                          (if (= index (length tmp-list))
+                              (concat
+                               (propertize "|" 'face 'bookmark-cursor)
+                               cur-str)
+                            (concat
+                             cur-str
+                             (propertize "|" 'face 'bookmark-cursor))))
+              (substring str (1+ real-index))))))
+
 (defun bookmark-read-search-input ()
   "Read each keyboard input and add it to `bookmark-search-pattern'."
   (let ((prompt       (propertize "Pattern: " 'face 'minibuffer-prompt))
         ;; (inhibit-quit t) ; inhibit-quit is evil.  Use it with extreme care!
-        (tmp-list     ()))
+        (tmp-list     ())
+        (index        0))
     (while
-        (let ((char (read-key (concat prompt bookmark-search-pattern))))
+        (let ((char (read-key (concat prompt (bookmark-set-cursor-in-prompt
+                                              bookmark-search-pattern index)))))
           (case char
             ((?\e ?\r) nil) ; RET or ESC break the search loop.
             (?\C-g (setq bookmark-quit-flag t) nil)
-            (?\d (pop tmp-list) t) ; Delete last char of pattern with DEL
+            (?\d (with-no-warnings ; Delete last char of pattern with DEL.
+                   (pop (nthcdr index tmp-list))) t)
+            ;; Movements in minibuffer.
+            (?\C-b                         ; backward-char.
+             (setq index (min (1+ index) (length tmp-list))) t)
+            (?\C-f                         ; forward-char.
+             (setq index (max (1- index) 0)) t)
+            (?\C-a                         ; move bol.
+             (setq index (length tmp-list)) t)
+            (?\C-e                         ; move eol.
+             (setq index 0) t)
+            (?\C-k
+             (kill-new (substring bookmark-search-pattern
+                                  (- (length tmp-list) index)))
+             (setq tmp-list (nthcdr index tmp-list)) (setq index 0) t)
+            (?\C-y
+             (let ((str (car kill-ring)))
+               (loop for char across str
+                     do (push char (nthcdr index tmp-list)))) t)
             (t
              (if (characterp char)
-                 (push char tmp-list)
+                 (push char (nthcdr index tmp-list))
                (setq unread-command-events
                      (nconc (mapcar 'identity
                                     (this-single-command-raw-keys))
--8<---------------cut here---------------end--------------->8---

-- 
  Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 

Message #8 received at 10998 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 10998 <at> debbugs.gnu.org
Subject: Re: bug#10998: Allow movements in bookmark-bmenu-search.
Date: Mon, 12 Mar 2012 08:31:32 -0400

> there is no cursor in the prompt of bookmark-bmenu-search, and it is not
> possible to move and edit it actually, this patch allow this.

Couldn't we fix that another way, using a real minibuffer, with an
after-change-function to update the set of matching entries?


        Stefan

Message #11 received at 10998 <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 10998 <at> debbugs.gnu.org
Subject: Re: bug#10998: Allow movements in bookmark-bmenu-search.
Date: Mon, 12 Mar 2012 13:49:07 +0100

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> there is no cursor in the prompt of bookmark-bmenu-search, and it is not
>> possible to move and edit it actually, this patch allow this.
>
> Couldn't we fix that another way, using a real minibuffer, with an
> after-change-function to update the set of matching entries?
I don't know much about after-change-functions, but I guess yes it would
be possible.
However it will change all the logic of this code and need a complete
rewrite.

BTW the patch sent contain free-variables, it is fixed here.

-- 
  Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 

Message #14 received at 10998 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 10998 <at> debbugs.gnu.org
Subject: Re: bug#10998: Allow movements in bookmark-bmenu-search.
Date: Mon, 12 Mar 2012 11:52:43 -0400

>>> there is no cursor in the prompt of bookmark-bmenu-search, and it is not
>>> possible to move and edit it actually, this patch allow this.
>> Couldn't we fix that another way, using a real minibuffer, with an
>> after-change-function to update the set of matching entries?
> I don't know much about after-change-functions, but I guess yes it
> would be possible.  However it will change all the logic of this code
> and need a complete rewrite.

Indeed, but it will make all cursor motion commands work "as usual"
rather than hard-coding C-f, C-b etc...


        Stefan

Message #17 received at 10998 <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 10998 <at> debbugs.gnu.org
Subject: Re: bug#10998: Allow movements in bookmark-bmenu-search.
Date: Tue, 13 Mar 2012 16:34:24 +0100

Hi Stefan,

Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:

>>>> there is no cursor in the prompt of bookmark-bmenu-search, and it is not
>>>> possible to move and edit it actually, this patch allow this.
>>> Couldn't we fix that another way, using a real minibuffer, with an
>>> after-change-function to update the set of matching entries?
>> I don't know much about after-change-functions, but I guess yes it
>> would be possible.  However it will change all the logic of this code
>> and need a complete rewrite.
>
> Indeed, but it will make all cursor motion commands work "as usual"
> rather than hard-coding C-f, C-b etc...
Agree, but I will not have the time to work on this, at least now.
I suggest you use the code I sent you (after 24.1 I guess) waiting 
something better. (I have a better version here).
You can see how it works by using last (dev version) of ioccur.el.

Thanks.

-- 
  Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 

Message #20 received at 10998 <at> debbugs.gnu.org (full text, mbox):

From: Karl Fogel <kfogel <at> openitp.org>
To: 10998 <at> debbugs.gnu.org
Subject: Re: Allow movements in bookmark-bmenu-search
Date: Sun, 30 Sep 2012 23:35:03 -0500

The code has changed somewhat since the original patch -- it now uses
`pcase' instead of `case', for example (and thus `_' instead of `t'),
ever since monnier <at> iro.umontreal.ca-20120710115154-012cs2xbndtlpvh1.

Furthermore, in http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10998#11
Thierry said "the patch sent contain free-variables, it is fixed here"
but there was no new patch attached.

So, I've adjusted the original patch (see below) and done some light
testing, but I'm not confident enough in this to commit it yet.
Thierry, what were those free variables?  And can you test this to make
sure it behaves as your original patch did?

I agree with Stefan that this is not an ideal solution, by the way, and
have left a comment in the patch below to that effect.  It might still
be better than the status quo, though.

-Karl

=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog	2012-10-01 03:44:59 +0000
+++ lisp/ChangeLog	2012-10-01 04:31:27 +0000
@@ -1,3 +1,11 @@
+2012-10-01  Karl Fogel  <kfogel <at> openitp.org>
+
+	* bookmark.el (bookmark-cursor, bookmark-set-cursor-in-prompt):
+	New face, new function.
+	(bookmark-read-search-input): Set the cursor and listen to more
+	command characters.
+	(Bug#10998)
+
 2012-10-01  Karl Fogel  <kfogel <at> red-bean.com>
 
 	* bookmark.el (bookmark-version-control): Give tags in the

=== modified file 'lisp/bookmark.el'
--- lisp/bookmark.el	2012-10-01 04:15:48 +0000
+++ lisp/bookmark.el	2012-10-01 04:34:25 +0000
@@ -168,6 +168,11 @@
   :group 'bookmark
   :version "22.1")
 
+(defface bookmark-cursor
+  '((t (:foreground "green")))
+  "Face for cursor color in `bookmark-bmenu-search' prompt."
+  :group 'bookmark)
+
 
 ;;; No user-serviceable parts beyond this point.
 
@@ -2013,20 +2018,62 @@
 ;; Store keyboard input for incremental search.
 (defvar bookmark-search-pattern)
 
+(defun bookmark-set-cursor-in-prompt (str pos)
+  "Add a cursor in string STR at index position POS."
+  (setq pos (min index (1- (length tmp-list))))
+  (when (not (string= str ""))
+    (let* ((real-index (- (1- (length tmp-list)) pos))
+           (cur-str (substring str real-index (1+ real-index))))
+      (concat (substring str 0 real-index)
+              (propertize cur-str 'display
+                          (if (= index (length tmp-list))
+                              (concat
+                               (propertize "|" 'face 'bookmark-cursor)
+                               cur-str)
+                            (concat
+                             cur-str
+                             (propertize "|" 'face 'bookmark-cursor))))
+              (substring str (1+ real-index))))))
+
 (defun bookmark-read-search-input ()
   "Read each keyboard input and add it to `bookmark-search-pattern'."
   (let ((prompt       (propertize "Pattern: " 'face 'minibuffer-prompt))
         ;; (inhibit-quit t) ; inhibit-quit is evil.  Use it with extreme care!
-        (tmp-list     ()))
+        (tmp-list     ())
+        (index        0))
     (while
-        (let ((char (read-key (concat prompt bookmark-search-pattern))))
+        (let ((char (read-key
+                     (concat prompt (bookmark-set-cursor-in-prompt
+                                     bookmark-search-pattern index)))))
+          ;; FIXME: This is kind of a kluge.  The right way to do this
+          ;; would be to use a real minibuffer, maybe with an
+          ;; after-change-function to update the set of matching
+          ;; entries each time.
           (pcase char
             ((or ?\e ?\r) nil) ; RET or ESC break the search loop.
             (?\C-g (setq bookmark-quit-flag t) nil)
-            (?\d (pop tmp-list) t) ; Delete last char of pattern with DEL
+            (?\d (with-no-warnings ; Delete last char of pattern with DEL.
+                   (pop (nthcdr index tmp-list))) t)
+            ;; Movements in minibuffer.
+            (?\C-b                         ; backward-char.
+             (setq index (min (1+ index) (length tmp-list))) t)
+            (?\C-f                         ; forward-char.
+             (setq index (max (1- index) 0)) t)
+            (?\C-a                         ; move bol.
+             (setq index (length tmp-list)) t)
+            (?\C-e                         ; move eol.
+             (setq index 0) t)
+            (?\C-k
+             (kill-new (substring bookmark-search-pattern
+                                  (- (length tmp-list) index)))
+             (setq tmp-list (nthcdr index tmp-list)) (setq index 0) t)
+            (?\C-y
+             (let ((str (car kill-ring)))
+               (loop for char across str
+                     do (push char (nthcdr index tmp-list)))) t)
             (_
              (if (characterp char)
-                 (push char tmp-list)
+                 (push char (nthcdr index tmp-list))
                (setq unread-command-events
                      (nconc (mapcar 'identity
                                     (this-single-command-raw-keys))

Message #23 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#10998: Allow movements in bookmark-bmenu-search
Date: Mon, 01 Oct 2012 09:45:42 +0200

Karl Fogel <kfogel <at> openitp.org> writes:

> The code has changed somewhat since the original patch -- it now uses
> `pcase' instead of `case', for example (and thus `_' instead of `t'),
> ever since monnier <at> iro.umontreal.ca-20120710115154-012cs2xbndtlpvh1.
>
> Furthermore, in http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10998#11
> Thierry said "the patch sent contain free-variables, it is fixed here"
> but there was no new patch attached.
It is fixed "here" mean I fixed this on my own version of bookmark but
didn't send any patch because no body was interested by this change.
It is working with no problem since then.

> So, I've adjusted the original patch (see below) and done some light
> testing, but I'm not confident enough in this to commit it yet.
> Thierry, what were those free variables?
Don't remember that was more than one year ago.


> And can you test this to make sure it behaves as your original patch
> did?
I have not anymore this patch.
You will find bookmark-extensions.el at:
https://github.com/thierryvolpiatto/emacs-bmk-ext.git
Install it and try cursor movements to see if it behave like your patch.

Here the function:


#+BEGIN_SRC lisp
(defun bookmark-read-search-input ()
  "Read each keyboard input and add it to `bookmark-search-pattern'."
  (let ((prompt       (propertize "Pattern: " 'face 'minibuffer-prompt))
        (tmp-list     ())
        (index        0))
    (while
        (let ((char (read-key (concat prompt (bookmark-set-cursor-in-prompt
                                              bookmark-search-pattern
                                              index tmp-list)))))
          (case char
            ((?\e ?\r) nil) ; RET or ESC break the search loop.
            (?\C-g (setq bookmark-quit-flag t) nil)
            (?\d (with-no-warnings ; Delete last char of pattern with DEL.
                   (pop (nthcdr index tmp-list))) t)
            ;; Movements in minibuffer.
            (?\C-b                         ; backward-char.
             (setq index (min (1+ index) (length tmp-list))) t)
            (?\C-f                         ; forward-char.
             (setq index (max (1- index) 0)) t)
            (?\C-a                         ; move bol.
             (setq index (length tmp-list)) t)
            (?\C-e                         ; move eol.
             (setq index 0) t)
            (?\C-k
             (kill-new (substring bookmark-search-pattern
                                  (- (length tmp-list) index)))
             (setq tmp-list (nthcdr index tmp-list)) (setq index 0) t)
            (?\C-y
             (let ((str (car kill-ring)))
               (loop for char across str
                     do (push char (nthcdr index tmp-list)))) t)
            (t
             (if (characterp char)
                 (push char (nthcdr index tmp-list))
               (setq unread-command-events
                     (nconc (mapcar 'identity
                                    (this-single-command-raw-keys))
                            unread-command-events))
               nil))))
      (setq bookmark-search-pattern
            (apply 'string (reverse tmp-list))))))

#+END_SRC

Note: I don't maintain anymore bookmark-extensions.el, only small
bugfixes for my personal usage (e.g recently `bookmark-write-file')

> I agree with Stefan that this is not an ideal solution, by the way, and
> have left a comment in the patch below to that effect.
Keep in mind that this is not a real minibuffer.


-- 
  Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 

Message #26 received at 10998 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 10998 <at> debbugs.gnu.org
Subject: Re: bug#10998: Allow movements in bookmark-bmenu-search
Date: Mon, 01 Oct 2012 10:53:47 -0400

>> I agree with Stefan that this is not an ideal solution, by the way, and
>> have left a comment in the patch below to that effect.
> Keep in mind that this is not a real minibuffer.

AFAICT the fact that it's not a minibuffer is an implementation detail.
IOW it currently isn't a minibuffer, but it could/should be changed to
be one.
See example below.


        Stefan


(defun bookmark-bmenu-search ()
  "Incremental search of bookmarks, hiding the non-matches as we go."
  (interactive)
  (let ((bmk (bookmark-bmenu-bookmark))
        (timer nil))
    (unwind-protect
        (minibuffer-with-setup-hook
            (lambda ()
              (setq timer (run-with-idle-timer
                           bookmark-search-delay 'repeat
                           #'(lambda (buf)
                               (with-current-buffer buf
                                 (bookmark-bmenu-filter-alist-by-regexp
                                  (minibuffer-contents))))
                           (current-buffer))))
          (read-string "Pattern: ")
          (when timer (cancel-timer timer) (setq timer nil)))
      (when timer ;; Signalled an error or a `quit'.
        (cancel-timer timer)
        (bookmark-bmenu-list)
        (bookmark-bmenu-goto-bookmark bmk)))))

Message #29 received at 10998 <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 10998 <at> debbugs.gnu.org
Subject: Re: bug#10998: Allow movements in bookmark-bmenu-search
Date: Mon, 01 Oct 2012 17:11:06 +0200

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>>> I agree with Stefan that this is not an ideal solution, by the way, and
>>> have left a comment in the patch below to that effect.
>> Keep in mind that this is not a real minibuffer.
>
> AFAICT the fact that it's not a minibuffer is an implementation detail.
> IOW it currently isn't a minibuffer, but it could/should be changed to
> be one.
> See example below.
Very nice use of `minibuffer-with-setup-hook' cool.

-- 
  Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 

Message #32 received at 10998 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 10998 <at> debbugs.gnu.org
Subject: Re: bug#10998: Allow movements in bookmark-bmenu-search
Date: Mon, 01 Oct 2012 22:47:50 -0400

> See example below.

Installed,


        Stefan

Message #35 received at 10998 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: 10998 <at> debbugs.gnu.org
Subject: Re: bug#10998: Allow movements in bookmark-bmenu-search
Date: Wed, 12 Jun 2019 18:28:40 +0200

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> See example below.
>
> Installed,

I think that fixed this bug; we now have a minibuffer.

Can we close it?

Thanks,
Stefan Kangas

Message #38 received at 10998 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> gmail.com
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 10998 <at> debbugs.gnu.org
Subject: Re: bug#10998: Allow movements in bookmark-bmenu-search
Date: Wed, 12 Jun 2019 16:30:29 -0400

tags 10998 fixed
close 10998 24.3
quit

Stefan Kangas <stefan <at> marxist.se> writes:
> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>>> See example below.
>>
>> Installed,

Seems to be [1: d83ef97].

> I think that fixed this bug; we now have a minibuffer.
>
> Can we close it?

No news is generally good news, so let's assume yes.

[1: d83ef97]: 2012-10-01 22:47:12 -0400
  * lisp/bookmark.el (bookmark-search-pattern): Remove var. (bookmark-read-search-input): Remove function. (bookmark-bmenu-search): Reimplement using a minibuffer.
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=d83ef9762efd4efe833c0c9687b057d6b62cdcb7

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.