Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
On Monday, 18 Oct 2021 at 23:53, Max Nikulin wrote: > I was trying to say that even with such *user* setup, behavior of Org > should be reasonable. Ah, okay. I agree. I also do not know what should be the default behaviour but I do know that I don't like the current default behaviour! I would, however, be happy with anything that I could control using display-buffer-alist. -- : Eric S Fraga via Emacs 28.0.60, Org release_9.5-93-gd87250 : Latest paper written in org: https://arxiv.org/abs/2106.05096
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
On 18/10/2021 16:25, Eric S Fraga wrote: On Sunday, 17 Oct 2021 at 23:35, Max Nikulin wrote: So (setq display-buffer-base-action '((display-buffer-reuse-window display-buffer-pop-up-frame) (reusable-frames . 0))) should not be considered as shooting a foot. I am not sure I understand what you are or are not proposing but org should not be setting this variable. This variable is for the "user", not the package, to set. I was trying to say that even with such *user* setup, behavior of Org should be reasonable. In particular, with draft patch I posted, *small* org-goto help buffer should not cause appearance of a new frame despite of `display-buffer-base-action' setting cited above, that is suitable for *regular* buffers. User still has opportunity to request arbitrary action for "*Org Help*" buffer through `display-buffer-alist'. I actually think that org imposes too much of its own view on how buffers should be managed & displayed. I am constantly annoyed (and posted about this recently to the list with no response) that capture buffers pop up all over the place and it doesn't seem like I have any control at all. I agree that behavior of `org-add-note' is extremely intrusive. On the other hand I have no idea concerning its preferred behavior by default. I do not think that something like `org-src-window-setup' should be introduced for notes and captures. Ideally it should have reasonable behavior by default and should be adjustable through `display-buffer-alist'. Currently I can not suggest a snippet for `display-buffer-alist' to exclude zoom or teams windows from candidates for `switch-to-buffer-other-window' (that is called from `org-capture').
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
On Sunday, 17 Oct 2021 at 23:35, Max Nikulin wrote: > So > >(setq display-buffer-base-action > '((display-buffer-reuse-window display-buffer-pop-up-frame) > (reusable-frames . 0))) > > should not be considered as shooting a foot. I am not sure I understand what you are or are not proposing but org should not be setting this variable. This variable is for the "user", not the package, to set. I actually think that org imposes too much of its own view on how buffers should be managed & displayed. I am constantly annoyed (and posted about this recently to the list with no response) that capture buffers pop up all over the place and it doesn't seem like I have any control at all. -- : Eric S Fraga via Emacs 28.0.60, Org release_9.5-93-gd87250 : Latest paper written in org: https://arxiv.org/abs/2106.05096
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
On 16/10/2021 13:52, Ihor Radchenko wrote:
Max Nikulin writes:
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.
I think you went too far. display-buffer-base-action is mostly for user
to specify. We should not override it. If user needs adjustments for
*Org Help*, it is always possible using a custom function inside
display-buffer-base-action. I do not think that we need to do this job
for the user.
From my perspective, there is no need to prevent users from shooting
their own leg (we are in Emacs after all). The cases we may need to
interfere are: (1) when some user customisations are mostly ok, except
they affect Org's usability, and the user has no easy way to fix it; (2)
when some customisation is very common and we significantly help many
users by supporting such customisation in Org itself.
I think pop-up-windows customisation qualifies for "(1)". Not
display-buffer-base-action.
I may be wrong but from
https://www.gnu.org/software/emacs/manual/html_node/elisp/Displaying-Buffers.html
"Displaying a Buffer in a Suitable Window" I have an impression that the
purpose of `display-buffer-base-action' is e.g. to say "please, show
every buffer in a new frame". However there are certainly small buffers
(e.g. "*Completion*") that do not deserve own frames, so it is
responsibility of `display-buffer' caller to pass an argument that
overrides *default* action. User may still customize
`display-buffer-alist' to override `display-buffer' argument. Notice
that I left `display-buffer-overriding-action' (highest priority
variable) untouched.
There are another type of buffers, e.g. "*Backtrace*". New frame is
created for them but on quit from debugger they disappear from the
screen (actually they become iconified) till next error.
I believe, Org should provide similar hints ("show this small window
below this buffer" for org-goto help, "hide this window on quit" for
edit src) with priority in between of `display-buffer-base-action' and
`display-buffer-alist'. Since `org-no-popups' is used in both cases and
due to API related to `display-buffer', it is impossible to just fix
`org-no-popups'. Some new macros or functions should be introduced.
So
(setq display-buffer-base-action
'((display-buffer-reuse-window display-buffer-pop-up-frame)
(reusable-frames . 0)))
should not be considered as shooting a foot.
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
Max Nikulin writes: > 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. I think you went too far. display-buffer-base-action is mostly for user to specify. We should not override it. If user needs adjustments for *Org Help*, it is always possible using a custom function inside display-buffer-base-action. I do not think that we need to do this job for the user. >From my perspective, there is no need to prevent users from shooting their own leg (we are in Emacs after all). The cases we may need to interfere are: (1) when some user customisations are mostly ok, except they affect Org's usability, and the user has no easy way to fix it; (2) when some customisation is very common and we significantly help many users by supporting such customisation in Org itself. I think pop-up-windows customisation qualifies for "(1)". Not display-buffer-base-action. Best, Ihor
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
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. 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 ( alternative-interface)
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
On 14/10/2021 17:16, Ihor Radchenko wrote: Marco Wahl writes: Since org-goto in main is still broken I'll commit the fix for org-goto which kicks out the use of the macro org-no-popups (but not the macro itself since it's used elsewhere AFAICS.) Max, Ihor! If you see the necessity of refinement please keep going! I am inclined to think that org-no-popups may still be useful to suppress pop-up-frames. However, no so much for pop-up-windows. Note that I introduced pop-up-windows let-binding in org-no-popups in place of overriding display-buffer-alist. However, I did not fully understand pop-up-windows does and the current let-binding actually changes pop-up-windows value to nil (t is the Emacs default). I think we can simply remove pop-up-windows binding from org-no-popups. Feel free to commit this change. I am currently working on org-element-cache and do not spend much time on other patches. Today I am against dropping of `org-no-popups'. Since Ihor confirmed that he had no particular reason to add `pop-up-windows', the minimal change (that should be applied to the bugfix branch) diff --git a/lisp/org-macs.el b/lisp/org-macs.el index 52fc09a06..5f2c29c42 100644 --- a/lisp/org-macs.el +++ b/lisp/org-macs.el @@ -209,7 +209,7 @@ because otherwise all these markers will point to nowhere." (defmacro org-no-popups ( body) "Suppress popup windows and evaluate BODY." - `(let (pop-up-frames pop-up-windows) + `(let (pop-up-frames) ,@body)) ^L 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))) e.g. https://list.orgmode.org/20200503033854.GA28741@singpolyma-beefy 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. Minimal patch works in the cases like https://list.orgmode.org/87h7ij12t8.fsf@localhost and the one raised in this thread. I hope it will not break https://list.orgmode.org/orgmode/871rdtupey@joshuao.com/T/#u as well. In the meanwhile I found the following thread on window creation in Org and `display-buffer' machinery (accessibility concerns were mentioned besides other things): https://list.orgmode.org/87eevw7jqk@gmail.com/T/#u Jack Kamm. [RFC PATCH] Changes to pop-up source buffers. 2020-01-18 17:33 Unfortunately info "(elisp) Displaying Buffers" https://www.gnu.org/software/emacs/manual/html_node/elisp/Displaying-Buffers.html is a reference rather than a guide. I have not realized when `pop-up-frames' or its modern equivalent can be convenient. I have not tried Eric's setup yet. BTW I think the name *Org Help* for the UI buffer could be better. Do you have better ideas? Maybe something like *Org-goto Help*? If we want to change it, we may want to do it now, before users actually use *Org Help* name i.e. in display-buffer-alist. display-buffer-alist did not affect *Org Help* before 399481bad, so we have low odds to break anybody's config yet. Are there use cases when "*Org Help*" is not specific enough and more than one help window should be shown simultaneously or they should be shown in different sides? P.S. Example of complex window setup unrelated to Org: info "(elisp) Frame Layouts with Side Windows" https://www.gnu.org/software/emacs/manual/html_node/elisp/Frame-Layouts-with-Side-Windows.html
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
Marco Wahl writes: > Since org-goto in main is still broken I'll commit the fix for org-goto > which kicks out the use of the macro org-no-popups (but not the macro > itself since it's used elsewhere AFAICS.) > > Max, Ihor! If you see the necessity of refinement please keep going! I am inclined to think that org-no-popups may still be useful to suppress pop-up-frames. However, no so much for pop-up-windows. Note that I introduced pop-up-windows let-binding in org-no-popups in place of overriding display-buffer-alist. However, I did not fully understand pop-up-windows does and the current let-binding actually changes pop-up-windows value to nil (t is the Emacs default). I think we can simply remove pop-up-windows binding from org-no-popups. Feel free to commit this change. I am currently working on org-element-cache and do not spend much time on other patches. > BTW I think the name *Org Help* for the UI buffer could be better. Do you have better ideas? Maybe something like *Org-goto Help*? If we want to change it, we may want to do it now, before users actually use *Org Help* name i.e. in display-buffer-alist. display-buffer-alist did not affect *Org Help* before 399481bad, so we have low odds to break anybody's config yet. Best, Ihor
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
Thanks Eric and Max!
> On Wednesday, 13 Oct 2021 at 19:23, Max Nikulin wrote:
>> Does someone have settings that pins help buffer to particular
>> window/frame of location in a frame (e.g. bottom of "sidebar")?
>
> This is what I use, which is slightly more complex because I have a wide
> landscape monitor and a tall portrait one and want different behaviour
> in each:
>
> #+begin_src emacs-lisp
> (defun esf/display-buffer-in-side-window (buffer alist)
> (let ((fw (/ 80.0 (frame-width
> (display-buffer-in-side-window buffer
> (if (> (frame-width) 120)
> (list (cons 'window-width fw)
>'(side . left)
>'(slot . 0))
>'((window-height . 0.25)
> (side . bottom)
> (slot . 0))
> (setq display-buffer-alist
> '(("^\\*Async Shell Command*" . (display-buffer-no-window))
> ("^magit-[a-z]+: " . (esf/display-buffer-in-side-window))
> ("\\*\\(Backtrace\\|Compile-Log\\|DICT
> .*\\|grep\\|[Hh]elp.*\\|Messages\\|Occur\\|tex-shell\\|vc-\\(diff\\|change-log\\)\\|Warnings\\|WoMan
> .*\\)\\*"
>(esf/display-buffer-in-side-window
> #+end_src
>
> This doesn't pin to a specific frame but does make the pop-ups appear in
> the same place always in each respectively frame. By the way, I use
> exwm so I have one frame per monitor, full screen, generally.
Thanks for the example and the implied teaching!
I experimented with the use of display-buffer-alist and the org-goto UI.
E.g. with the config:
(defun experiment/202110141141 (buffer alist)
(display-buffer-in-side-window
buffer
(list '(window-width . 23)
'(side . right)
'(slot . 0
(setq display-buffer-alist
'(("\\*Org Help\\*" . (experiment/202110141141
AFAICS this has an effect for org-goto. The user can control the
appearance of the org-goto UI.
BTW I think the name *Org Help* for the UI buffer could be better.
Since org-goto in main is still broken I'll commit the fix for org-goto
which kicks out the use of the macro org-no-popups (but not the macro
itself since it's used elsewhere AFAICS.)
Max, Ihor! If you see the necessity of refinement please keep going!
Best regards,
--
Marco
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
On Wednesday, 13 Oct 2021 at 19:23, Max Nikulin wrote:
> Does someone have settings that pins help buffer to particular
> window/frame of location in a frame (e.g. bottom of "sidebar")?
This is what I use, which is slightly more complex because I have a wide
landscape monitor and a tall portrait one and want different behaviour
in each:
#+begin_src emacs-lisp
(defun esf/display-buffer-in-side-window (buffer alist)
(let ((fw (/ 80.0 (frame-width
(display-buffer-in-side-window buffer
(if (> (frame-width) 120)
(list (cons 'window-width fw)
'(side . left)
'(slot . 0))
'((window-height . 0.25)
(side . bottom)
(slot . 0))
(setq display-buffer-alist
'(("^\\*Async Shell Command*" . (display-buffer-no-window))
("^magit-[a-z]+: " . (esf/display-buffer-in-side-window))
("\\*\\(Backtrace\\|Compile-Log\\|DICT
.*\\|grep\\|[Hh]elp.*\\|Messages\\|Occur\\|tex-shell\\|vc-\\(diff\\|change-log\\)\\|Warnings\\|WoMan
.*\\)\\*"
(esf/display-buffer-in-side-window
#+end_src
This doesn't pin to a specific frame but does make the pop-ups appear in
the same place always in each respectively frame. By the way, I use
exwm so I have one frame per monitor, full screen, generally.
HTH,
eric
--
: Eric S Fraga via Emacs 28.0.60, Org release_9.5-93-gd87250
: Latest paper written in org: https://arxiv.org/abs/2106.05096
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
On 13/10/2021 16:44, Marco Wahl wrote: Clearly I'm for kicking out org-no-popups completely. Many details have been mentioned already. The big argument for that change for me is that the code gets simpler. I have no strong opinion. Second patch locally restoring `pop-up-windows' is more suitable for the bugfix branch since it is closer to older behavior. Dropping the macro completely may be a step to future, so first version of patch is more suitable for main (do not forget to drop other variant during merge however). I am not confident with complicated `display-buffer' machinery. It has action argument that looks promising to specify local hint to override default actions but to let users customize `display-buffer-alist'. I have no idea of particular hint for `org-goto' though. Unfortunately `with-output-to-temp-buffer' used there does not allow to specify action argument for `display-buffer'. Unsure if `temp-buffer-setup-hook' can be used to prepare a window for `display-buffer'. Does someone have settings that pins help buffer to particular window/frame of location in a frame (e.g. bottom of "sidebar")?
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
Thanks Ihor! > Marco Wahl writes: > >> My feeling is that the "protection" is good intention but brings more >> harm than good. I think it's not a good idea to enforce a certain >> window setting. I guess the knowing user has an easier path to fine >> tune the org-goto user interface when there is less "protection". > > I fully agree. That was the motivation behind removing > dislpay-buffer-alist in 399481bad1. It is indeed not a good idea to > overwrite user customisations. They can be deliberate. For example, see > https://orgmode.org/list/87h7ij12t8.fsf@localhost > > Max Nikulin writes: >> However current version of macro does not protect against >> >>(setq display-buffer-base-action >> '((display-buffer-reuse-window display-buffer-pop-up-frame) >> (reusable-frames . 0))) >> >> The example is taken from (info "(elisp) Choosing Window Options"). I >> have no idea if such customization can be considered as shooting a foot. > > display-buffer-base-action, if customised by user, can later be > fine-tuned using display-buffer-alist. If necessary, the user can easily > add org-goto popup as an exception. At least, it is my understanding > from reading docs. > > However, pop-up-frames and pop-up-windows are different beasts. They > cannot be fine-tuned by the user to not affect org-goto. AFAIK, the > only way for the user to overcome the problem would be advicing > org-goto. > >> Summary: The org-goto interface today is somewhat broken. I vote for >> taking the occasion and kicking out the macro org-no-popups entirely. >> This way the org-goto interface is functional AFAICS. If problems occur >> on that path we'll take care and action. >> >> Do you agree? > > My second version of the patch also fixes org-goto interface :) Indeed, thanks. > On the other hand, kicking org-no-popups macro completely may be an > option. pop-up-windows and pop-up-frames are obsolete and should not be > used anymore. > > Also, a compromise could be changing org-no-popups to just > (let (pop-up-frames) ...) > > WDYT? Clearly I'm for kicking out org-no-popups completely. Many details have been mentioned already. The big argument for that change for me is that the code gets simpler. But that's just me. All the other fixes may have advantages too. In the first place I want back a usable org-goto interface. I'd like to leave it to you to commit a fix. Best regards, -- Marco
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
Marco Wahl writes: > My feeling is that the "protection" is good intention but brings more > harm than good. I think it's not a good idea to enforce a certain > window setting. I guess the knowing user has an easier path to fine > tune the org-goto user interface when there is less "protection". I fully agree. That was the motivation behind removing dislpay-buffer-alist in 399481bad1. It is indeed not a good idea to overwrite user customisations. They can be deliberate. For example, see https://orgmode.org/list/87h7ij12t8.fsf@localhost Max Nikulin writes: > However current version of macro does not protect against > >(setq display-buffer-base-action >'((display-buffer-reuse-window display-buffer-pop-up-frame) > (reusable-frames . 0))) > > The example is taken from (info "(elisp) Choosing Window Options"). I > have no idea if such customization can be considered as shooting a foot. display-buffer-base-action, if customised by user, can later be fine-tuned using display-buffer-alist. If necessary, the user can easily add org-goto popup as an exception. At least, it is my understanding from reading docs. However, pop-up-frames and pop-up-windows are different beasts. They cannot be fine-tuned by the user to not affect org-goto. AFAIK, the only way for the user to overcome the problem would be advicing org-goto. > Summary: The org-goto interface today is somewhat broken. I vote for > taking the occasion and kicking out the macro org-no-popups entirely. > This way the org-goto interface is functional AFAICS. If problems occur > on that path we'll take care and action. > > Do you agree? My second version of the patch also fixes org-goto interface :) On the other hand, kicking org-no-popups macro completely may be an option. pop-up-windows and pop-up-frames are obsolete and should not be used anymore. Also, a compromise could be changing org-no-popups to just (let (pop-up-frames) ...) WDYT? Best, Ihor
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
Hi Max and all! > On 08/10/2021 17:22, Marco Wahl wrote: >> Max Nikulin writes: >>> On 05/10/2021 23:32, Ihor Radchenko wrote: Max Nikulin writes: I tried come up with the reason why org-no-popup was used in the initial implementation. I think, the reason is avoiding situation like what you may see after running (let ((pop-up-frames t)) (funcall-interactively #'org-goto)) So, removing the macro completely is not a good idea. I have updated the patch that should work without dropping the macro. See the attached. >> >> Please note the documentation of variable `pop-up-windows'. >> >> This variable is provided mainly for backward compatibility and >> should not be used in new code. >> >> The same holds for `pop-up-frames'. >> >> The drop of the macro looks like a good idea to me. Can someone please >> describe the price for dropping macro `org-no-popups'? >> >> @Ihor I do not understand what "situation" you mean. > > Marco, have you tried > (setq pop-up-frames t) > with first version of patch? It shows help in a new separate frame. > Unsure if it is expected behavior even with such customization. > > However current version of macro does not protect against > > (setq display-buffer-base-action > '((display-buffer-reuse-window display-buffer-pop-up-frame) > (reusable-frames . 0))) > > The example is taken from (info "(elisp) Choosing Window Options"). I > have no idea if such customization can be considered as shooting a foot. TBH I don't fully understand that display-buffer stuff. I experimented a little with regards to org-goto. AFAICT org-goto does a good job without macro org-no-popups. IIUC the use of macro org-no-popups in org-goto shall "protect" the org-goto user interface in some sense. You already mentioned that. My feeling is that the "protection" is good intention but brings more harm than good. I think it's not a good idea to enforce a certain window setting. I guess the knowing user has an easier path to fine tune the org-goto user interface when there is less "protection". > http://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=399481bad10845a77f210c9320ff1efee9a312c8 > that caused the current problem changed namely `pop-up-windows' Thanks for the link! AFAICT the change in org-macs.el should not be in that commit since it has nothing to do with org links -- which are the actual concern of that commit. And indeed the change in org-macs.el of that commit broke the org-goto interface some. Summary: The org-goto interface today is somewhat broken. I vote for taking the occasion and kicking out the macro org-no-popups entirely. This way the org-goto interface is functional AFAICS. If problems occur on that path we'll take care and action. Do you agree? Regards, -- Marco
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
On 08/10/2021 17:22, Marco Wahl wrote: Max Nikulin writes: On 05/10/2021 23:32, Ihor Radchenko wrote: Max Nikulin writes: I tried come up with the reason why org-no-popup was used in the initial implementation. I think, the reason is avoiding situation like what you may see after running (let ((pop-up-frames t)) (funcall-interactively #'org-goto)) So, removing the macro completely is not a good idea. I have updated the patch that should work without dropping the macro. See the attached. Please note the documentation of variable `pop-up-windows'. This variable is provided mainly for backward compatibility and should not be used in new code. The same holds for `pop-up-frames'. The drop of the macro looks like a good idea to me. Can someone please describe the price for dropping macro `org-no-popups'? @Ihor I do not understand what "situation" you mean. Marco, have you tried (setq pop-up-frames t) with first version of patch? It shows help in a new separate frame. Unsure if it is expected behavior even with such customization. However current version of macro does not protect against (setq display-buffer-base-action '((display-buffer-reuse-window display-buffer-pop-up-frame) (reusable-frames . 0))) The example is taken from (info "(elisp) Choosing Window Options"). I have no idea if such customization can be considered as shooting a foot. P.S. http://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=399481bad10845a77f210c9320ff1efee9a312c8 that caused the current problem changed namely `pop-up-windows' Joshua mentioned `display-buffer-overriding-action'.
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
Max Nikulin writes: > On 05/10/2021 23:32, Ihor Radchenko wrote: >> Max Nikulin writes: >> I tried come up with the reason why org-no-popup was used in the >> initial >> implementation. I think, the reason is avoiding situation like what you >> may see after running >> (let ((pop-up-frames t)) (funcall-interactively #'org-goto)) >> So, removing the macro completely is not a good idea. >> I have updated the patch that should work without dropping the >> macro. >> See the attached. > > Thank you, Ihor. > > Your updated patch works in default configuration (-Q). I am not > familiar with knobs of `display-buffer' function so I have no idea if > someone may complain that such variant overrides his setup or works > incorrectly with his preferences. Please note the documentation of variable `pop-up-windows'. This variable is provided mainly for backward compatibility and should not be used in new code. The same holds for `pop-up-frames'. The drop of the macro looks like a good idea to me. Can someone please describe the price for dropping macro `org-no-popups'? @Ihor I do not understand what "situation" you mean.
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
On 05/10/2021 23:32, Ihor Radchenko wrote: Max Nikulin writes: I tried come up with the reason why org-no-popup was used in the initial implementation. I think, the reason is avoiding situation like what you may see after running (let ((pop-up-frames t)) (funcall-interactively #'org-goto)) So, removing the macro completely is not a good idea. I have updated the patch that should work without dropping the macro. See the attached. Thank you, Ihor. Your updated patch works in default configuration (-Q). I am not familiar with knobs of `display-buffer' function so I have no idea if someone may complain that such variant overrides his setup or works incorrectly with his preferences.
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
Max Nikulin writes: > Thank you, Ihor. I am a user of alternative `org-goto' interface. I have > tried default one having a couple of windows in the frame (indirect > buffer for subtree, indirect for src block). It seems, previous window > configuration is restored correctly when `org-goto' is finished. > > Curiously `org-no-popup' was introduced namely for `org-goto` Thanks for the testing and digging the old commits. I tried come up with the reason why org-no-popup was used in the initial implementation. I think, the reason is avoiding situation like what you may see after running (let ((pop-up-frames t)) (funcall-interactively #'org-goto)) So, removing the macro completely is not a good idea. I have updated the patch that should work without dropping the macro. See the attached. Best, Ihor >From 438c476066e897b1fc3d758fd48712c1846cd845 Mon Sep 17 00:00:00 2001 Message-Id: <438c476066e897b1fc3d758fd48712c1846cd845.1633451289.git.yanta...@gmail.com> From: Ihor Radchenko Date: Tue, 5 Oct 2021 20:37:02 +0800 Subject: [PATCH] org-goto: Fix window broken arrangement after 399481bad * lisp/org-goto.el (org-goto-location): Make sure that `org-on-popups' macro does not suppress the *Org Help* window pop-up. Fixes https://list.orgmode.org/e169a2f9-72b1-02bb-96c1-6e7368f64...@gmail.com/T/#t --- lisp/org-goto.el | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/lisp/org-goto.el b/lisp/org-goto.el index 0a3470f54..cd832d72c 100644 --- a/lisp/org-goto.el +++ b/lisp/org-goto.el @@ -217,15 +217,16 @@ (defun org-goto-location ( _buf help) (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) - (with-output-to-temp-buffer "*Org Help*" - (princ (format help (if org-goto-auto-isearch - " Just type for auto-isearch." - " n/p/f/b/u to navigate, q to quit.") - (org-fit-window-to-buffer (get-buffer-window "*Org Help*")) - (org-overview) - (setq buffer-read-only t) - (if (and (boundp 'org-goto-start-pos) + (let ((pop-up-windows t)) + (let (temp-buffer-show-function temp-buffer-show-hook) + (with-output-to-temp-buffer "*Org Help*" + (princ (format help (if org-goto-auto-isearch + " Just type for auto-isearch." + " n/p/f/b/u to navigate, q to quit.") + (org-fit-window-to-buffer (get-buffer-window "*Org Help*"))) + (org-overview) + (setq buffer-read-only t) + (if (and (boundp 'org-goto-start-pos) (integer-or-marker-p org-goto-start-pos)) (progn (goto-char org-goto-start-pos) (when (org-invisible-p) -- 2.32.0
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
On 05/10/2021 19:45, Ihor Radchenko wrote: Max Nikulin writes: Regression is caused by commit 399481bad10845a77f210c9320ff1efee9a312c8 Author: Ihor Radchenko Date: Mon May 31 20:47:45 2021 +0800 Do not ignore user-defined display-buffer-alist in org-insert-link See the attached fix. The fix looks reasonable, though I fail to understand why org-no-popup was even used in org-goto-location. We kind of want a popup there. git blame did not reveal anything useful either. Thank you, Ihor. I am a user of alternative `org-goto' interface. I have tried default one having a couple of windows in the frame (indirect buffer for subtree, indirect for src block). It seems, previous window configuration is restored correctly when `org-goto' is finished. Curiously `org-no-popup' was introduced namely for `org-goto` commit b508943d329fed2af457c50afd5f63938b23c58c Author: Achim Gratz Date: Thu Dec 20 10:18:02 2012 +0100 org-compat: new macro org-no-popups * lisp/org-compat.el (org-no-popups): New wrapper macro which let-binds the correct variables to suppress popup windows depending on the Emacs version in use. This is a compile-time decision when byte-compiling. * lisp/org.el (org-get-location, org-switch-to-buffer-other-window): Use the wrapper `org-no-popups´ to let-bind the correct variables for suppression of popup window To emphasize actual changes due to the patch just suggested by Ihor: git diff -b HEAD~ diff --git a/lisp/org-goto.el b/lisp/org-goto.el index 0a3470f54..352bf9f2e 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 @@ -236,7 +235,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 ( alternative-interface)
Re: [PATCH] [BUG] Org 9.5: org-goto UI seems broken
Hi Ihor, > See the attached fix. The fix looks reasonable, though I fail to > understand why org-no-popup was even used in org-goto-location. We kind > of want a popup there. git blame did not reveal anything useful either. > > Adam, can you test the fix in different scenarios first? I do not use > org-goto interface, so I only did a light testing. A quick test seems to indicate that it works again. Please note that I only recently started using org-goto myself, so I can't claim to know all the ways in which it ought to be tested. :) Thanks.
