Adding olivetti to GNU ELPA

Adding olivetti to GNU ELPA

[Paul W. Rankin]

Hello,

I'd be quite happy if FSF would accept olivetti into GNU ELPA. I already
have a copyright assignment with FSF for fountain-mode, and I think I
listed olivetti on it at the same time.

The repository is here: https://github.com/rnkn/olivetti

The package is modestly popular (271 GitHub Likes) but it tends to run
into issues when Emacs display code changes upon major releases, and
unfortunately I don't have enough time to devote to continued
maintenance.

Thanks, I hope you'll take this little bird under your wing :)

-- 
https://www.paulwrankin.com

Re: Adding olivetti to GNU ELPA

[Eli Zaretskii]

> Date: Thu, 25 Apr 2019 04:32:39 -0400
> From: "Paul W. Rankin" <hello@paulwrankin.com>
> 
> I'd be quite happy if FSF would accept olivetti into GNU ELPA. I already
> have a copyright assignment with FSF for fountain-mode, and I think I
> listed olivetti on it at the same time.

Your assignment is for "past and future changes" to Emacs, not
specifically to any package.

I think the current procedure is that you should explicitly write that
you contribute a package to Emacs, and then your assignment will cover
that package as well.

Thanks.

Re: Adding olivetti to GNU ELPA

[Paul W. Rankin]

On Thu, 25 Apr 2019, at 6:56 PM, Eli Zaretskii wrote:
> Your assignment is for "past and future changes" to Emacs, not
> specifically to any package.
> 
> I think the current procedure is that you should explicitly write that
> you contribute a package to Emacs, and then your assignment will cover
> that package as well.

I don't understand what you mean. Is my first email enough to cover olivetti?

Re: Adding olivetti to GNU ELPA

[Eli Zaretskii]

> Date: Thu, 25 Apr 2019 08:32:16 -0400
> From: "Paul W. Rankin" <hello@paulwrankin.com>
> Cc: emacs-devel@gnu.org
> 
> > I think the current procedure is that you should explicitly write that
> > you contribute a package to Emacs, and then your assignment will cover
> > that package as well.
> 
> I don't understand what you mean. Is my first email enough to cover olivetti?

I think something more explicit is needed, not just a request to add
olivetti to ELPA.  The main issue here is that your assignment is for
Emacs, and olivetti is not currently part of Emacs.  We need some
explicit wording to allow us to consider it part of Emacs, so that
your copyright assignment would cover it.

Re: Adding olivetti to GNU ELPA

[Paul W. Rankin]

On Thu, 25 Apr 2019, at 10:44 PM, Eli Zaretskii wrote:
> I think something more explicit is needed, not just a request to add
> olivetti to ELPA.  The main issue here is that your assignment is for
> Emacs, and olivetti is not currently part of Emacs.  We need some
> explicit wording to allow us to consider it part of Emacs, so that
> your copyright assignment would cover it.

I see. I've pushed a commit and tag that that explicitly says it's part
of Emacs and (c) FSF, and signed that commit/tag.

Re: Adding olivetti to GNU ELPA

[Eli Zaretskii]

> Date: Thu, 25 Apr 2019 09:01:01 -0400
> From: "Paul W. Rankin" <hello@paulwrankin.com>
> Cc: emacs-devel@gnu.org
> 
> On Thu, 25 Apr 2019, at 10:44 PM, Eli Zaretskii wrote:
> > I think something more explicit is needed, not just a request to add
> > olivetti to ELPA.  The main issue here is that your assignment is for
> > Emacs, and olivetti is not currently part of Emacs.  We need some
> > explicit wording to allow us to consider it part of Emacs, so that
> > your copyright assignment would cover it.
> 
> I see. I've pushed a commit and tag that that explicitly says it's part
> of Emacs and (c) FSF, and signed that commit/tag.

Thanks.

Re: Adding olivetti to GNU ELPA

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > I think the current procedure is that you should explicitly write that
  > you contribute a package to Emacs, and then your assignment will cover
  > that package as well.

Yes, that's sufficient.  When we get that message we will send it to
the FSF archives.

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

Re: Adding olivetti to GNU ELPA

[Paul W. Rankin]

