bug#49275: 28.0.50; [PATCH] improve-switch-to-prev-buffer-skip

bug#49275: 28.0.50; [PATCH] improve-switch-to-prev-buffer-skip

[Trust me I am a Doctor]

[-- Attachment #1: Type: text/plain, Size: 1713 bytes --]


$ emacs -Q

;; Considering the next commands :

(defun pils/cycle-buffer-of-major-mode (&optional arg)
  "Switch to previous buffer of this major mode.
With ARG as \\[universal-argument], switch to the next instead."
  (interactive "P")
  (let ((switch-to-prev-buffer-skip
         (lambda (window buffer _bury-or-kill)
           (not (eq (buffer-local-value 'major-mode (window-buffer window))
                    (buffer-local-value 'major-mode buffer))))))
    (if arg (switch-to-next-buffer) (switch-to-prev-buffer nil 'append)))
  (when (eq (current-buffer) (window-old-buffer))
    (user-error "No other %s buffer available." major-mode)))

(defun pils/next-buffer-of-major-mode ()
  "Switch to the next buffer of this major mode."
  (interactive)
  (pils/cycle-buffer-of-major-mode t))

;; That we could temporary bind to :

(global-set-key (kbd "M-p") #'pils/cycle-buffer-of-major-mode)
(global-set-key (kbd "M-n") #'pils/next-buffer-of-major-mode)

Now gently but firmly, play theses emacs chords.

What happened ? The first command will put you in another mode.
That is unexpected ... Worse the second one will quickly jam.

The current implementation 'switch-to-prev-buffer-skip' fallback to the
first skipped buffer if no one have satisfied its anti-predicate. That's
why you could end up in another mode despite setting
'switch-to-prev-buffer-skip' to not select others modes.

Conservatively I fix that by checking if 'switch-to-prev-buffer-skip'
is a function.

The second issue is a bug in the implementation of
'switch-to-next-buffer'. This command should never return the same
buffer, it is wrote in its docstring, and it escaped me in commit
d0c7d8bc22.

So here a little patch to fix both.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Improve-switch-to-prev-buffer-skip --]
[-- Type: text/x-diff, Size: 1974 bytes --]

From 687725fa0ab9c5cae9a6c5efb7e24c14ce2c2a1f Mon Sep 17 00:00:00 2001
From: Trust me I am a doctor <pillule@riseup.net>
Date: Sun, 27 Jun 2021 02:24:27 +0200
Subject: [PATCH] Improve switch-to-prev-buffer-skip

* lisp/window.el (switch-to-prev-buffer): It was fall-backing to the
first skipped buffer, what I know is it is most probably undesirable
when 'switch-to-prev-buffer-skip' is a function. So we only do it when
it is not a function.
(switch-to-next-buffer): Tiddo, and also fix a bug where this function
was returning the same buffer, as it never should do.
---
 lisp/window.el | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lisp/window.el b/lisp/window.el
index c0511bec4c..668bcf0d04 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4721,8 +4721,8 @@ switch-to-prev-buffer
 	       window new-buffer (nth 1 entry) (nth 2 entry))
 	      (throw 'found t)))))
 
-      (when skipped
-        ;; Show first skipped buffer.
+      (when (and skipped (not functionp switch-to-prev-buffer-skip-p))
+        ;; Show first skipped buffer, unless skip was a function.
 	(setq new-buffer skipped)
 	(set-window-buffer-start-and-point window new-buffer)))
 
@@ -4831,6 +4831,7 @@ switch-to-next-buffer
       ;; nreverse here!)
       (dolist (entry (reverse (window-prev-buffers window)))
 	(when (and (not (eq new-buffer (car entry)))
+                   (not (eq old-buffer (car entry)))
                    (setq new-buffer (car entry))
 		   (or (buffer-live-p new-buffer)
 		       (not (setq killed-buffers
@@ -4842,8 +4843,8 @@ switch-to-next-buffer
 	     window new-buffer (nth 1 entry) (nth 2 entry))
 	    (throw 'found t))))
 
-      (when skipped
-        ;; Show first skipped buffer.
+      (when (and skipped (not functionp switch-to-prev-buffer-skip-p))
+        ;; Show first skipped buffer, unless skip was a function.
 	(setq new-buffer skipped)
 	(set-window-buffer-start-and-point window new-buffer)))
 
-- 
2.20.1

bug#49275: 28.0.50; [PATCH] improve-switch-to-prev-buffer-skip

[martin rudalics]

 > Conservatively I fix that by checking if 'switch-to-prev-buffer-skip'
 > is a function.

What is the intended meaning of

+      (when (and skipped (not functionp switch-to-prev-buffer-skip-p))

Did you mean

+      (when (and skipped (not (functionp switch-to-prev-buffer-skip)))

here?

martin

bug#49275: 28.0.50; [PATCH] improve-switch-to-prev-buffer-skip

[Trust me I am a Doctor]

[-- Attachment #1: Type: text/plain, Size: 365 bytes --]


martin rudalics <rudalics@gmx.at> writes:

>> Conservatively I fix that by checking if 'switch-to-prev-buffer-skip'
>> is a function.
>
> What is the intended meaning of
>
> +      (when (and skipped (not functionp switch-to-prev-buffer-skip-p))
>
> Did you mean
>
> +      (when (and skipped (not (functionp switch-to-prev-buffer-skip)))
>
> here?

Duh. Indeed.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: improve switch to prev buffer skip with fixed parens --]
[-- Type: text/x-diff, Size: 1974 bytes --]

From 687725fa0ab9c5cae9a6c5efb7e24c14ce2c2a1f Mon Sep 17 00:00:00 2001
From: Trust me I am a doctor <pillule@riseup.net>
Date: Sun, 27 Jun 2021 02:24:27 +0200
Subject: [PATCH] Improve switch-to-prev-buffer-skip

* lisp/window.el (switch-to-prev-buffer): It was fall-backing to the
first skipped buffer, what I know is it is most probably undesirable
when 'switch-to-prev-buffer-skip' is a function. So we only do it when
it is not a function.
(switch-to-next-buffer): Tiddo, and also fix a bug where this function
was returning the same buffer, as it never should do.
---
 lisp/window.el | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lisp/window.el b/lisp/window.el
index c0511bec4c..668bcf0d04 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4721,8 +4721,8 @@ switch-to-prev-buffer
 	       window new-buffer (nth 1 entry) (nth 2 entry))
 	      (throw 'found t)))))
 
-      (when skipped
-        ;; Show first skipped buffer.
+      (when (and skipped (not functionp switch-to-prev-buffer-skip-p))
+        ;; Show first skipped buffer, unless skip was a function.
 	(setq new-buffer skipped)
 	(set-window-buffer-start-and-point window new-buffer)))
 
@@ -4831,6 +4831,7 @@ switch-to-next-buffer
       ;; nreverse here!)
       (dolist (entry (reverse (window-prev-buffers window)))
 	(when (and (not (eq new-buffer (car entry)))
+                   (not (eq old-buffer (car entry)))
                    (setq new-buffer (car entry))
 		   (or (buffer-live-p new-buffer)
 		       (not (setq killed-buffers
@@ -4842,8 +4843,8 @@ switch-to-next-buffer
 	     window new-buffer (nth 1 entry) (nth 2 entry))
 	    (throw 'found t))))
 
-      (when skipped
-        ;; Show first skipped buffer.
+      (when (and skipped (not functionp switch-to-prev-buffer-skip-p))
+        ;; Show first skipped buffer, unless skip was a function.
 	(setq new-buffer skipped)
 	(set-window-buffer-start-and-point window new-buffer)))
 
-- 
2.20.1


[-- Attachment #3: Type: text/plain, Size: 4 bytes --]


--

bug#49275: 28.0.50; [PATCH] improve-switch-to-prev-buffer-skip

[martin rudalics]

Sorry but this is still not right.  It now gets me

In switch-to-prev-buffer:
../../lisp/window.el:4724:27: Warning: ‘not’ called with 2 args, but requires
     1
../../lisp/window.el:4724:27: Warning: reference to free variable ‘functionp’
../../lisp/window.el:4724:27: Warning: reference to free variable
     ‘switch-to-prev-buffer-skip-p’

In switch-to-next-buffer:
../../lisp/window.el:4846:27: Warning: ‘not’ called with 2 args, but requires
     1
../../lisp/window.el:4846:27: Warning: reference to free variable ‘functionp’
../../lisp/window.el:4846:27: Warning: reference to free variable
     ‘switch-to-prev-buffer-skip-p’

martin

bug#49275: 28.0.50; [PATCH] improve-switch-to-prev-buffer-skip

[Trust me I am a Doctor]

[-- Attachment #1: Type: text/plain, Size: 798 bytes --]


martin rudalics <rudalics@gmx.at> writes:

> Sorry but this is still not right.  It now gets me
>
> In switch-to-prev-buffer:
> ../../lisp/window.el:4724:27: Warning: ‘not’ called with 2 args, but requires
>     1
> ../../lisp/window.el:4724:27: Warning: reference to free variable ‘functionp’
> ../../lisp/window.el:4724:27: Warning: reference to free variable
>     ‘switch-to-prev-buffer-skip-p’
>
> In switch-to-next-buffer:
> ../../lisp/window.el:4846:27: Warning: ‘not’ called with 2 args, but requires
>     1
> ../../lisp/window.el:4846:27: Warning: reference to free variable ‘functionp’
> ../../lisp/window.el:4846:27: Warning: reference to free variable
>     ‘switch-to-prev-buffer-skip-p’
>
> martin

Sorry that was the same buggy one.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: this one is corrected --]
[-- Type: text/x-diff, Size: 1985 bytes --]

From b1375219d247d87ac72a41fb4f1887a9a40039fc Mon Sep 17 00:00:00 2001
From: Trust me I am a doctor <pillule@riseup.net>
Date: Sun, 27 Jun 2021 02:27:48 +0200
Subject: [PATCH] Make switch-to-prev-buffer-skip more reliable

* lisp/window.el (switch-to-prev-buffer): It was fall-backing to the
first skipped buffer, what I know is it is most probably undesirable
when 'switch-to-prev-buffer-skip' is a function. So we only do it when
it is not a function.
(switch-to-next-buffer): Tiddo, and also fix a bug where this function
was returning the same buffer, as it never should do.
---
 lisp/window.el | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lisp/window.el b/lisp/window.el
index c0511bec4c..9315107f1f 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4721,8 +4721,8 @@ switch-to-prev-buffer
 	       window new-buffer (nth 1 entry) (nth 2 entry))
 	      (throw 'found t)))))
 
-      (when skipped
-        ;; Show first skipped buffer.
+      (when (and skipped (not (functionp switch-to-prev-buffer-skip)))
+        ;; Show first skipped buffer, unless skip was a function.
 	(setq new-buffer skipped)
 	(set-window-buffer-start-and-point window new-buffer)))
 
@@ -4831,6 +4831,7 @@ switch-to-next-buffer
       ;; nreverse here!)
       (dolist (entry (reverse (window-prev-buffers window)))
 	(when (and (not (eq new-buffer (car entry)))
+                   (not (eq old-buffer (car entry)))
                    (setq new-buffer (car entry))
 		   (or (buffer-live-p new-buffer)
 		       (not (setq killed-buffers
@@ -4842,8 +4843,8 @@ switch-to-next-buffer
 	     window new-buffer (nth 1 entry) (nth 2 entry))
 	    (throw 'found t))))
 
-      (when skipped
-        ;; Show first skipped buffer.
+      (when (and skipped (not (functionp switch-to-prev-buffer-skip)))
+        ;; Show first skipped buffer, unless skip was a function.
 	(setq new-buffer skipped)
 	(set-window-buffer-start-and-point window new-buffer)))
 
-- 
2.20.1


[-- Attachment #3: Type: text/plain, Size: 5 bytes --]



--

bug#49275: 28.0.50; [PATCH] improve-switch-to-prev-buffer-skip

[martin rudalics]

 > Sorry that was the same buggy one.

Installed, finally.  Sorry for the delay.

Thanks, martin

bug#49275: 28.0.50; [PATCH] improve-switch-to-prev-buffer-skip

[Trust me I am a Doctor]

close 49275
thanks
--