bug#29111: 26.0.90; Erc keep-place module broken with new default of switch-to-buffer-preserve-window-point

[Jay Kamat]

Hi,

> "keep place" means "preserve point" so it's somewhat surprising that
> this is a problem in the first place.  Can you explain why and how the
> keep place mechanism and the ‘switch-to-buffer-preserve-window-point’
> mechanisms differ?  Is ‘window-point-insertion-type’ involved somehow?

I'm not very familiar with emacs internals, so all of this could be
wrong, but here's my best shot of explaining why this is a problem.

By default, point stays at the bottom of erc buffers as new messages
come in. If point is not at the bottom, point stays where it is (so even
when new messages come in, point wont move).

Keep place adds a hook so that, when a new message comes in on a
non-visible buffer *and* point is at the bottom, point is moved up by
one line. Doing this means that point stays on the last 'read'
message. Here's the relevant code from keep-place:

> (defun erc-keep-place (ignored)
> "Move point away from the last line in a non-selected ERC buffer."
> (when (and (not (eq (window-buffer (selected-window))
>                     (current-buffer)))
>            (>= (point) erc-insert-marker))
>   (deactivate-mark)
>   (goto-char (erc-beg-of-input-line))
>   (forward-line -1)))

Previously, this worked fine, since moving point on non-visible buffers
meant the movement persisted till the buffer was next 'shown'. However,
the new default of `switch-to-buffer-preserve-window-point' ensures that
point stays wherever it was 'last seen'. I'm not sure how it does it,
but I think it's saving point when a buffer is hidden and restoring it
when it's seen again

> (when (and entry
>                  (or (eq switch-to-buffer-preserve-window-point t)
>                      displayed))
>         ;; Try to restore start and point of buffer in the selected
>         ;; window (Bug#4041).
>         (set-window-start (selected-window) (nth 1 entry) t)
>         (set-window-point nil (nth 2 entry)))


>> Alternatively, we could allow ‘switch-to-buffer-preserve-window-point’
>> have a buffer local value which would override the customizable value
>> and add a new value, say 'always', which would allow the user to
>> override a local 'nil' while the default 't' would have the local 'nil'
>> prevail.  Maybe we'd then need a 'never' too ...
>
>That's a possibility, but it would be too radical to go into 26.1, so
>I think we should explore the less drastic alternatives first.

I agree with Eli that this is too big of a change at this point, but I
think this is the best long term solution. Perhaps we could
`make-variable-buffer-local' on `switch-to-buffer-preserve-window-point'
when enabling keep-place (later, of course).

> Since ‘keep-place’ is some sort of a minor mode, enabling it should warn
> the user about the ‘switch-to-buffer-preserve-window-point’
> incompatibility.  But I'm not familiar with ‘define-erc-module’ so we'd
> probably need someone knowledgeable to do that.

I think this is probably the best idea before the emacs 26 release. The
definition of 'keep-place' is currently

> (define-erc-module keep-place nil
>   "Leave point above un-viewed text in other channels."
>   ((add-hook 'erc-insert-pre-hook  'erc-keep-place))
>   ((remove-hook 'erc-insert-pre-hook  'erc-keep-place)))

I think we should be able to simply do something like

> (define-erc-module keep-place nil
>   "Leave point above un-viewed text in other channels."
>   ((add-hook 'erc-insert-pre-hook  'erc-keep-place)
>    ;; poor name, but just an example
>    (erc-check-if-switch-to-buffer-preserve-and-warn))
>   ((remove-hook 'erc-insert-pre-hook  'erc-keep-place)))

I would be happy to submit a patch for this, if the Erc maintainer is
busy. Does this seem like a good solution, or does anyone see a better
way around this?

Thanks for the responses so far!

-Jay