bug#25352: 26.0.50; ffap-other-window should not change window configuration when not displaying a buffer

[Philipp Stephani <p.stephani2@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 490 bytes --]

Philipp <p.stephani2@gmail.com> schrieb am Mi., 4. Jan. 2017 um 11:58 Uhr:

>
> Insert some URL, e.g. "http://www.gnu.org".
> Move point inside the URL.
> M-x load-library RET ffap RET
> M-x ffap-other-window RET RET
> The URL is now opened in the default browser, as expected.  However, the
> window is also split, even if the default browser is external (not
> EWW).  ffap-other-window should not change the window configuration
> unless it has to display a buffer.
>
>
Attached a patch.

[-- Attachment #1.2: Type: text/html, Size: 1073 bytes --]

[-- Attachment #2: 0001-ffap-Don-t-switch-window-unless-needed.txt --]
[-- Type: text/plain, Size: 2411 bytes --]

From d0982ad65bdce2838777c8dcd62bfe2986b4e872 Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Sun, 8 Jan 2017 19:44:22 +0100
Subject: [PATCH] ffap: Don't switch window unless needed

When using ffap-other-window, don't change the window configuration
unless a new buffer has actually been created (Bug#25352).

* lisp/ffap.el (ffap-other-frame): Don't change the window
configuration if no new buffer has been created.
* test/lisp/ffap-tests.el (ffap-other-window--bug-25352): Add unit
test.
---
 lisp/ffap.el            | 11 +++--------
 test/lisp/ffap-tests.el | 13 +++++++++++++
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/lisp/ffap.el b/lisp/ffap.el
index 1ea32b75f1..87531110b8 100644
--- a/lisp/ffap.el
+++ b/lisp/ffap.el
@@ -1768,14 +1768,9 @@ ffap-other-window
   "Like `ffap', but put buffer in another window.
 Only intended for interactive use."
   (interactive)
-  (let (value)
-    (switch-to-buffer-other-window
-     (save-window-excursion
-       (setq value (call-interactively 'ffap))
-       (unless (or (bufferp value) (bufferp (car-safe value)))
-	 (setq value (current-buffer)))
-       (current-buffer)))
-    value))
+  (pcase (save-window-excursion (call-interactively 'ffap))
+    ((or (and (pred bufferp) b) `(,(and (pred bufferp) b) . ,_))
+     (switch-to-buffer-other-window b))))
 
 (defun ffap-other-frame ()
   "Like `ffap', but put buffer in another frame.
diff --git a/test/lisp/ffap-tests.el b/test/lisp/ffap-tests.el
index 827d751be6..1862c6c327 100644
--- a/test/lisp/ffap-tests.el
+++ b/test/lisp/ffap-tests.el
@@ -23,6 +23,7 @@
 
 ;;; Code:
 
+(require 'cl-lib)
 (require 'ert)
 (require 'ffap)
 
@@ -66,6 +67,18 @@
     (let ((ffap-gopher-regexp nil))
       (should-not (ffap-gopher-at-point)))))
 
+(ert-deftest ffap-other-window--bug-25352 ()
+  "Test for Bug#25352.  Checks that the window configuration is
+left alone when opening a URL in an external browser."
+  (cl-letf* ((old (current-window-configuration))
+             ((symbol-function 'ffap-prompter)
+              (lambda () "http://www.gnu.org"))
+             (urls nil)
+             (ffap-url-fetcher (lambda (url) (push url urls) nil)))
+    (should-not (ffap-other-window))
+    (should (equal (current-window-configuration) old))
+    (should (equal urls '("http://www.gnu.org")))))
+
 (provide 'ffap-tests)
 
 ;;; ffap-tests.el ends here
-- 
2.12.2