* bug#7585: 23.2.90; [PATCH] fix eshell-previous-matching-input
@ 2010-12-07 18:51 Leo
2010-12-13 17:00 ` Stefan Monnier
0 siblings, 1 reply; 7+ messages in thread
From: Leo @ 2010-12-07 18:51 UTC (permalink / raw)
To: 7585; +Cc: John Wiegley
There is a customisable variable eshell-hist-move-to-end when set to
nil, point is not guaranteed to be located behind
eshell-last-output-end. Thus blindly calling delete-region and
insert-and-inherit will generate an error in that case.
diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index 45fe050..defaf5a 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -837,6 +837,8 @@ With prefix argument N, search for Nth previous match.
If N is negative, find the next or Nth next match."
(interactive (eshell-regexp-arg "Previous input matching (regexp): "))
(setq arg (eshell-search-arg arg))
+ (assert (<= eshell-last-output-end (point))
+ nil "Point not located after prompt")
(let ((pos (eshell-previous-matching-input-string-position regexp arg)))
;; Has a match been found?
(if (null pos)
^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#7585: 23.2.90; [PATCH] fix eshell-previous-matching-input
2010-12-07 18:51 bug#7585: 23.2.90; [PATCH] fix eshell-previous-matching-input Leo
@ 2010-12-13 17:00 ` Stefan Monnier
2010-12-13 18:31 ` Leo
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2010-12-13 17:00 UTC (permalink / raw)
To: Leo; +Cc: John Wiegley, 7585
> There is a customisable variable eshell-hist-move-to-end when set to
> nil, point is not guaranteed to be located behind
> eshell-last-output-end. Thus blindly calling delete-region and
> insert-and-inherit will generate an error in that case.
The patch looks good as well, tho we usually prefer not to use `assert'
in this way (I'd keep assert for actual coding errors, so better use an
if test that then `signal's an error).
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#7585: 23.2.90; [PATCH] fix eshell-previous-matching-input
2010-12-13 17:00 ` Stefan Monnier
@ 2010-12-13 18:31 ` Leo
2010-12-13 20:42 ` Stefan Monnier
0 siblings, 1 reply; 7+ messages in thread
From: Leo @ 2010-12-13 18:31 UTC (permalink / raw)
To: Stefan Monnier; +Cc: John Wiegley, 7585
On 2010-12-13 17:00 +0000, Stefan Monnier wrote:
>> There is a customisable variable eshell-hist-move-to-end when set to
>> nil, point is not guaranteed to be located behind
>> eshell-last-output-end. Thus blindly calling delete-region and
>> insert-and-inherit will generate an error in that case.
>
> The patch looks good as well, tho we usually prefer not to use `assert'
> in this way (I'd keep assert for actual coding errors, so better use an
> if test that then `signal's an error).
Would something like the following acceptable? Thanks. Leo
2010-12-13 Leo <sdl.web@gmail.com>
* eshell/em-hist.el (eshell-previous-matching-input): Check point
is located after eshell prompt in case `eshell-hist-move-to-end'
is nil.
lisp/eshell/em-hist.el | 22 ++++++++++++----------
1 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index 45fe050..47e0b2b 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -837,16 +837,18 @@ With prefix argument N, search for Nth previous match.
If N is negative, find the next or Nth next match."
(interactive (eshell-regexp-arg "Previous input matching (regexp): "))
(setq arg (eshell-search-arg arg))
- (let ((pos (eshell-previous-matching-input-string-position regexp arg)))
- ;; Has a match been found?
- (if (null pos)
- (error "Not found")
- (setq eshell-history-index pos)
- (unless (minibuffer-window-active-p (selected-window))
- (message "History item: %d" (- (ring-length eshell-history-ring) pos)))
- ;; Can't use kill-region as it sets this-command
- (delete-region eshell-last-output-end (point))
- (insert-and-inherit (eshell-get-history pos)))))
+ (if (<= eshell-last-output-end (point))
+ (let ((pos (eshell-previous-matching-input-string-position regexp arg)))
+ ;; Has a match been found?
+ (if (null pos)
+ (error "Not found")
+ (setq eshell-history-index pos)
+ (unless (minibuffer-window-active-p (selected-window))
+ (message "History item: %d" (- (ring-length eshell-history-ring) pos)))
+ ;; Can't use kill-region as it sets this-command
+ (delete-region eshell-last-output-end (point))
+ (insert-and-inherit (eshell-get-history pos))))
+ (message "Point not located after prompt")))
(defun eshell-next-matching-input (regexp arg)
"Search forwards through input history for match for REGEXP.
--
1.7.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#7585: 23.2.90; [PATCH] fix eshell-previous-matching-input
2010-12-13 18:31 ` Leo
@ 2010-12-13 20:42 ` Stefan Monnier
2010-12-14 6:13 ` Leo
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2010-12-13 20:42 UTC (permalink / raw)
To: Leo; +Cc: John Wiegley, 7585
> Would something like the following acceptable? Thanks. Leo
No, don't use `message': this is a real error, use `signal' or `error'.
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#7585: 23.2.90; [PATCH] fix eshell-previous-matching-input
2010-12-13 20:42 ` Stefan Monnier
@ 2010-12-14 6:13 ` Leo
2010-12-14 19:46 ` Stefan Monnier
2010-12-17 11:04 ` Chong Yidong
0 siblings, 2 replies; 7+ messages in thread
From: Leo @ 2010-12-14 6:13 UTC (permalink / raw)
To: Stefan Monnier; +Cc: John Wiegley, 7585
On 2010-12-13 20:42 +0000, Stefan Monnier wrote:
>> Would something like the following acceptable? Thanks. Leo
>
> No, don't use `message': this is a real error, use `signal' or `error'.
>
>
> Stefan
OK. Here is the whole thing with the change of `message' to `error'.
Without this patch, one usually gets an error like "Text is read-only"
when M-n/p. So the error is obscure. The patch makes the error more
useful.
2010-12-13 Leo <sdl.web@gmail.com>
* eshell/em-hist.el (eshell-previous-matching-input): Check point
is located after eshell prompt in case `eshell-hist-move-to-end'
is nil.
lisp/eshell/em-hist.el | 22 ++++++++++++----------
1 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index 45fe050..47e0b2b 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -837,16 +837,18 @@ With prefix argument N, search for Nth previous match.
If N is negative, find the next or Nth next match."
(interactive (eshell-regexp-arg "Previous input matching (regexp): "))
(setq arg (eshell-search-arg arg))
- (let ((pos (eshell-previous-matching-input-string-position regexp arg)))
- ;; Has a match been found?
- (if (null pos)
- (error "Not found")
- (setq eshell-history-index pos)
- (unless (minibuffer-window-active-p (selected-window))
- (message "History item: %d" (- (ring-length eshell-history-ring) pos)))
- ;; Can't use kill-region as it sets this-command
- (delete-region eshell-last-output-end (point))
- (insert-and-inherit (eshell-get-history pos)))))
+ (if (<= eshell-last-output-end (point))
+ (let ((pos (eshell-previous-matching-input-string-position regexp arg)))
+ ;; Has a match been found?
+ (if (null pos)
+ (error "Not found")
+ (setq eshell-history-index pos)
+ (unless (minibuffer-window-active-p (selected-window))
+ (message "History item: %d" (- (ring-length eshell-history-ring) pos)))
+ ;; Can't use kill-region as it sets this-command
+ (delete-region eshell-last-output-end (point))
+ (insert-and-inherit (eshell-get-history pos))))
+ (error "Point not located after prompt")))
(defun eshell-next-matching-input (regexp arg)
"Search forwards through input history for match for REGEXP.
--
1.7.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#7585: 23.2.90; [PATCH] fix eshell-previous-matching-input
2010-12-14 6:13 ` Leo
@ 2010-12-14 19:46 ` Stefan Monnier
2010-12-17 11:04 ` Chong Yidong
1 sibling, 0 replies; 7+ messages in thread
From: Stefan Monnier @ 2010-12-14 19:46 UTC (permalink / raw)
To: Leo; +Cc: John Wiegley, 7585
> OK. Here is the whole thing with the change of `message' to `error'.
Looks good, please install it,
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#7585: 23.2.90; [PATCH] fix eshell-previous-matching-input
2010-12-14 6:13 ` Leo
2010-12-14 19:46 ` Stefan Monnier
@ 2010-12-17 11:04 ` Chong Yidong
1 sibling, 0 replies; 7+ messages in thread
From: Chong Yidong @ 2010-12-17 11:04 UTC (permalink / raw)
To: Leo; +Cc: John Wiegley, 7585
Leo <sdl.web@gmail.com> writes:
> OK. Here is the whole thing with the change of `message' to `error'.
>
> Without this patch, one usually gets an error like "Text is read-only"
> when M-n/p. So the error is obscure. The patch makes the error more
> useful.
Thanks; committed.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-12-17 11:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-07 18:51 bug#7585: 23.2.90; [PATCH] fix eshell-previous-matching-input Leo
2010-12-13 17:00 ` Stefan Monnier
2010-12-13 18:31 ` Leo
2010-12-13 20:42 ` Stefan Monnier
2010-12-14 6:13 ` Leo
2010-12-14 19:46 ` Stefan Monnier
2010-12-17 11:04 ` Chong Yidong
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).