I've pushed some changes provided by Stefan for compatibility with Emacs 
27.x.

I'm sorry to say but this goes a bit over my head... From Emacs 24 - 27 
there seem to be three hooks that have shifting/overlapping 
capabilities. The hooks are:

- window-configuration-change-hook
- post-command-hook
- window-size-change-functions

When I first created the minor mode, window-configuration-change-hook 
was enough for all situations I encountered.

With the introduction of display-line-numbers-mode, Eli suggested I 
change this to post-command-hook. This seemed not to cover a few window 
resize events, so I added window-configuration-change-hook back, but 
I've since received complaints from people experiencing laggy input, but 
then also errors with Emacs 27.x.

At this point I've become too confused to find an adequate solution 
between the Three Hooks so I (and I'm sure those who use the minor mode) 
would love if someone more familiar with the changing nature of the 
Three Hooks could provide a solution...

I *think* what is needed is a way to apply the hooks contingent on 
emacs-major-version <= 24 and <= 25 or 26(?) and <= 27.

Much appreciation :)

-- 
https://www.paulwrankin.com

Re: Adding olivetti to GNU ELPA

[Eli Zaretskii]

> From: "Paul W. Rankin" <hello@paulwrankin.com>
> Cc: emacs-devel@gnu.org
> Date: Wed, 08 May 2019 14:07:24 +1000
> 
> I'm sorry to say but this goes a bit over my head... From Emacs 24 - 27 
> there seem to be three hooks that have shifting/overlapping 
> capabilities. The hooks are:
> 
> - window-configuration-change-hook
> - post-command-hook
> - window-size-change-functions
> 
> When I first created the minor mode, window-configuration-change-hook 
> was enough for all situations I encountered.
> 
> With the introduction of display-line-numbers-mode, Eli suggested I 
> change this to post-command-hook. This seemed not to cover a few window 
> resize events, so I added window-configuration-change-hook back, but 
> I've since received complaints from people experiencing laggy input, but 
> then also errors with Emacs 27.x.
> 
> At this point I've become too confused to find an adequate solution 
> between the Three Hooks so I (and I'm sure those who use the minor mode) 
> would love if someone more familiar with the changing nature of the 
> Three Hooks could provide a solution...

I agree that we have a royal mess with these hooks.  And if you think
the above 3 are all of it, then, unfortunately, they aren't: there are
also pre-redisplay-functions and window-scroll-functions -- those may
be necessary if your mode needs to pay attention to when the screen
space used by line numbers changes, either because the buffer is
scrolled past the point where the line numbers need more or less
digits than before, or because the user customizes the line-number
face in a way which changes the font used by the face, with your mode
already active.

I'd like to try to help you, but I'm not sure I understand the problem
well enough.  Are you asking whether what you did could be simplified
by using less hooks?  Or are you saying that the current code doesn't
yet handle well some situations?  In the latter case, could you please
describe those problematic situations?

> I *think* what is needed is a way to apply the hooks contingent on 
> emacs-major-version <= 24 and <= 25 or 26(?) and <= 27.

Yes, and there's nothing wrong with that for a package that wants to
support multiple Emacs versions.

Thanks.

Re: Adding olivetti to GNU ELPA

[Stefan Monnier]

> At this point I've become too confused to find an adequate solution between
> the Three Hooks so I (and I'm sure those who use the minor mode) would love
> if someone more familiar with the changing nature of the Three Hooks could
> provide a solution...

AFAIK part of the purpose behind the change in the semantics of
window-size-change-functions (and related hooks) in Emacs-27 is so that
modes like yours can use window-size-change-functions and nothing else.

So I'd recommend you use that in Emacs-27 and file a bug report if it's
not sufficient.


        Stefan

Re: Adding olivetti to GNU ELPA

[Stephen Leake]

"Paul W. Rankin" <hello@paulwrankin.com> writes:

> I *think* what is needed is a way to apply the hooks contingent on
> emacs-major-version <= 24 and <= 25 or 26(?) and <= 27.

you can use the variables emacs-version (a string), emacs-major-version
(an integer), emacs-minor-version (an integer).

Something like:

(cl-ecase emacs-major-version
(23
    (add-hook ...)
    )

(24
    (add-hook ...)

...

)


-- 
-- Stephe

Re: Adding olivetti to GNU ELPA

[Eli Zaretskii]

> From: Stephen Leake <stephen_leake@stephe-leake.org>
> Date: Wed, 08 May 2019 07:45:04 -0800
> 
> you can use the variables emacs-version (a string), emacs-major-version
> (an integer), emacs-minor-version (an integer).
> 
> Something like:
> 
> (cl-ecase emacs-major-version
> (23
>     (add-hook ...)
>     )
> 
> (24
>     (add-hook ...)

We have version-comparison functions in subr.el.

Re: Adding olivetti to GNU ELPA

[Paul W. Rankin]

On Wed, May 08 2019, Eli Zaretskii wrote:

>> From: "Paul W. Rankin" <hello@paulwrankin.com>
>> Cc: emacs-devel@gnu.org
>> Date: Wed, 08 May 2019 14:07:24 +1000
>> 
>> I'm sorry to say but this goes a bit over my head... From Emacs 24 - 
>> 27 
>> there seem to be three hooks that have shifting/overlapping 
>> capabilities. The hooks are:
>> 
>> - window-configuration-change-hook
>> - post-command-hook
>> - window-size-change-functions
>> 
>> When I first created the minor mode, window-configuration-change-hook 
>> was enough for all situations I encountered.

> I agree that we have a royal mess with these hooks.  And if you think
> the above 3 are all of it, then, unfortunately, they aren't: there are
> also pre-redisplay-functions and window-scroll-functions -- those may
> be necessary if your mode needs to pay attention to when the screen
> space used by line numbers changes, either because the buffer is
> scrolled past the point where the line numbers need more or less
> digits than before, or because the user customizes the line-number
> face in a way which changes the font used by the face, with your mode
> already active.
>
> I'd like to try to help you, but I'm not sure I understand the problem
> well enough.  Are you asking whether what you did could be simplified
> by using less hooks?  Or are you saying that the current code doesn't
> yet handle well some situations?  In the latter case, could you please
> describe those problematic situations?

I think there are three separate but perhaps related problems. At its 
most basic, olivetti is really just a single main function, which could 
be distilled to:

    (set-window-margins
      (current-window) (/ 2 (- (window-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.

For compatibility with changes in Emacs 25.x, we also reset the margins 
to 0 before any window splitting, to prevent the combined window width 
being too large to split.

So the three problems I'm running into are:

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.

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)

It would be nice to achieve consistency with these.

3. Issues scrolling with display-line-numbers-mode, but I don't mind 
about this.

>> I *think* what is needed is a way to apply the hooks contingent on 
>> emacs-major-version <= 24 and <= 25 or 26(?) and <= 27.
>
> Yes, and there's nothing wrong with that for a package that wants to
> support multiple Emacs versions.

My difficulty is that I don't understand enough how these hooks have 
changed from 24 -> 27. Stefan tried to explain it to me, but... I still 
didn't get it :(

-- 
https://www.paulwrankin.com

Re: Adding olivetti to GNU ELPA

[Eli Zaretskii]

> From: "Paul W. Rankin" <hello@paulwrankin.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 09 May 2019 16:54:48 +1000
> 
> So the three problems I'm running into are:
> 
> 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.
> 
> 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)
> 
> It would be nice to achieve consistency with these.
> 
> 3. Issues scrolling with display-line-numbers-mode, but I don't mind 
> about this.
> 
> >> I *think* what is needed is a way to apply the hooks contingent on 
> >> emacs-major-version <= 24 and <= 25 or 26(?) and <= 27.
> >
> > Yes, and there's nothing wrong with that for a package that wants to
> > support multiple Emacs versions.
> 
> My difficulty is that I don't understand enough how these hooks have 
> changed from 24 -> 27. Stefan tried to explain it to me, but... I still 
> didn't get it :(

My suggestion would be to deal with each problem separately.  Are
these problems described somewhere in more detail, including perhaps
reproduction recipes?  If they are, just point to those descriptions,
otherwise I suggest to file bug reports for them (or ask the users who
complained to do so).  Then we should ask Martin to look into that,
since he's our best expert on this stuff.

For Emacs 27, Stefan's recommendation is clear, I think: use only one
hook he mentioned, and if there are any problems, have them reported,
because they might be bugs in how we implemented that hook.

HTH

Re: Adding olivetti to GNU ELPA

[martin rudalics]

 >     (set-window-margins
 >       (current-window) (/ 2 (- (window-width) olivetti-body-width)))

What is 'current-window' and how does it relate to 'selected-window'?

 > 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

 > 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?

 > 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?

 > 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?

martin

Re: Adding olivetti to GNU ELPA

[Stefan Monnier]

> '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?

I think the issue here is that even when margins "aren't modified",
olivetti does change them, because it first sets them to 0 before
setting them again to the previous value, as can be seen in
olivetti--set-margins (which is function called for post-command-hook,
window-size-change-functions, and friends):

    (defun olivetti--set-margins (&optional window-or-frame)
      [...]
        (olivetti-reset-window window-or-frame)
        [...]
          (set-window-margins window-or-frame left-margin right-margin))))
    
    (defun olivetti-reset-window (window)
      [...]
      (set-window-margins window nil))

I don't know why it starts by calling olivetti-reset-window.
But I think that for Emacs-27, it doesn't matter because it should only
use window-size-change-functions so the extra (set-window-margins window
nil) shouldn't be problematic.


        Stefan

Re: Adding olivetti to GNU ELPA

[martin rudalics]

> I think the issue here is that even when margins "aren't modified",
> olivetti does change them, because it first sets them to 0 before
> setting them again to the previous value, as can be seen in
> olivetti--set-margins (which is function called for post-command-hook,
> window-size-change-functions, and friends):
>
>      (defun olivetti--set-margins (&optional window-or-frame)
>        [...]
>          (olivetti-reset-window window-or-frame)
>          [...]
>            (set-window-margins window-or-frame left-margin right-margin))))
>
>      (defun olivetti-reset-window (window)
>        [...]
>        (set-window-margins window nil))
>
> I don't know why it starts by calling olivetti-reset-window.
> But I think that for Emacs-27, it doesn't matter because it should only
> use window-size-change-functions so the extra (set-window-margins window
> nil) shouldn't be problematic.

'window-size-change-functions' gets also called when the total or text
height of the window changes.  In either of these cases, resetting the
margins is gratuitous.

martin

Re: Adding olivetti to GNU ELPA

[Paul W. Rankin]

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

Re: Adding olivetti to GNU ELPA

[Paul W. Rankin]

>> 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:

p.s. Sorry Stefan I just found what you were referring to by 
"global/local" in the window-configuration-change-hook documentation.

--
https://www.paulwrankin.com

Re: Adding olivetti to GNU ELPA

[Stefan Monnier]

> 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:

Every hook has a global part shared among all buffers and then in each
buffer it has a local part, which can be different for each buffer.
This is what the `local` argument to `add-hook` and `remove-hook` refer to.


        Stefan

Re: Adding olivetti to GNU ELPA

[Joost Kremers]

On Tue, May 14 2019, Paul W. Rankin wrote:
>    (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)))

I have no idea if this helps (I'm not entirely sure if I correctly 
understand the issues you're facing), but FWIW, this is what I do 
in `visual-fill-column-mode` 
<https://github.com/joostkremers/visual-fill-column>, which must 
solve essentially the same problem:

```
(defun visual-fill-column-mode--enable ()
  "Set up `visual-fill-column-mode' for the current buffer."
  (add-hook 'window-configuration-change-hook 
  #'visual-fill-column--adjust-window 'append 'local)
  (if (>= emacs-major-version 26)
      (add-hook 'window-size-change-functions 
      #'visual-fill-column--adjust-frame 'append))
  (visual-fill-column--adjust-window))
```

So I use `window-configuration-change-hook` in every Emacs version 
and `window-size-change-functions` in Emacs >= 26. ISTR that it's 
not possible to forego `window-configuration-change-hook` in Emacs 
26, but I can't remember why...

I have at least one report of this working on Emacs 27, but I must 
admit that I'm running Emacs 26 myself, so perhaps there are 
problems this particular user doesn't care about.

`visual-fill-column--adjust-frame simply calls 
`visual-fill-column--adjust-window` for every window on the 
relevant frame. In Emacs 27, this function must be in the global 
part of `window-size-change-functions`, because Emacs 27 changed 
the way the local part is called. `visual-fill-column-mode` could 
of course be adapted to use the local part of the hook, but there 
are more hooks involved with window size/config changes in Emacs 
27 and I haven't looked into which ones I should use.

 `visual-fill-column--adjust-window` does the following:

```
(defun visual-fill-column--adjust-window ()
  "Adjust the window margins and fringes."
  ;; Only run when we're really looking at a buffer that has 
  v-f-c-mode enabled. See #22.
  (when (buffer-local-value 'visual-fill-column-mode 
  (window-buffer (selected-window)))
    (set-window-fringes (get-buffer-window (current-buffer)) nil 
    nil visual-fill-column-fringes-outside-margins)
    (if (>= emacs-major-version 25)
        (set-window-parameter (get-buffer-window (current-buffer)) 
        'split-window #'visual-fill-column-split-window))
    (visual-fill-column--set-margins)))
```

So it has additional code for Emacs >= 25 to set the window 
parameter `split-window` to `visual-fill-column-split-window`, 
which unsets the margins before calling `split-window`.

In addition, there is a function 
`visual-fill-column-split-window-sensibly`, which can be used as 
the value of `split-window-preferred-function`. It handles virtual 
splitting of windows with wide margins.

HTH


--
Joost Kremers
Life has its moments

Re: Adding olivetti to GNU ELPA

[Stefan Monnier]

> (defun visual-fill-column-mode--enable ()
>  "Set up `visual-fill-column-mode' for the current buffer."
>  (add-hook 'window-configuration-change-hook
> #'visual-fill-column--adjust-window 'append 'local)
>  (if (>= emacs-major-version 26)
>      (add-hook 'window-size-change-functions
> #'visual-fill-column--adjust-frame 'append))
>  (visual-fill-column--adjust-window))

If you fundamentally only care about the size of the windows, then in
Emacs-27 all you need is

      (add-hook 'window-size-change-functions
                #'visual-fill-column--adjust-window 'append 'local)

> frame. In Emacs 27, this function must be in the global part of
> `window-size-change-functions`,

Not really: in Emacs-26, if it's on the local-part of the hook, then it
will fail to be called when your window's size is changed as
a side-effect of some other window being resized (this is fixed in
Emacs-27).  In Emacs-27, on the contrary it can be in the local part of
the hook and can limit its action to the window passed as an argument
rather than having to cycle through all windows on the selected frame,
because the local part of the hook is run (separately) for every window
whose size has changed.


        Stefan

Re: Adding olivetti to GNU ELPA

[Joost Kremers]

On Tue, May 14 2019, Stefan Monnier wrote:
>> (defun visual-fill-column-mode--enable ()
>>  "Set up `visual-fill-column-mode' for the current buffer."
>>  (add-hook 'window-configuration-change-hook
>> #'visual-fill-column--adjust-window 'append 'local)
>>  (if (>= emacs-major-version 26)
>>      (add-hook 'window-size-change-functions
>> #'visual-fill-column--adjust-frame 'append))
>>  (visual-fill-column--adjust-window))
>
> If you fundamentally only care about the size of the windows,

Yes, specifically their width.

> then in
> Emacs-27 all you need is
>
>       (add-hook 'window-size-change-functions
>                 #'visual-fill-column--adjust-window 'append 
>                 'local)
>
>> frame. In Emacs 27, this function must be in the global part of
>> `window-size-change-functions`,
>
> Not really: in Emacs-26, if it's on the local-part of the hook, 
> then it
> will fail to be called when your window's size is changed as
> a side-effect of some other window being resized (this is fixed 
> in
> Emacs-27).  In Emacs-27, on the contrary it can be in the local 
> part of
> the hook and can limit its action to the window passed as an 
> argument
> rather than having to cycle through all windows on the selected 
> frame,
> because the local part of the hook is run (separately) for every 
> window
> whose size has changed.

Ah, I see.

The problem I ran into (and with prompted my remark above) was 
that in Emacs 26, the functions in window-size-change-functions 
are called with the frame as argument, even the functions in the 
local part of the hook. In Emacs 27, the functions in the global 
part are called with the frame as argument, while the functions in 
the local part are called with the window as argument. Since the 
function that `visual-fill-column-mode` puts in the hook assumes 
the frame as argument, the simplest change seemed to be to put it 
in the global part. It seems that wasn't the only solution, and 
arguably it's not the best solution either...

I'll set up a VirtualBox so I can test it with Emacs 27 and make 
the change when I have some time.

Thanks for the info. 

-- 
Joost Kremers
Life has its moments

Re: Adding olivetti to GNU ELPA

[Stefan Monnier]

> The problem I ran into (and with prompted my remark above) was that in Emacs
> 26, the functions in window-size-change-functions are called with the frame
> as argument, even the functions in the local part of the hook.

That's right: that's because it didn't actually pay any attention to the
local part, instead it just ran the hook (both local and global parts)
in the buffer that happened to be current at that point.


        Stefan

Re: Adding olivetti to GNU ELPA

[Paul W. Rankin]

I think the patch below should cover all the bases, from 24 -> 27, 
using window-configuration-change-hook and 
window-size-change-functions:

- for Emacs 24, we just use window-configuration-change-hook
- for Emacs 25 & 26 we use the combination architected by Stefan
- for Emacs 27, just window-size-change-functions

This doesn't cover for window size changes in Emacs 26 (25?) when 
display-line-numbers-mode scrolls from 99 to 100, but I spent 
hours trying to get that working a while back to no avail... 
*shrug*


diff --git a/olivetti.el b/olivetti.el
index a64607e..6e140ce 100644
--- a/olivetti.el
+++ b/olivetti.el
@@ -259,7 +259,7 @@ fraction of the window width."
              (read-number "Set text body width (integer or 
              float): "
                           olivetti-body-width))))
   (setq olivetti-body-width n)
-  (olivetti-set-margins)
+  (olivetti-set-all-margins)
   (message "Text body width set to %s" olivetti-body-width))
 
 (defun olivetti-expand (&optional arg)
@@ -273,7 +273,7 @@ If prefixed with ARG, incrementally decrease."
                   ((floatp olivetti-body-width)
                    (+ olivetti-body-width (* 0.01 p))))))
     (setq olivetti-body-width (olivetti-safe-width n 
     (selected-window))))
-  (olivetti-set-margins)
+  (olivetti-set-all-margins)
   (message "Text body width set to %s" olivetti-body-width)
   (set-transient-map
    (let ((map (make-sparse-keymap)))
@@ -312,10 +312,17 @@ body width set with `olivetti-body-width'."
   :lighter olivetti-lighter
   (if olivetti-mode
       (progn
-        (add-hook 'window-size-change-functions
-                  #'olivetti-set-margins t t)
-        (add-hook 'window-configuration-change-hook
-                  'olivetti-set-all-margins t t)
+        (cond ((<= emacs-major-version 24)
+               (add-hook 'window-configuration-change-hook
+                         #'olivetti-set-all-margins t t))
+              ((<= emacs-major-version 26)
+               (add-hook 'window-configuration-change-hook
+                         #'olivetti-set-all-margins t t)
+               (add-hook 'window-size-change-functions
+                         #'olivetti-set-margins t t))
+              ((<= 27 emacs-major-version)
+               (add-hook 'window-size-change-functions
+                         #'olivetti-set-margins t t)))
         (add-hook 'change-major-mode-hook
                   #'olivetti-reset-all-windows nil t)
         (setq-local split-window-preferred-function


-- 
https://www.paulwrankin.com

Re: Adding olivetti to GNU ELPA

[martin rudalics]

 > 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.

Always resetting the margins is bad.  I'd suggest you devise a special
'olivetti-split-window' function and set the 'split-window' parameter
of every window displaying a buffer in olivetti mode to that function.

'olivetti-split-window' would reset the margins iff the window shall
be split side-by-side, set the window's 'split-window' parameter
temporarily to nil, call 'split-window' and assign the 'split-window'
parameter again for all windows in question.

martin

Re: Adding olivetti to GNU ELPA

[Paul W. Rankin]

On Mon, May 20 2019, martin rudalics wrote:
> Always resetting the margins is bad.  I'd suggest you devise a 
> special
> 'olivetti-split-window' function and set the 'split-window' 
> parameter
> of every window displaying a buffer in olivetti mode to that 
> function.
>
> 'olivetti-split-window' would reset the margins iff the window 
> shall
> be split side-by-side, set the window's 'split-window' parameter
> temporarily to nil, call 'split-window' and assign the 
> 'split-window'
> parameter again for all windows in question.

Hmm this isn't it, the code already works this way (see L#191).

-- 
https://www.paulwrankin.com

Re: Adding olivetti to GNU ELPA

[martin rudalics]

> Hmm this isn't it, the code already works this way (see L#191).

Sorry, what is L#191?

martin

Re: Adding olivetti to GNU ELPA

[Paul W. Rankin]

> Sorry, what is L#191?

Line 191

Re: Adding olivetti to GNU ELPA

[martin rudalics]

>> Sorry, what is L#191?
>
> Line 191

I'm still too dense.  Line 191 of what?

martin

Re: Adding olivetti to GNU ELPA

[Paul W. Rankin]

On Tue, May 21 2019, martin rudalics wrote:
> I'm still too dense.  Line 191 of what?

It's line 191 of olivetti.el. It's in GNU ELPA on branch 
externals/olivetti.

That's just the line with the function to reset the margins of a 
window if window-parameter split-window is olivetti-split-window.

I think the main thing is how and when the window margin setting 
and resetting occurs. This is how olivetti now adds its hooks:

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

Which seems okay to me, but I'm a far cry from an authoritive 
voice.

Re: Adding olivetti to GNU ELPA

[martin rudalics]

 > It's line 191 of olivetti.el. It's in GNU ELPA on branch externals/olivetti.

I see it now.  Where do you reinstall the 'split-window' parameter
after splitting?

martin

Re: Adding olivetti to GNU ELPA

[Paul W. Rankin]

On Tue, May 21 2019, martin rudalics wrote:

> I see it now.  Where do you reinstall the 'split-window' 
> parameter
> after splitting?

It's in the main function, olivetti-set-margins @ line 177.

Re: Adding olivetti to GNU ELPA

[martin rudalics]

 >> I see it now.  Where do you reinstall the 'split-window' parameter
 >> after splitting?
 >
 > It's in the main function, olivetti-set-margins @ line 177.

OK.  Then I'm afraid you have to do the following: Whenever olivetti
mode is active in at least _one_ live window on a frame you have to
use 'olivetti-split-window' on _all_ internal windows of that frame
too.  You can use 'walk-window-tree' for setting and resetting this
parameter and the margins of all olivetti mode windows.  It should not
be too much of a hassle (and tell me immediately when you encounter
any problems).

The reason is that 'split-window' may be called for internal windows
too.  For example, if you want to make a new window on the left or
right side of a frame, Emacs may want to split the root window of that
frame.  In this case it will check the sizes of all live subwindows of
the root window and if any of these windows has too large margins, the
split may fail because of insufficient space for the new window.

martin

Re: Adding olivetti to GNU ELPA

[Paul W. Rankin]

On Wed, May 22 2019, martin rudalics wrote:
> OK.  Then I'm afraid you have to do the following: Whenever 
> olivetti
> mode is active in at least _one_ live window on a frame you have 
> to
> use 'olivetti-split-window' on _all_ internal windows of that 
> frame
> too.  You can use 'walk-window-tree' for setting and resetting 
> this
> parameter and the margins of all olivetti mode windows.  It 
> should not
> be too much of a hassle (and tell me immediately when you 
> encounter
> any problems).
>
> The reason is that 'split-window' may be called for internal 
> windows
> too.  For example, if you want to make a new window on the left 
> or
> right side of a frame, Emacs may want to split the root window 
> of that
> frame.  In this case it will check the sizes of all live 
> subwindows of
> the root window and if any of these windows has too large 
> margins, the
> split may fail because of insufficient space for the new window.

Ah awesome. Thanks Martin, I think this well end up solving the 
funny little peculiarities I've encountered, and also the problem 
I had with adding a side-window to a frame containing 2 x olivetti 
windows split horizontally (failing due to insufficient space).

I appreciate your insights.