From: Jay Kamat <jaygkamat@gmail.com>
To: martin rudalics <rudalics@gmx.at>
Cc: 29111@debbugs.gnu.org
Subject: bug#29111: 26.0.90; Erc keep-place module broken with new default of switch-to-buffer-preserve-window-point
Date: Sat, 18 Nov 2017 16:57:01 -0500 [thread overview]
Message-ID: <87po8fqmhe.fsf@gmail.com> (raw)
In-Reply-To: <59FD7C27.7090803@gmx.at> (martin rudalics's message of "Sat, 04 Nov 2017 09:36:55 +0100")
[-- Attachment #1: Type: text/plain, Size: 524 bytes --]
Hi,
Sorry for the delay, I've been a bit busy for the last couple of
weeks. The code that Martin provided worked perfectly, and solves the
issue! I removed a few unneeded lines (`window-next-buffers' need not be
modified, as it is not used when setting point) and wrapped it so it
only runs when `switch-to-buffer-preserve-window-point' is set, and
created a patch, which is attached.
Let me know if you think another approach is better or if there are
other issues with the patch that I can help resolve.
Thanks,
-Jay
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-erc-keep-place-module-with-new-defaults-bug-2911.patch --]
[-- Type: text/x-diff, Size: 1386 bytes --]
From 1ccb3110be45a9492a53bc149ff7f824dc132479 Mon Sep 17 00:00:00 2001
From: Jay Kamat <jaygkamat@gmail.com>
Date: Sat, 18 Nov 2017 16:41:34 -0500
Subject: [PATCH] Fix erc keep-place module with new defaults (bug#29111)
Allows erc keep-place to continue working with
switch-to-buffer-preserve-window-point set to t, the new default.
---
lisp/erc/erc-goodies.el | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el
index a655d48a6a..8906da1e47 100644
--- a/lisp/erc/erc-goodies.el
+++ b/lisp/erc/erc-goodies.el
@@ -147,7 +147,19 @@ erc-keep-place
(>= (point) erc-insert-marker))
(deactivate-mark)
(goto-char (erc-beg-of-input-line))
- (forward-line -1)))
+ (forward-line -1)
+ ;; if `switch-to-buffer-preserve-window-point' is set,
+ ;; we cannot rely on point being saved, and must commit
+ ;; it to window-prev-buffers.
+ (when switch-to-buffer-preserve-window-point
+ (dolist (frame (frame-list))
+ (walk-window-tree
+ (lambda (window)
+ (let ((prev (assq (current-buffer)
+ (window-prev-buffers window))))
+ (when prev
+ (setf (nth 2 prev) (point-marker)))))
+ frame nil 'nominibuf)))))
;;; Distinguish non-commands
(defvar erc-noncommands-list '(erc-cmd-ME
--
2.11.0
[-- Attachment #3: Type: text/plain, Size: 4233 bytes --]
martin rudalics <rudalics@gmx.at> writes:
>> 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)))
>
> You could try to pretend that the position at which the buffer was
> previously shown in a window is the position calculated here. Add after
> the
>
> (forward-line -1)
>
> something like
>
> ;; For all non-minibuffer windows on all frames check whether the
> ;; current buffer was shown in that window before. If so, update the
> ;; window point positions stored for the buffer to the position we just
> ;; calculated.
> (dolist (frame (frame-list))
> (walk-window-tree
> (lambda (window)
> (let ((prev (assq (current-buffer) (window-prev-buffers window)))
> (next (assq (current-buffer) (window-next-buffers window))))
> (when prev
> (setf (nth 2 prev) (point-marker)))
> (when next
> (setf (nth 2 next) (point-marker)))))
> frame nil 'nominibuf))
>
> Completely _untested_ so you may have to tweak it appropriately!
>
>> 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)))
>
> Right. So with the hack above you would set (nth 2 entry) to the value
> keep-place calculated.
>
>>> 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).
>
> I can't yet assess all implications of such a solution so I'd certainly
> defer it until all other measures have been exhausted.
>
>>> 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?
>
> If all else fails let's do that. First please try the proposal above.
>
> martin
next prev parent reply other threads:[~2017-11-18 21:57 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-02 3:41 bug#29111: 26.0.90; Erc keep-place module broken with new default of switch-to-buffer-preserve-window-point Jay Kamat
2017-11-02 9:51 ` martin rudalics
2017-11-03 5:54 ` Jay Kamat
2017-11-03 7:58 ` martin rudalics
2017-11-03 8:59 ` Eli Zaretskii
2017-11-03 19:35 ` Jay Kamat
2017-11-04 8:36 ` martin rudalics
2017-11-18 21:57 ` Jay Kamat [this message]
2017-11-20 8:26 ` martin rudalics
2017-11-20 18:02 ` Eli Zaretskii
2017-11-21 9:23 ` martin rudalics
2017-11-21 15:44 ` Eli Zaretskii
2017-11-21 16:25 ` Jay Kamat
2017-11-22 8:21 ` martin rudalics
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87po8fqmhe.fsf@gmail.com \
--to=jaygkamat@gmail.com \
--cc=29111@debbugs.gnu.org \
--cc=rudalics@gmx.at \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).