bmag/emacs-purpose

ebib incompatibility

Closed this issue · 4 comments

leezu commented

Hi @bmag,

if emacs-purpose is loaded, some functionality of ebib package breaks. In particular the editing of multiline fields, for which ebib usually opens a new buffer https://joostkremers.github.io/ebib/ebib-manual.html#editing-multiline-values .

If emacs-purpose is loaded and ebib is instructed to edit a multi-line field, ie. if https://github.com/joostkremers/ebib/blob/50c95ad/ebib.el#L3146-L3154 is called, nothing happens but the following error is logged funcall: Lisp nesting exceeds ‘max-lisp-eval-depth’ or purpose--action-function: Lisp nesting exceeds ‘max-lisp-eval-depth’. I am using emacs-purpose via the default spacemacs configuration of the spacemacs-purpose layer in the develop branch.

Do you have any insights into what could be causing the issue? @joostkremers, the author of ebib is not sure about the cause of this incompatibility. See joostkremers/ebib#128

bmag commented

After reading the code, I surmise there is an infinite-recursion bug in the interaction between Purpose and Ebib. Some commands in Ebib call ebib--pop-to-buffer, which calls pop-to-buffer with an action argument:

(pop-to-buffer buffer
               '((ebib--display-buffer-reuse-window
                  ebib--display-buffer-largest-window
                  display-buffer-pop-up-window
                  display-buffer-pop-up-frame))
               t)

As result, ebib--display-buffer-reuse-window or ebib--display-buffer-largest-window are called, and whichever is called calls switch-to-buffer. The call-chain is basically ebib--pop-to-buffer -> pop-to-buffer -> display-buffer -> ebib--display-buffer-reuse-window -> switch-to-buffer -> END.

The problem is that Purposes advises switch-to-buffer to call display-buffer, while also storing the action argument in an internal variable and passing it again to display-buffer causing infinite recursion:

ebib--pop-to-buffer -> pop-to-buffer
-> display-buffer -> ebib--display-buffer-reuse-window -> switch-to-buffer
-> display-buffer -> ebib--display-buffer-reuse-window -> switch-to-buffer
-> display-buffer -> ...

There are actually two bugs here, one in Purpose and one in Ebib, and together they cause the issue. Purpose doesn't reset the internal variable properly, and I'll have to research why and fix it. On Ebib side, the bug is that ebib--pop-to-buffer treats ebib--display-buffer-reuse-window and ebib--display-buffer-largest-window as "display functions", but they don't behave as proper display functions.

A display function is supposed to display a buffer in a window and return the window, but not select the window or switch to the buffer. Selecting and switching is the job of pop-to-buffer. In particular, ebib--display-buffer-reuse-window and ebib--display-buffer-largest-window shouldn't call switch-to-buffer and select-window. They can use set-window-buffer or window--display-buffer instead. For example, in ebib--display-largest-window replace:

< (select-window window)
< (switch-to-buffer buffer)
> (set-window-buffer window buffer)

Fixing ebib--display-buffer-reuse-window is just a bit trickier because it needs to handle dedication:

< (select-window window)
< (with-ebib-window-nondedicated
<   (switch-to-buffer buffer t))
> (if (window-dedicated-p window)
>     (progn
>       (set-window-dedicated-p window nil)
>       (set-window-buffer window buffer)
>       (set-window-dedicated-p window t))
>   (set-window-buffer window buffer))

There is also an option to modify with-ebib-window-nondedicated to take a window argument instead of using the if wrapper.

@joostkremers can you make these changes to Ebib? I haven't tested them but they should be harmless.

@leezu Fixing the Purpose-side bug could be difficult. In the meantime, if Ebib buffer names have a known structure you can work around the bug by adding matching regexps to purpose-action-function-ignore-buffer-names. This will deactivate Purpose for Ebib buffers and avoid the infinite recursion.

@bmag Thanks for your reply! I didn't realise I was using the display functions incorrectly... I just pushed a commit that fixes this: joostkremers/ebib@f2f4edd.

There is also an option to modify with-ebib-window-nondedicated to take a window argument instead of using the if wrapper.

Yes, I prefer that, actually.

@leezu

if Ebib buffer names have a known structure

Well, multiline edit buffers have a name of the form key-->field, where key is the entry key and field the field being edited. The arrow part --> doesn't change. Most other Ebib buffers have a name that starts with *Ebib-, except for the buffer holding the entry list, which has a name consisting of a number plus a colon plus the name of the .bib file.

It would be easier to find the Ebib buffers by their modes, if Purpose has that option. All Ebib buffers have a dedicated major mode, except for multiline edit buffers, but they have a dedicated minor mode (ebib-multiline-mode).

leezu commented

Thanks @bmag for your help diagnosing the issue and suggesting fixes! Thanks @joostkremers for promptly making the changes to ebib! I confirmed the changes in ebib avoid the infinite recursion and closed joostkremers/ebib#128

bmag commented

Closing this issue as it was resolved. I opened a new issue for tracking the bug in Purpose.

@joostkremers thank for making the fixes. Purpose doesn't have an option to exclude buffers by mode, although catching the --> arrow part might have worked and also wrapping the display functions in without-purpose is an option, but I'm glad it's not necessary now.