* bug#7231: 23.3; Don't rebuild buffer list in iswitchb-visit-buffer
@ 2010-10-17 17:59 Leo
2010-10-17 18:22 ` Óscar Fuentes
2010-10-17 18:48 ` Stefan Monnier
0 siblings, 2 replies; 12+ messages in thread
From: Leo @ 2010-10-17 17:59 UTC (permalink / raw)
To: 7231
The original change was to address buffer name changes due to packages
such as uniquify.el. But it causes another annoying bug: changing the
order of matches seen by users.
This reverts it.
If people are annoyed (which I doubt) by outdated buffer names due to
uniquify.el, one solution is to map the buffer-names to buffer objects
before killing and map them back to names after killing.
From 7e6597c54a7764688855c3ab2efa6cfa1cffbea6 Mon Sep 17 00:00:00 2001
Date: Mon, 18 Oct 2010 01:44:24 +0800
Subject: [PATCH] Don't rebuild buffer list in iswitchb-visit-buffer
Rebuilding buffer list will lose the order of matches seen by users
and thus cause surprises.
---
lisp/iswitchb.el | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/lisp/iswitchb.el b/lisp/iswitchb.el
index 081897a..b7baa03 100644
--- a/lisp/iswitchb.el
+++ b/lisp/iswitchb.el
@@ -1042,10 +1042,8 @@ Return the modified list with the last element prepended to it."
(if (get-buffer buf)
;; buffer couldn't be killed.
(setq iswitchb-rescan t)
- ;; Else `kill-buffer' succeeds so re-make the buffer list
- ;; taking into account packages like uniquify may rename
- ;; buffers
- (iswitchb-make-buflist iswitchb-default))))))
+ ;; else buffer was killed so remove name from list.
+ (setq iswitchb-buflist (delq buf iswitchb-buflist)))))))
;;; VISIT CHOSEN BUFFER
(defun iswitchb-visit-buffer (buffer)
--
1.7.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#7231: 23.3; Don't rebuild buffer list in iswitchb-visit-buffer
2010-10-17 17:59 bug#7231: 23.3; Don't rebuild buffer list in iswitchb-visit-buffer Leo
@ 2010-10-17 18:22 ` Óscar Fuentes
2010-10-17 19:16 ` Leo
2010-10-17 18:48 ` Stefan Monnier
1 sibling, 1 reply; 12+ messages in thread
From: Óscar Fuentes @ 2010-10-17 18:22 UTC (permalink / raw)
To: Leo; +Cc: 7231
Leo <sdl.web@gmail.com> writes:
> The original change was to address buffer name changes due to packages
> such as uniquify.el. But it causes another annoying bug: changing the
> order of matches seen by users.
>
> This reverts it.
>
> If people are annoyed (which I doubt) by outdated buffer names due to
> uniquify.el,
The purpose of the patch you reverted was addressing that annoyance, so
it is easy to infer that someone indeed was annoyed by that behavior.
> one solution is to map the buffer-names to buffer objects
> before killing and map them back to names after killing.
So why don't you implement that instead of reverting a patch that
corrects a bug? (After killing a buffer, having a list containing
non-existent buffers is a bug, right? while seeing how the list after
the second item is reordered is an annoyance, so you are proposing to
re-introduce a bug for avoiding an annoyance)
[snip]
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#7231: 23.3; Don't rebuild buffer list in iswitchb-visit-buffer
2010-10-17 18:22 ` Óscar Fuentes
@ 2010-10-17 19:16 ` Leo
2010-10-18 14:54 ` Stefan Monnier
2010-10-28 1:31 ` Stefan Monnier
0 siblings, 2 replies; 12+ messages in thread
From: Leo @ 2010-10-17 19:16 UTC (permalink / raw)
To: Óscar Fuentes; +Cc: 7231
On 2010-10-18 02:22 +0800, Óscar Fuentes wrote:
> The purpose of the patch you reverted was addressing that annoyance, so
> it is easy to infer that someone indeed was annoyed by that behavior.
[...]
>
> So why don't you implement that instead of reverting a patch that
> corrects a bug? (After killing a buffer, having a list containing
> non-existent buffers is a bug, right? while seeing how the list after
> the second item is reordered is an annoyance, so you are proposing to
> re-introduce a bug for avoiding an annoyance)
On 2010-10-18 02:48 +0800, Stefan Monnier wrote:
> I don't think we should just revert. If the fix for the old problem
> introduces a new problem, we should try and find a solution that fixes
> both problems at once.
>
> Stefan
Sorry for being too lazy. Please try the following patch.
From d61fe17fff2926731ea317b21d647a4cf2d136f4 Mon Sep 17 00:00:00 2001
Date: Mon, 18 Oct 2010 01:44:24 +0800
Subject: [PATCH] Rebuild buffer list using buffer objects
in iswitchb-kill-buffer.
Avoid `iswitchb-make-buflist' which changes the order of matches seen
by users.
---
lisp/iswitchb.el | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/lisp/iswitchb.el b/lisp/iswitchb.el
index 081897a..4cab5ee 100644
--- a/lisp/iswitchb.el
+++ b/lisp/iswitchb.el
@@ -1033,7 +1033,9 @@ Return the modified list with the last element prepended to it."
(setq buf (car iswitchb-matches))
;; check to see if buf is non-nil.
(if buf
- (progn
+ (let ((bufobjs (mapcar (lambda (name)
+ (or (get-buffer name) name))
+ iswitchb-buflist)))
(kill-buffer buf)
;; Check if buffer exists. XEmacs gnuserv.el makes alias
@@ -1042,10 +1044,13 @@ Return the modified list with the last element prepended to it."
(if (get-buffer buf)
;; buffer couldn't be killed.
(setq iswitchb-rescan t)
- ;; Else `kill-buffer' succeeds so re-make the buffer list
- ;; taking into account packages like uniquify may rename
- ;; buffers
- (iswitchb-make-buflist iswitchb-default))))))
+ ;; else buffer was killed
+ (setq iswitchb-buflist
+ (delq nil (mapcar (lambda (b)
+ (if (bufferp b)
+ (buffer-name b)
+ b))
+ bufobjs))))))))
;;; VISIT CHOSEN BUFFER
(defun iswitchb-visit-buffer (buffer)
--
1.7.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* bug#7231: 23.3; Don't rebuild buffer list in iswitchb-visit-buffer
2010-10-17 19:16 ` Leo
@ 2010-10-18 14:54 ` Stefan Monnier
2010-10-20 3:00 ` Leo
2010-10-28 1:31 ` Stefan Monnier
1 sibling, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2010-10-18 14:54 UTC (permalink / raw)
To: Leo; +Cc: Óscar Fuentes, 7231
> - ;; Else `kill-buffer' succeeds so re-make the buffer list
> - ;; taking into account packages like uniquify may rename
> - ;; buffers
> - (iswitchb-make-buflist iswitchb-default))))))
> + ;; else buffer was killed
> + (setq iswitchb-buflist
> + (delq nil (mapcar (lambda (b)
> + (if (bufferp b)
> + (buffer-name b)
> + b))
> + bufobjs))))))))
I was about to install that change when I realized that this is
fundamentally not the right approach: since some of the buffers may have
changed name, the new list of matching buffers may be different (some
buffers that didn't match before may match now and vice-versa).
So iswitchb-make-buflist is more correct. To deal with the problem of
ordering, we'll need to combine the two: call iswitchb-make-buflist to
get the new list of matches, and then use bufobjs to sort the new
iswitchb-buflist.
Something like
(iswitchb-make-buflist iswitchb-default))))))
;; Try to preserve the previous sort order.
(setq iswitchb-buflist
(sort iswitchb-buflist
(lambda (bn1 bn2)
(< (length (or (memq (get-buffer bn1) bufobjs)
;; Place new buffers at the end.
bufobjs))
(length (or (memq (get-buffer bn2) bufobjs)
bufobjs))))))
Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread* bug#7231: 23.3; Don't rebuild buffer list in iswitchb-visit-buffer
2010-10-18 14:54 ` Stefan Monnier
@ 2010-10-20 3:00 ` Leo
2010-10-20 4:53 ` Leo
2010-10-20 16:13 ` Stefan Monnier
0 siblings, 2 replies; 12+ messages in thread
From: Leo @ 2010-10-20 3:00 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Óscar Fuentes, 7231
On 2010-10-18 22:54 +0800, Stefan Monnier wrote:
> I was about to install that change when I realized that this is
> fundamentally not the right approach: since some of the buffers may have
> changed name, the new list of matching buffers may be different (some
> buffers that didn't match before may match now and vice-versa).
>
> So iswitchb-make-buflist is more correct. To deal with the problem of
> ordering, we'll need to combine the two: call iswitchb-make-buflist to
> get the new list of matches, and then use bufobjs to sort the new
> iswitchb-buflist.
My understanding seems to be:
iswitchb-buflist already contains all buffers needed although its order
can be modified by iswitchb-next/prev-match. That modified ordering info
is lost after iswitchb-make-buflist. There may be new matches appearing
due to buffer name changes but this is taken care of automatically by
iswitchb-exhibit.
Leo
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#7231: 23.3; Don't rebuild buffer list in iswitchb-visit-buffer
2010-10-20 3:00 ` Leo
@ 2010-10-20 4:53 ` Leo
2010-10-20 9:19 ` Óscar Fuentes
2010-10-21 7:19 ` Leo
2010-10-20 16:13 ` Stefan Monnier
1 sibling, 2 replies; 12+ messages in thread
From: Leo @ 2010-10-20 4:53 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Óscar Fuentes, 7231
Hello Stefan,
On 2010-10-20 11:00 +0800, Leo wrote:
> On 2010-10-18 22:54 +0800, Stefan Monnier wrote:
>> I was about to install that change when I realized that this is
>> fundamentally not the right approach: since some of the buffers may have
>> changed name, the new list of matching buffers may be different (some
>> buffers that didn't match before may match now and vice-versa).
>>
>> So iswitchb-make-buflist is more correct. To deal with the problem of
>> ordering, we'll need to combine the two: call iswitchb-make-buflist to
>> get the new list of matches, and then use bufobjs to sort the new
>> iswitchb-buflist.
>
> My understanding seems to be:
>
> iswitchb-buflist already contains all buffers needed although its order
> can be modified by iswitchb-next/prev-match. That modified ordering info
> is lost after iswitchb-make-buflist. There may be new matches appearing
> due to buffer name changes but this is taken care of automatically by
> iswitchb-exhibit.
>
> Leo
If the proposed solution to iswitchb is acceptable, a similar one should
be done for ido.
Here is a patch to be applied on top of
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6943.
From 5686c7c723d9bad601d870aff54b7a3d8edce471 Mon Sep 17 00:00:00 2001
Date: Wed, 20 Oct 2010 12:17:53 +0800
Subject: [PATCH] Don't rebuild buffer list in ido-kill-buffer-at-head
Rebuilding buffer list will lose the order of matches seen by the user
in particular when it has been rotated.
---
lisp/ido.el | 32 ++++++++++++++------------------
1 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/lisp/ido.el b/lisp/ido.el
index 4a60288..d913069 100644
--- a/lisp/ido.el
+++ b/lisp/ido.el
@@ -3978,26 +3978,22 @@ If cursor is not at the end of the user input, delete to end of input."
(if (not (eobp))
(delete-region (point) (line-end-position))
(let ((enable-recursive-minibuffers t)
- (buf (ido-name (car ido-matches)))
- (nextbuf (cadr ido-matches)))
+ (buf (ido-name (car ido-matches))))
(cond
((get-buffer buf)
- ;; If next match names a buffer use the buffer object; buffer
- ;; name may be changed by packages such as uniquify.
- (when (and nextbuf (get-buffer nextbuf))
- (setq nextbuf (get-buffer nextbuf)))
- (if (null (kill-buffer buf))
- ;; Buffer couldn't be killed.
- (setq ido-rescan t)
- ;; Else `kill-buffer' succeeds so re-make the buffer list
- ;; taking into account packages like uniquify may rename
- ;; buffers.
- (if (bufferp nextbuf)
- (setq nextbuf (buffer-name nextbuf)))
- (setq ido-default-item nextbuf
- ido-text-init ido-text
- ido-exit 'refresh)
- (exit-minibuffer)))
+ ;; Note that some packages (eg uniquify.el) may change buffer
+ ;; names. So save a list of buffer objects.
+ (let ((bufobjs (mapcar (lambda (name)
+ (or (get-buffer name) name))
+ ido-cur-list)))
+ (if (null (kill-buffer buf))
+ ;; Buffer couldn't be killed.
+ (setq ido-rescan t)
+ (setq ido-cur-list
+ (delq nil (mapcar (lambda (b) (if (bufferp b)
+ (buffer-name b)
+ b))
+ bufobjs))))))
;; Handle virtual buffers
((assoc buf ido-virtual-buffers)
(setq recentf-list
--
1.7.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* bug#7231: 23.3; Don't rebuild buffer list in iswitchb-visit-buffer
2010-10-20 4:53 ` Leo
@ 2010-10-20 9:19 ` Óscar Fuentes
2010-10-21 7:19 ` Leo
1 sibling, 0 replies; 12+ messages in thread
From: Óscar Fuentes @ 2010-10-20 9:19 UTC (permalink / raw)
To: Leo; +Cc: 7231
Leo <sdl.web@gmail.com> writes:
> If the proposed solution to iswitchb is acceptable, a similar one should
> be done for ido.
>
> Here is a patch to be applied on top of
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6943.
>
> From 5686c7c723d9bad601d870aff54b7a3d8edce471 Mon Sep 17 00:00:00 2001
> Date: Wed, 20 Oct 2010 12:17:53 +0800
> Subject: [PATCH] Don't rebuild buffer list in ido-kill-buffer-at-head
Can you post a version that does not require to previosly introduce
another patch, so we can test *this* proposed solution instead of
something else?
[snip]
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#7231: 23.3; Don't rebuild buffer list in iswitchb-visit-buffer
2010-10-20 4:53 ` Leo
2010-10-20 9:19 ` Óscar Fuentes
@ 2010-10-21 7:19 ` Leo
1 sibling, 0 replies; 12+ messages in thread
From: Leo @ 2010-10-21 7:19 UTC (permalink / raw)
To: bug-gnu-emacs
On 2010-10-20 12:53 +0800, Leo wrote:
[...]
>
> If the proposed solution to iswitchb is acceptable, a similar one should
> be done for ido.
[...]
On 2010-10-20 17:19 +0800, Óscar Fuentes wrote:
[...]
> Can you post a version that does not require to previosly introduce
> another patch, so we can test *this* proposed solution instead of
> something else?
>
> [snip]
Due to the way ido set matches i.e. it ranges matches into a few
sections unless ido-rotate is non-nil (see #5724) and the order of
rotated matches is lost after any none rotating commands, please ignore
the patch for ido for now.
Could someone having commit access apply the patch for iswitchb? I have
received a complaint from someone via IRC the other day.
Thanks.
Leo
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#7231: 23.3; Don't rebuild buffer list in iswitchb-visit-buffer
2010-10-20 3:00 ` Leo
2010-10-20 4:53 ` Leo
@ 2010-10-20 16:13 ` Stefan Monnier
2010-10-23 5:36 ` Leo
1 sibling, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2010-10-20 16:13 UTC (permalink / raw)
To: Leo; +Cc: Óscar Fuentes, 7231
>> I was about to install that change when I realized that this is
>> fundamentally not the right approach: since some of the buffers may have
>> changed name, the new list of matching buffers may be different (some
>> buffers that didn't match before may match now and vice-versa).
>>
>> So iswitchb-make-buflist is more correct. To deal with the problem of
>> ordering, we'll need to combine the two: call iswitchb-make-buflist to
>> get the new list of matches, and then use bufobjs to sort the new
>> iswitchb-buflist.
> My understanding seems to be:
> iswitchb-buflist already contains all buffers needed although its order
> can be modified by iswitchb-next/prev-match. That modified ordering info
> is lost after iswitchb-make-buflist. There may be new matches appearing
> due to buffer name changes but this is taken care of automatically by
> iswitchb-exhibit.
I see, so iswitchb-buflist contains all buffers, not just the ones that
match the current input?
In that case, my concern is indeed a non-issue and your patch
looks fine. Please install it.
Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread* bug#7231: 23.3; Don't rebuild buffer list in iswitchb-visit-buffer
2010-10-20 16:13 ` Stefan Monnier
@ 2010-10-23 5:36 ` Leo
0 siblings, 0 replies; 12+ messages in thread
From: Leo @ 2010-10-23 5:36 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Óscar Fuentes, 7231
On 2010-10-21 00:13 +0800, Stefan Monnier wrote:
> I see, so iswitchb-buflist contains all buffers, not just the ones that
> match the current input?
Except those being ignored. iswitchb-matches holds the matches.
> In that case, my concern is indeed a non-issue and your patch looks
> fine. Please install it.
Someone in IRC (whose complaint this bug report tries to address) has
also confirmed the patch fixes the issue for him. Could you or someone
else install the patch? Many thanks.
Leo
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#7231: 23.3; Don't rebuild buffer list in iswitchb-visit-buffer
2010-10-17 19:16 ` Leo
2010-10-18 14:54 ` Stefan Monnier
@ 2010-10-28 1:31 ` Stefan Monnier
1 sibling, 0 replies; 12+ messages in thread
From: Stefan Monnier @ 2010-10-28 1:31 UTC (permalink / raw)
To: Leo; +Cc: Óscar Fuentes
> Sorry for being too lazy. Please try the following patch.
Thanks, installed in the trunk.
Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#7231: 23.3; Don't rebuild buffer list in iswitchb-visit-buffer
2010-10-17 17:59 bug#7231: 23.3; Don't rebuild buffer list in iswitchb-visit-buffer Leo
2010-10-17 18:22 ` Óscar Fuentes
@ 2010-10-17 18:48 ` Stefan Monnier
1 sibling, 0 replies; 12+ messages in thread
From: Stefan Monnier @ 2010-10-17 18:48 UTC (permalink / raw)
To: Leo; +Cc: 7231
> The original change was to address buffer name changes due to packages
> such as uniquify.el. But it causes another annoying bug: changing the
> order of matches seen by users.
> This reverts it.
I don't think we should just revert. If the fix for the old problem
introduces a new problem, we should try and find a solution that fixes
both problems at once.
Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2010-10-28 1:31 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-17 17:59 bug#7231: 23.3; Don't rebuild buffer list in iswitchb-visit-buffer Leo
2010-10-17 18:22 ` Óscar Fuentes
2010-10-17 19:16 ` Leo
2010-10-18 14:54 ` Stefan Monnier
2010-10-20 3:00 ` Leo
2010-10-20 4:53 ` Leo
2010-10-20 9:19 ` Óscar Fuentes
2010-10-21 7:19 ` Leo
2010-10-20 16:13 ` Stefan Monnier
2010-10-23 5:36 ` Leo
2010-10-28 1:31 ` Stefan Monnier
2010-10-17 18:48 ` Stefan Monnier
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).