Re: Adding olivetti to GNU ELPA

["Paul W. Rankin" <hello@paulwrankin.com>]

Sorry for my delayed reply...

On Thu, May 09 2019, martin rudalics wrote:

>>     (set-window-margins
>>       (current-window) (/ 2 (- (window-width) olivetti-body-width)))
>
> What is 'current-window' and how does it relate to 'selected-window'?

Sorry, this should be selected-window, and also window-width -> 
window-total-width.

i.e. The kernel of the minor mode should just be computing when to run 
this:

    (set-window-margins
     (selected-window) (/ 2 (- (window-total-width)
                               olivetti-body-width)))

>> There are some other ancillary functions, but really it's just a
>> case of running this function in all windows displaying the current
>> buffer when olivetti-body-width changes or when the window width
>> changes.
>
> What does "current buffer" refer to here?  I suppose you want to run
> "this function" whenever the body width of a window showing a buffer
> with olivetti mode enabled changes.  So with Emacs 27 it should
> suffice to add your function to `window-size-change-functions' for
> each buffer where olivetti mode is enabled

I was thinking that current buffer is just the result of function 
current-buffer, but yes, your description is accurate.

>> 1. Some users have complained of lagging input, which seems to be
>> due to olivetti setting the margins too often, i.e. on too many
>> hooks. So using the minimal number of hooks is preferable.
>
> 'set-window-margins' doesn't do much when margins don't change.  So I
> suppose that for some reason olivetti mode wants margins to change too
> often.  If so, could you try to find out why?

The problem I ran into was with Emacs 25 the window splitting code 
changed such that windows with large margins would calculate as being 
too small to split and result in an error. So I needed to make olivetti 
first cycle through all "olivetti-ized windows" and reset all the 
margins to 0, then allow the desired window splitting to occur, then 
cycle through and set up the margins again.

But also, the scenarios in which window-configuration-change-hook ran 
seemed to change too, so on Eli's recommendation I added the above 
function to post-command-hook (Eli, sorry again for my poor and 
argumentative communication back then).

>> 2. But I've experienced olivetti failing to set the margins during a 
>> few common cases, e.g.:
>>   - splitting window with icomplete "*Completions*" buffer
>>   - splitting window with ispell-word "*Choices*" buffer
>>   - splitting window with magit-status
>>   - splitting window vertically with any buffers in a side-window in 
>>   the  same frame (which results in window being too large to split 
>>   -- I  think this is a separate issue)
>
> Too _large_ to split?

Sorry, what I wrote was incorrect. I think this issue is wholly 
separate. It occurs when:

1. olivetti is active in two windows, split above/below
2. a call is made to display-buffer-in-side-window

I've stepped through what's happening here and it seems to be when 
display-buffer-in-side-window calls split-window-no-error, which I think 
has something to do with olivetti wanting split-window or 
split-window-sensibly.

But this is reaching the extent of my skills/knowledge.

>> It would be nice to achieve consistency with these.
>
> Are these failures due to the fact that a releated hook is not run or
> are there other issues?

I am pretty sure it's because the right hook is not being called; from 
Stefan:

> The old window-size-change-functions was fundamentally broken for your 
> use-case because it wasn't run in all cases where you needed it.

> More specifically there were several different "missing" cases 
> (typically creation of new windows as well as changes in size that are 
> consequences of resizing *other* windows). So in Emacs<27, you need to 
> use the *global* part of window-size-change-functions (to react to 
> resizing in other windows) as well as (the local part of) 
> window-configuration-change-hook (to detect window creation) to cover 
> all relevant cases, AFAIK.

> But with the changes in Emacs-27 the local part of 
> window-size-change-functions should be all you need in your case. [ 
> And in Emacs<27 you should be able to replace the use of 
> post-command-hook by the use of (the part of 
> window-configuration-change-hook). ]

Stefan, sorry to be so dense, but I'm not exactly sure what you mean be 
the global/local distinction above. Does this refer to:

    olivetti--set-margins           ;; <- global?
    olivetti--set-window-margins    ;; <- local?

As you've all said, for Emacs 27 it appears that this all should be 
solved with just window-size-change-functions, but the minor mode is 
supposed to be compatible with Emacs >= 24.5, so I think something like 
the following is needed:

    (cond ((<= emacs-major-version 24)
           (add-hook 'window-configuration-change-hook
                     'olivetti--??? t t))
          ((<= emacs-major-version 26)
           (add-hook 'window-configuration-change-hook
                     'olivetti--??? t t)
           (add-hook 'window-size-change-functions
                     'olivetti--??? t t))
          ((<= 27 emacs-major-version)
           (add-hook 'window-size-change-functions
                     'olivetti--??? t t)))

But I'm not sure which of olivetti--set-margins or 
olivetti--set-window-margins should be used in every case, or the best 
solution for Emacs <= 26 (e.g. if window-configuration-change-hook 
should be replaced with post-command-hook there).

Sorry for the lengthy email, and thanks for all your help :)

--
https://www.paulwrankin.com