Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken

[Max Nikulin <manikulin@gmail.com>]

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

On 14/10/2021 22:44, Max Nikulin wrote:
> 
> I think, something should be done with `org-no-popups'. Assume a user 
> who has (I have no idea concerning the goal though)
> 
>    (setq pop-up-frames t)
>    (setq display-buffer-base-action
>          '((display-buffer-reuse-window display-buffer-pop-up-frame)
>            (reusable-frames . 0)))
> 
> With "emacs -Q" and above settings completion e.g. for "C-h f" does not 
> cause creation of a new frame. Org help windows appear in new frames 
> though. That is why `org-no-popups' should have more code.

I was wrong, (setq pop-up-frames t) leads to creation of new frame for 
*Completion* buffer at least in Emacs-26.3.

It seems, each case of `org-no-popups' may require specific code. I have 
tried to take some code related to completion. It overrides 
display-buffer-base-action, but something more is required for 
pop-up-frames.
That code uses `with-current-buffer-window' while org-goto uses 
`with-output-to-temp-buffer'. I am unsure what variant is more suitable 
for org-goto.

I am attaching my draft with minimal changes. I do not like to rely on 
internal functions but I have not found high level replacement to 
achieve the same result. Maybe emacs code has a better variant somewhere.

[-- Attachment #2: org-goto-hack-1.patch --]
[-- Type: text/x-patch, Size: 2400 bytes --]

diff --git a/lisp/org-goto.el b/lisp/org-goto.el
index 0a3470f54..26fc2b735 100644
--- a/lisp/org-goto.el
+++ b/lisp/org-goto.el
@@ -203,7 +203,6 @@ When nil, you can use these keybindings to navigate the buffer:
   "Let the user select a location in current buffer.
 This function uses a recursive edit.  It returns the selected
 position or nil."
-  (org-no-popups
    (let ((isearch-mode-map org-goto-local-auto-isearch-map)
 	 (isearch-hide-immediately nil)
 	 (isearch-search-fun-function
@@ -217,7 +216,35 @@ position or nil."
 	  (condition-case nil
 	      (make-indirect-buffer (current-buffer) "*org-goto*" t)
 	    (error (make-indirect-buffer (current-buffer) "*org-goto*" t))))
-	 (let (temp-buffer-show-function temp-buffer-show-hook)
+	 (let (temp-buffer-show-hook
+	       (temp-buffer-show-function
+		(lambda (buffer)
+		  "Prevent new frame in the case of
+
+  (setq display-buffer-base-action
+        '((display-buffer-reuse-window display-buffer-pop-up-frame)
+          (reusable-frames . 0)))
+
+It is not immune to
+
+  (setq pop-up-frames t)
+
+just as \"*Completion*\" buffer.
+The idea is borrowed from `minibuffer-completion-help'."
+		  (display-buffer
+                   buffer
+                   `((display-buffer--maybe-same-window
+                      display-buffer-reuse-window
+                      ,(if (functionp 'display-buffer--maybe-pop-up-frame)
+                           ;; Unavailable in emacs-26
+                           'display-buffer--maybe-pop-up-frame
+                         'display-buffer--maybe-pop-up-frame-or-window)
+                      display-buffer-below-selected)
+                     ,(if temp-buffer-resize-mode
+                          '(window-height . resize-temp-buffer-window)
+                        '(window-height . fit-window-to-buffer))
+                     ,(when temp-buffer-resize-mode
+                        '(preserve-size . (nil . t))))))))
 	   (with-output-to-temp-buffer "*Org Help*"
 	     (princ (format help (if org-goto-auto-isearch
 				     "  Just type for auto-isearch."
@@ -236,7 +263,7 @@ position or nil."
 	 (use-local-map org-goto-map)
 	 (recursive-edit)))
      (kill-buffer "*org-goto*")
-     (cons org-goto-selected-point org-goto-exit-command))))
+     (cons org-goto-selected-point org-goto-exit-command)))
 
 ;;;###autoload
 (defun org-goto (&optional alternative-interface)