* bug#10998: Allow movements in bookmark-bmenu-search.
@ 2012-03-12 6:08 Thierry Volpiatto
2012-03-12 12:31 ` Stefan Monnier
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Thierry Volpiatto @ 2012-03-12 6:08 UTC (permalink / raw)
To: 10998
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#10998: Allow movements in bookmark-bmenu-search.
2012-03-12 6:08 bug#10998: Allow movements in bookmark-bmenu-search Thierry Volpiatto
@ 2012-03-12 12:31 ` Stefan Monnier
2012-03-12 12:49 ` Thierry Volpiatto
2012-10-01 4:35 ` Karl Fogel
2019-06-12 16:28 ` Stefan Kangas
2 siblings, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2012-03-12 12:31 UTC (permalink / raw)
To: Thierry Volpiatto; +Cc: 10998
> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#10998: Allow movements in bookmark-bmenu-search.
2012-03-12 12:31 ` Stefan Monnier
@ 2012-03-12 12:49 ` Thierry Volpiatto
2012-03-12 15:52 ` Stefan Monnier
0 siblings, 1 reply; 12+ messages in thread
From: Thierry Volpiatto @ 2012-03-12 12:49 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 10998
Stefan Monnier <monnier@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
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#10998: Allow movements in bookmark-bmenu-search.
2012-03-12 12:49 ` Thierry Volpiatto
@ 2012-03-12 15:52 ` Stefan Monnier
2012-03-13 15:34 ` Thierry Volpiatto
0 siblings, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2012-03-12 15:52 UTC (permalink / raw)
To: Thierry Volpiatto; +Cc: 10998
>>> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#10998: Allow movements in bookmark-bmenu-search.
2012-03-12 15:52 ` Stefan Monnier
@ 2012-03-13 15:34 ` Thierry Volpiatto
0 siblings, 0 replies; 12+ messages in thread
From: Thierry Volpiatto @ 2012-03-13 15:34 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 10998
Hi Stefan,
Stefan Monnier <monnier@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
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#10998: Allow movements in bookmark-bmenu-search
2012-03-12 6:08 bug#10998: Allow movements in bookmark-bmenu-search Thierry Volpiatto
2012-03-12 12:31 ` Stefan Monnier
@ 2012-10-01 4:35 ` Karl Fogel
2012-10-01 7:45 ` Thierry Volpiatto
2019-06-12 16:28 ` Stefan Kangas
2 siblings, 1 reply; 12+ messages in thread
From: Karl Fogel @ 2012-10-01 4:35 UTC (permalink / raw)
To: 10998
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@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@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@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))
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#10998: Allow movements in bookmark-bmenu-search
2012-10-01 4:35 ` Karl Fogel
@ 2012-10-01 7:45 ` Thierry Volpiatto
2012-10-01 14:53 ` Stefan Monnier
0 siblings, 1 reply; 12+ messages in thread
From: Thierry Volpiatto @ 2012-10-01 7:45 UTC (permalink / raw)
To: 10998
Karl Fogel <kfogel@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@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
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#10998: Allow movements in bookmark-bmenu-search
2012-10-01 7:45 ` Thierry Volpiatto
@ 2012-10-01 14:53 ` Stefan Monnier
2012-10-01 15:11 ` Thierry Volpiatto
2012-10-02 2:47 ` Stefan Monnier
0 siblings, 2 replies; 12+ messages in thread
From: Stefan Monnier @ 2012-10-01 14:53 UTC (permalink / raw)
To: Thierry Volpiatto; +Cc: 10998
>> 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)))))
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#10998: Allow movements in bookmark-bmenu-search
2012-10-01 14:53 ` Stefan Monnier
@ 2012-10-01 15:11 ` Thierry Volpiatto
2012-10-02 2:47 ` Stefan Monnier
1 sibling, 0 replies; 12+ messages in thread
From: Thierry Volpiatto @ 2012-10-01 15:11 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 10998
Stefan Monnier <monnier@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
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#10998: Allow movements in bookmark-bmenu-search
2012-10-01 14:53 ` Stefan Monnier
2012-10-01 15:11 ` Thierry Volpiatto
@ 2012-10-02 2:47 ` Stefan Monnier
1 sibling, 0 replies; 12+ messages in thread
From: Stefan Monnier @ 2012-10-02 2:47 UTC (permalink / raw)
To: Thierry Volpiatto; +Cc: 10998
> See example below.
Installed,
Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#10998: Allow movements in bookmark-bmenu-search
2012-03-12 6:08 bug#10998: Allow movements in bookmark-bmenu-search Thierry Volpiatto
2012-03-12 12:31 ` Stefan Monnier
2012-10-01 4:35 ` Karl Fogel
@ 2019-06-12 16:28 ` Stefan Kangas
2019-06-12 20:30 ` npostavs
2 siblings, 1 reply; 12+ messages in thread
From: Stefan Kangas @ 2019-06-12 16:28 UTC (permalink / raw)
To: 10998
Stefan Monnier <monnier@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
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#10998: Allow movements in bookmark-bmenu-search
2019-06-12 16:28 ` Stefan Kangas
@ 2019-06-12 20:30 ` npostavs
0 siblings, 0 replies; 12+ messages in thread
From: npostavs @ 2019-06-12 20:30 UTC (permalink / raw)
To: Stefan Kangas; +Cc: 10998
tags 10998 fixed
close 10998 24.3
quit
Stefan Kangas <stefan@marxist.se> writes:
> Stefan Monnier <monnier@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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-06-12 20:30 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-12 6:08 bug#10998: Allow movements in bookmark-bmenu-search Thierry Volpiatto
2012-03-12 12:31 ` Stefan Monnier
2012-03-12 12:49 ` Thierry Volpiatto
2012-03-12 15:52 ` Stefan Monnier
2012-03-13 15:34 ` Thierry Volpiatto
2012-10-01 4:35 ` Karl Fogel
2012-10-01 7:45 ` Thierry Volpiatto
2012-10-01 14:53 ` Stefan Monnier
2012-10-01 15:11 ` Thierry Volpiatto
2012-10-02 2:47 ` Stefan Monnier
2019-06-12 16:28 ` Stefan Kangas
2019-06-12 20:30 ` npostavs
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).