all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#60381: [PATCH] Preserve Window Position with Proced
@ 2022-12-28 15:55 Laurence Warne
  2022-12-28 17:14 ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Laurence Warne @ 2022-12-28 15:55 UTC (permalink / raw)
  To: 60381


[-- Attachment #1.1: Type: text/plain, Size: 1311 bytes --]

Hi,

This patch fixes a minor issue with proced buffers.  Previously, the window
position would be set to the start of the buffer when a proced buffer was
updated and it was not displayed in the selected window.  To reproduce:

(require 'proced)
(setq-default proceed-auto-update-flag t)
(M-x proced)

Move down to the next process, then change to a different window, and wait
a bit.  You should see the point in the proced buffer move back to the
beginning of the buffer. A similar issue occurs when the proced buffer is
not displayed in any window.

This patch addresses this by setting the window point (if applicable)
whenever a proced buffer is updated, and the second issue by not updating a
proced buffer if it is not displayed in any window.

I tried to add a test for this, but for example this:

(ert-deftest proced-update-preserves-point-test ()
  (proced--within-buffer
   'medium
   'user
   (proced--move-to-column "PID")
   (let ((point (window-point))
         (window (split-window)))
     (select-window window)
     (bury-buffer)
     (with-current-buffer "*Proced*"
       (proced-update t t))
     (switch-to-buffer "*Proced*")
     (should (= point (window-point))))))

passes even without this patch (though if I step through it with
edebug-defun it fails as expected).

Thanks, Laurence

[-- Attachment #1.2: Type: text/html, Size: 1674 bytes --]

[-- Attachment #2: 0001-Preserve-the-window-position-with-proced.patch --]
[-- Type: text/x-patch, Size: 2221 bytes --]

From 90bf6675bdf50a0fd3392b7ac4acc56e1e6436c4 Mon Sep 17 00:00:00 2001
From: Laurence Warne <laurencewarne@gmail.com>
Date: Thu, 22 Dec 2022 17:16:08 +0000
Subject: [PATCH] Preserve the window position with proced

Preserve the window position for windows which display a proced buffer,
but are not the selected window when a proced buffer is updated.  Previously,
the window position would be set to the start of the buffer when a
proced buffer was updated and it was not displayed in the selected window.

* lisp/proced.el (proced-auto-update-timer): Only update a given
proced buffer if it is displayed in a window.
(proced-update): Set the window position if the proced buffer is
displayed in a window.
---
 lisp/proced.el | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lisp/proced.el b/lisp/proced.el
index c09ee18a8b..102171bef0 100644
--- a/lisp/proced.el
+++ b/lisp/proced.el
@@ -905,7 +905,8 @@ proced-auto-update-timer
   (unless (seq-filter (lambda (buf)
                         (with-current-buffer buf
                           (when (eq major-mode 'proced-mode)
-                            (if proced-auto-update-flag
+                            (if (and proced-auto-update-flag
+                                     (get-buffer-window buf t))
                                 (proced-update t t))
                             t)))
                       (buffer-list))
@@ -1986,7 +1987,13 @@ proced-update
     ;; done
     (or quiet (input-pending-p)
         (message (if revert "Updating process information...done."
-                   "Updating process display...done.")))))
+                   "Updating process display...done."))))
+  ;; This sets the window point for all windows displaying the Proced buffer
+  ;; which means all windows will have their point set to the most recently
+  ;; visited window displaying the buffer.  Possibly we could save all window
+  ;; points ahead of time, though this is more complicated.
+  (mapc (lambda (win) (set-window-point win (point)))
+        (get-buffer-window-list (current-buffer))))
 
 (defun proced-revert (&rest _args)
   "Reevaluate the process listing based on the currently running processes.
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#60381: [PATCH] Preserve Window Position with Proced
  2022-12-28 15:55 bug#60381: [PATCH] Preserve Window Position with Proced Laurence Warne
@ 2022-12-28 17:14 ` Eli Zaretskii
  2022-12-28 20:30   ` Laurence Warne
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2022-12-28 17:14 UTC (permalink / raw)
  To: Laurence Warne; +Cc: 60381

> From: Laurence Warne <laurencewarne@gmail.com>
> Date: Wed, 28 Dec 2022 15:55:20 +0000
> 
> This patch addresses this by setting the window point (if applicable) whenever a proced buffer is updated,
> and the second issue by not updating a proced buffer if it is not displayed in any window.

I don't think I like the solution for the second issue, since it is a
backward-incompatible change.  It is not at all obvious that not
updating a proced buffer which isn't shown in a window is TRT.

Why cannot we simply save and restore point across updates?  If the
old value of point ends up in a slightly different position (because
many processes have died, for example), that is not a disaster.

Alternatively, record the process on whose line point was before the
update, and try to find the same process after update.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#60381: [PATCH] Preserve Window Position with Proced
  2022-12-28 17:14 ` Eli Zaretskii
@ 2022-12-28 20:30   ` Laurence Warne
  2022-12-29  6:09     ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Laurence Warne @ 2022-12-28 20:30 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60381

[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]

> I don't think I like the solution for the second issue, since it is a
> backward-incompatible change.  It is not at all obvious that not
> updating a proced buffer which isn't shown in a window is TRT.

Fair enough, I'll revert this and try and think of another solution.

> Alternatively, record the process on whose line point was before the
> update, and try to find the same process after update.

I believe this is currently what proced-update tries to do, it works as
expected when an update occurs and the buffer is displayed in the selected
window (so if the process your point is on moves down a row, the point will
follow), but when the buffer is not displayed in the selected window, I
find the point is always reset to the beginning of the buffer regardless of
the process which was under point prior to the update.  In the end of
proced-update, the patch sets all the window points to the new buffer point
(recently set by proced-update to reflect the position of the new process),
similar to what append-to-buffer does, if that makes sense.

[-- Attachment #2: Type: text/html, Size: 1235 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#60381: [PATCH] Preserve Window Position with Proced
  2022-12-28 20:30   ` Laurence Warne
@ 2022-12-29  6:09     ` Eli Zaretskii
  2022-12-29 12:52       ` Laurence Warne
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2022-12-29  6:09 UTC (permalink / raw)
  To: Laurence Warne; +Cc: 60381

> From: Laurence Warne <laurencewarne@gmail.com>
> Date: Wed, 28 Dec 2022 20:30:41 +0000
> Cc: 60381@debbugs.gnu.org
> 
> > Alternatively, record the process on whose line point was before the
> > update, and try to find the same process after update.
> 
> I believe this is currently what proced-update tries to do, it works as expected when an update occurs and
> the buffer is displayed in the selected window (so if the process your point is on moves down a row, the
> point will follow), but when the buffer is not displayed in the selected window, I find the point is always reset
> to the beginning of the buffer regardless of the process which was under point prior to the update.

This sounds like some kind of bug, or perhaps we have some knob to
control this behavior.  So I think you should investigate why the
difference between selected and non-select windows, and why a buffer
that is not displayed behaves like that.  When you find the reason(s),
we could discuss possible solutions.  The solutions you describe (both
of them, actually) sound like workarounds to me: we let Emacs do
whatever nonsensical thing it does, and the correct the results.  It
is better to find a way of avoiding the wrong behavior in the first
place.

Btw, the variable switch-to-buffer-preserve-window-point may have
something to do with at least some of the behaviors you see.

Thanks.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#60381: [PATCH] Preserve Window Position with Proced
  2022-12-29  6:09     ` Eli Zaretskii
@ 2022-12-29 12:52       ` Laurence Warne
  2022-12-29 14:09         ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Laurence Warne @ 2022-12-29 12:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60381

[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]

I've logged the buffer and window point at certain lines (most important is
before/after erase-buffer on line 1911) in a few run throughs of
proced-update, first when the selected window displays a proced buffer:

before update: window point: 20121, buffer point: 20121
after goto-char min: window point: 1, buffer point: 1
before erase buffer: window point: 1, buffer point: 1
after erase buffer: window point: 1, buffer point: 1
after update: window point: 20121, buffer point: 20121

So this is expected since buffer point always mirrors window point for the
selected window's displayed buffer.  Next, when the selected window doesn't
display a proced buffer, but there does exist a window displaying a proced
buffer (the window point logged corresponds to the window point of the
window containing the proced buffer):

before update: window point: 20235, buffer point: 20235
after goto-char min: window point: 20235, buffer point: 1
before erase buffer: window point: 20235, buffer point: 1
after erase buffer: window point: 1, buffer point: 1
after update: window point: 1, buffer point: 20235

So my understanding is since the selected window does not display a proced
buffer, the window point is not updated in line with the buffer point, but
the erase-buffer call sets the window point to start of the buffer, and so
this is not updated in line with the buffer point in the subsequent
insertion of processes.

The last case (the second issue) where no window shows a proced buffer is
similar to the previous, but erase buffer instead appears to set pos for
the proced buffer's value in (window-prev-buffers) if it's the case a
window has shown a proced buffer previously.

[-- Attachment #2: Type: text/html, Size: 1882 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#60381: [PATCH] Preserve Window Position with Proced
  2022-12-29 12:52       ` Laurence Warne
@ 2022-12-29 14:09         ` Eli Zaretskii
       [not found]           ` <CAE2oLqh5i-fFVeYwyRufWhFZzrxDCfO+VrWFpe3tRLW9OJKUbg@mail.gmail.com>
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2022-12-29 14:09 UTC (permalink / raw)
  To: Laurence Warne; +Cc: 60381

> From: Laurence Warne <laurencewarne@gmail.com>
> Date: Thu, 29 Dec 2022 12:52:20 +0000
> Cc: 60381@debbugs.gnu.org
> 
> before update: window point: 20235, buffer point: 20235
> after goto-char min: window point: 20235, buffer point: 1
> before erase buffer: window point: 20235, buffer point: 1
> after erase buffer: window point: 1, buffer point: 1
> after update: window point: 1, buffer point: 20235
> 
> So my understanding is since the selected window does not display a proced buffer, the window point is not
> updated in line with the buffer point, but the erase-buffer call sets the window point to start of the buffer, and
> so this is not updated in line with the buffer point in the subsequent insertion of processes.

Why cannot proced update point when the window is not the selected
one?

> The last case (the second issue) where no window shows a proced buffer is similar to the previous, but
> erase buffer instead appears to set pos for the proced buffer's value in (window-prev-buffers) if it's the case
> a window has shown a proced buffer previously.

Same question here.

And I don't think I understand how the above explains why searching
for the same process also fails.  That should be independent of
whether the buffer is displayed in the selected window or in any
window.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#60381: [PATCH] Preserve Window Position with Proced
       [not found]           ` <CAE2oLqh5i-fFVeYwyRufWhFZzrxDCfO+VrWFpe3tRLW9OJKUbg@mail.gmail.com>
@ 2022-12-29 17:37             ` Eli Zaretskii
  2023-01-05 15:59               ` Laurence Warne
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2022-12-29 17:37 UTC (permalink / raw)
  To: Laurence Warne; +Cc: 60381

[Why private email?]

> From: Laurence Warne <laurencewarne@gmail.com>
> Date: Thu, 29 Dec 2022 17:07:42 +0000
> 
> > Why cannot proced update point when the window is not the selected
> > one?
> 
> Say window A is displaying a proced buffer and is not selected.  Unless my understanding from
> https://www.gnu.org/software/emacs/manual/html_node/elisp/Window-Point.html is wrong, whenever
> proced-update uses goto-char, it would not update the window point in window A, but erase-buffer would, by
> setting the window point of A to the beginning of the buffer.

You need to use set-window-point in that case, yes.  But if the buffer
is not displayed, there should be no problem, and at most you will
have to set the local value of switch-to-buffer-preserve-window-point
to nil in proced buffers.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#60381: [PATCH] Preserve Window Position with Proced
  2022-12-29 17:37             ` Eli Zaretskii
@ 2023-01-05 15:59               ` Laurence Warne
  2023-01-07  9:28                 ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Laurence Warne @ 2023-01-05 15:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60381


[-- Attachment #1.1: Type: text/plain, Size: 2111 bytes --]

Hi,

Minor update, whilst working on the first case, I've come across some
strange behaviour which causes the window position to be reset to
(point-min) in some circumstances.  To reproduce this with a more minimal
setup than proced-update (I've also asked here:
https://emacs.stackexchange.com/questions/75165/window-point-reset-after-update
):

(defun example ()
  (interactive)
  (let* ((buf (get-buffer-create "*Example*"))
         (w-points (mapcar (lambda (win)
                             `(,win . ,(window-point win)))
                           (get-buffer-window-list buf))))
    (with-current-buffer buf
      (let ((buf-point (point)))
        (erase-buffer)
        (insert "line1\nline2\nline3\nline4\nline5")
        (goto-char buf-point)
        (mapc (lambda (wp) (set-window-point (car wp) (cdr wp))) w-points)))
    (message "Ran update")))

(setq example-timer (run-at-time t 5 #'example))

Now with a configuration of two windows, switch to the example buffer in
one of them, and move down a few lines. Switch to the other window, you
should see the window point stay the same after every update.

If you then invoke M-x, wait for an update to occur, and then cancel the
invocation using C-g, then you should see the point in the window
displaying the example buffer go back to the start of the window. I've
created a video here:
https://user-images.githubusercontent.com/17688577/210167335-f7a4d50f-dbaf-4ffc-b1e0-38c5612ed2e3.mp4.
I'm a bit confused by this, any pointers would be greatly appreciated.
On the upside, I've managed to put together a test which won't pass without
the patch.

In terms of the difference between this patch and the original, the new
patch maintains the window point for all windows which display a proced
buffer.  The original just set the window point for one, as a consequence
all windows displaying the proced buffer in question would have their point
set to the most recently visited window displaying the buffer.   I've had
to extract out some logic from proced-update to separate functions, and so
this patch is a bit more invasive.

Thanks, Laurence

[-- Attachment #1.2: Type: text/html, Size: 3944 bytes --]

[-- Attachment #2: 0001-Preserve-the-window-position-with-proced.patch --]
[-- Type: text/x-patch, Size: 8264 bytes --]

From d2135211cf5adcc2a510f7d250d520e8fb58a3d9 Mon Sep 17 00:00:00 2001
From: Laurence Warne <laurencewarne@gmail.com>
Date: Thu, 22 Dec 2022 17:16:08 +0000
Subject: [PATCH] Preserve the window position with proced

Preserve the window position for windows which display a proced buffer,
but are not the selected window when a proced buffer is updated.  Previously,
the window position would be set to the start of the buffer when a
proced buffer was updated and it was not displayed in the selected window.

* lisp/proced.el (proced-auto-update-timer): Only update a given
proced buffer if it is displayed in a window.
(proced-update): Set the window position if the proced buffer is
displayed in a window.
* test/lisp/proced-tests.el
(proced-update-preserves-pid-at-point-test): New test.
---
 lisp/proced.el            | 103 ++++++++++++++++++++++++++------------
 test/lisp/proced-tests.el |  17 +++++++
 2 files changed, 87 insertions(+), 33 deletions(-)

diff --git a/lisp/proced.el b/lisp/proced.el
index c09ee18a8b..858540d540 100644
--- a/lisp/proced.el
+++ b/lisp/proced.el
@@ -792,6 +792,52 @@ proced-pid-at-point
     (if (looking-at "^. .")
         (get-text-property (match-end 0) 'proced-pid))))
 
+(defun proced--position-info (pos)
+  "Return information of the process at POS.
+
+The returned information will have the form `(PID KEY COLUMN)' where
+PID is the process ID of the process at point, KEY is the value of the
+proced-key text property at point, and COLUMN is the column for which the
+current value of the proced-key text property starts, or 0 if KEY is nil."
+  ;; If point is on a field, we try to return point to that field.
+  ;; Otherwise we try to return to the same column
+  (save-excursion
+    (goto-char pos)
+    (let ((pid (proced-pid-at-point))
+          (key (get-text-property (point) 'proced-key)))
+      (list pid key ; can both be nil
+            (if key
+                (if (get-text-property (1- (point)) 'proced-key)
+                    (- (point) (previous-single-property-change
+                                (point) 'proced-key))
+                  0)
+              (current-column))))))
+
+(defun proced--determine-pos (key column)
+  "Return the point in the current line using KEY and COLUMN.
+
+Attempt to find the first position on the current line where the
+text property proced-key is equal to KEY.  If this is not possible, return
+the point of column COLUMN on the current line."
+  (save-excursion
+    (let (new-pos)
+      (if key
+          (let ((limit (line-end-position)) pos)
+            (while (and (not new-pos)
+                        (setq pos (next-property-change (point) nil limit)))
+              (goto-char pos)
+              (when (eq key (get-text-property (point) 'proced-key))
+                (forward-char (min column (- (next-property-change (point))
+                                             (point))))
+                (setq new-pos (point))))
+            (unless new-pos
+              ;; we found the process, but the field of point
+              ;; is not listed anymore
+              (setq new-pos (proced-move-to-goal-column))))
+        (setq new-pos (min (+ (line-beginning-position) column)
+                           (line-end-position))))
+      new-pos)))
+
 ;; proced mode
 
 (define-derived-mode proced-mode special-mode "Proced"
@@ -1889,17 +1935,10 @@ proced-update
   (if (consp buffer-undo-list)
       (setq buffer-undo-list nil))
   (let ((buffer-undo-list t)
-        ;; If point is on a field, we try to return point to that field.
-        ;; Otherwise we try to return to the same column
-        (old-pos (let ((pid (proced-pid-at-point))
-                       (key (get-text-property (point) 'proced-key)))
-                   (list pid key ; can both be nil
-                         (if key
-                             (if (get-text-property (1- (point)) 'proced-key)
-                                 (- (point) (previous-single-property-change
-                                             (point) 'proced-key))
-                               0)
-                           (current-column)))))
+        (window-pos-infos
+         (mapcar (lambda (w) `(,w . ,(proced--position-info (window-point w))))
+                 (get-buffer-window-list (current-buffer) nil t)))
+        (old-pos (proced--position-info (point)))
         buffer-read-only mp-list)
     ;; remember marked processes (whatever the mark was)
     (goto-char (point-min))
@@ -1932,7 +1971,8 @@ proced-update
     ;; Sometimes this puts point in the middle of the proced buffer
     ;; where it is not interesting.  Is there a better / more flexible solution?
     (goto-char (point-min))
-    (let (pid mark new-pos)
+
+    (let (pid mark new-pos win-points)
       (if (or mp-list (car old-pos))
           (while (not (eobp))
             (setq pid (proced-pid-at-point))
@@ -1941,28 +1981,25 @@ proced-update
               (delete-char 1)
               (beginning-of-line))
             (when (eq (car old-pos) pid)
-              (if (nth 1 old-pos)
-                  (let ((limit (line-end-position)) pos)
-                    (while (and (not new-pos)
-                                (setq pos (next-property-change (point) nil limit)))
-                      (goto-char pos)
-                      (when (eq (nth 1 old-pos)
-                                (get-text-property (point) 'proced-key))
-                        (forward-char (min (nth 2 old-pos)
-                                           (- (next-property-change (point))
-                                              (point))))
-                        (setq new-pos (point))))
-                    (unless new-pos
-                      ;; we found the process, but the field of point
-                      ;; is not listed anymore
-                      (setq new-pos (proced-move-to-goal-column))))
-                (setq new-pos (min (+ (line-beginning-position) (nth 2 old-pos))
-                                   (line-end-position)))))
+              (setq new-pos (proced--determine-pos (nth 1 old-pos)
+                                                   (nth 2 old-pos))))
+            (mapc (lambda (w-pos)
+                    (when (eq (cadr w-pos) pid)
+                      (push `(,(car w-pos) . ,(proced--determine-pos
+                                               (nth 1 (cdr w-pos))
+                                               (nth 2 (cdr w-pos))))
+                            win-points)))
+                  window-pos-infos)
             (forward-line)))
-      (if new-pos
-          (goto-char new-pos)
-        (goto-char (point-min))
-        (proced-move-to-goal-column)))
+      (let ((fallback (save-excursion (goto-char (point-min))
+                                      (proced-move-to-goal-column)
+                                      (point))))
+        (goto-char (or new-pos fallback))
+        ;; Update window points
+        (mapc (lambda (w-pos)
+                (set-window-point (car w-pos)
+                                  (alist-get (car w-pos) win-points fallback)))
+              window-pos-infos)))
     ;; update mode line
     ;; Does the long `mode-name' clutter the mode line?  It would be nice
     ;; to have some other location for displaying the values of the various
diff --git a/test/lisp/proced-tests.el b/test/lisp/proced-tests.el
index 3c1f5493e7..1f47566529 100644
--- a/test/lisp/proced-tests.el
+++ b/test/lisp/proced-tests.el
@@ -101,5 +101,22 @@ proced-refine-with-update-test
        (should (string= pid (word-at-point)))
        (forward-line)))))
 
+(ert-deftest proced-update-preserves-pid-at-point-test ()
+  (proced--within-buffer
+   'medium
+   'user
+   (goto-char (point-min))
+   (search-forward (number-to-string (emacs-pid)))
+   (proced--move-to-column "PID")
+   (save-window-excursion
+     (let ((pid (proced-pid-at-point))
+           (new-window (split-window))
+           (old-window (get-buffer-window)))
+       (select-window new-window)
+       (with-current-buffer "*Proced*"
+         (proced-update t t))
+       (select-window old-window)
+       (should (= pid (proced-pid-at-point)))))))
+
 (provide 'proced-tests)
 ;;; proced-tests.el ends here
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#60381: [PATCH] Preserve Window Position with Proced
  2023-01-05 15:59               ` Laurence Warne
@ 2023-01-07  9:28                 ` Eli Zaretskii
  2023-01-07 11:58                   ` Laurence Warne
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2023-01-07  9:28 UTC (permalink / raw)
  To: Laurence Warne; +Cc: 60381

> From: Laurence Warne <laurencewarne@gmail.com>
> Date: Thu, 5 Jan 2023 15:59:57 +0000
> Cc: 60381@debbugs.gnu.org
> 
> Minor update, whilst working on the first case, I've come across some strange behaviour which causes the
> window position to be reset to (point-min) in some circumstances.  To reproduce this with a more minimal
> setup than proced-update (I've also asked here:
> https://emacs.stackexchange.com/questions/75165/window-point-reset-after-update):

It is not easy to understand the convoluted example, but did you try
to see whether switch-to-buffer-preserve-window-point has any effect
on the behavior you consider strange?





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#60381: [PATCH] Preserve Window Position with Proced
  2023-01-07  9:28                 ` Eli Zaretskii
@ 2023-01-07 11:58                   ` Laurence Warne
  2023-01-07 13:28                     ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Laurence Warne @ 2023-01-07 11:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60381

[-- Attachment #1: Type: text/plain, Size: 610 bytes --]

> It is not easy to understand the convoluted example, but did you try
> to see whether switch-to-buffer-preserve-window-point has any effect
> on the behavior you consider strange?

After some more debugging, I found setting read-minibuffer-restore-windows
to nil gave the desired behaviour.

From the doc for read-minibuffer-restore-windows : "Non-nil means restore
window configurations on exit from minibuffer."  I guess this includes
window points as well - so this is expected behaviour in that changes to
the window configuration as a result of a timer run whilst the minibuffer
is active are reverted?

[-- Attachment #2: Type: text/html, Size: 752 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#60381: [PATCH] Preserve Window Position with Proced
  2023-01-07 11:58                   ` Laurence Warne
@ 2023-01-07 13:28                     ` Eli Zaretskii
  2023-01-07 17:23                       ` Laurence Warne
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2023-01-07 13:28 UTC (permalink / raw)
  To: Laurence Warne; +Cc: 60381

> From: Laurence Warne <laurencewarne@gmail.com>
> Date: Sat, 7 Jan 2023 11:58:28 +0000
> Cc: 60381@debbugs.gnu.org
> 
> After some more debugging, I found setting read-minibuffer-restore-windows to nil gave the desired
> behaviour.
> 
> From the doc for read-minibuffer-restore-windows : "Non-nil means restore window configurations on exit
> from minibuffer."  I guess this includes window points as well - so this is expected behaviour in that changes
> to the window configuration as a result of a timer run whilst the minibuffer is active are reverted?

Yes, this is the expected behavior in that case.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#60381: [PATCH] Preserve Window Position with Proced
  2023-01-07 13:28                     ` Eli Zaretskii
@ 2023-01-07 17:23                       ` Laurence Warne
  2023-01-14  8:40                         ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Laurence Warne @ 2023-01-07 17:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60381


[-- Attachment #1.1: Type: text/plain, Size: 331 bytes --]

Cool, I think I've now fixed the second issue (preserving the position in
proced buffers which are not displayed in any window) using your suggestion
of setting switch-to-buffer-preserve-window-point locally to nil in proced
buffers.  I've attached a new patch, the same as the previous one but with
one line change for the above.

[-- Attachment #1.2: Type: text/html, Size: 373 bytes --]

[-- Attachment #2: 0001-Preserve-the-window-position-with-proced.patch --]
[-- Type: text/x-patch, Size: 8996 bytes --]

From b030dbce20668386b60212b90ace9f1100620de3 Mon Sep 17 00:00:00 2001
From: Laurence Warne <laurencewarne@gmail.com>
Date: Thu, 22 Dec 2022 17:16:08 +0000
Subject: [PATCH] Preserve the window position with proced

Preserve the window position for windows which display a proced buffer,
but are not the selected window when a proced buffer is updated.  Previously,
the window position would be set to the start of the buffer when a
proced buffer was updated and it was not displayed in the selected window.

Similarly, preserve the position in proced buffers which are not
displayed in any window by setting
switch-to-buffer-preserve-window-point to nil in proced buffers.

* lisp/proced.el (proced-auto-update-timer): Only update a given
proced buffer if it is displayed in a window.
(proced-update): Set the window position if the proced buffer is
displayed in a window.
(proced--position-info, proced--determine-pos): New Functions.
(proced-mode): Set switch-to-buffer-preserve-window-point to nil in
proced buffers.
* test/lisp/proced-tests.el
(proced-update-preserves-pid-at-point-test): New test.
---
 lisp/proced.el            | 104 ++++++++++++++++++++++++++------------
 test/lisp/proced-tests.el |  17 +++++++
 2 files changed, 88 insertions(+), 33 deletions(-)

diff --git a/lisp/proced.el b/lisp/proced.el
index c09ee18a8b..d8a11bd778 100644
--- a/lisp/proced.el
+++ b/lisp/proced.el
@@ -792,6 +792,52 @@ proced-pid-at-point
     (if (looking-at "^. .")
         (get-text-property (match-end 0) 'proced-pid))))
 
+(defun proced--position-info (pos)
+  "Return information of the process at POS.
+
+The returned information will have the form `(PID KEY COLUMN)' where
+PID is the process ID of the process at point, KEY is the value of the
+proced-key text property at point, and COLUMN is the column for which the
+current value of the proced-key text property starts, or 0 if KEY is nil."
+  ;; If point is on a field, we try to return point to that field.
+  ;; Otherwise we try to return to the same column
+  (save-excursion
+    (goto-char pos)
+    (let ((pid (proced-pid-at-point))
+          (key (get-text-property (point) 'proced-key)))
+      (list pid key ; can both be nil
+            (if key
+                (if (get-text-property (1- (point)) 'proced-key)
+                    (- (point) (previous-single-property-change
+                                (point) 'proced-key))
+                  0)
+              (current-column))))))
+
+(defun proced--determine-pos (key column)
+  "Return the point in the current line using KEY and COLUMN.
+
+Attempt to find the first position on the current line where the
+text property proced-key is equal to KEY.  If this is not possible, return
+the point of column COLUMN on the current line."
+  (save-excursion
+    (let (new-pos)
+      (if key
+          (let ((limit (line-end-position)) pos)
+            (while (and (not new-pos)
+                        (setq pos (next-property-change (point) nil limit)))
+              (goto-char pos)
+              (when (eq key (get-text-property (point) 'proced-key))
+                (forward-char (min column (- (next-property-change (point))
+                                             (point))))
+                (setq new-pos (point))))
+            (unless new-pos
+              ;; we found the process, but the field of point
+              ;; is not listed anymore
+              (setq new-pos (proced-move-to-goal-column))))
+        (setq new-pos (min (+ (line-beginning-position) column)
+                           (line-end-position))))
+      new-pos)))
+
 ;; proced mode
 
 (define-derived-mode proced-mode special-mode "Proced"
@@ -847,6 +893,7 @@ proced-mode
   (setq-local revert-buffer-function #'proced-revert)
   (setq-local font-lock-defaults
               '(proced-font-lock-keywords t nil nil beginning-of-line))
+  (setq-local switch-to-buffer-preserve-window-point nil)
   (if (and (not proced-auto-update-timer) proced-auto-update-interval)
       (setq proced-auto-update-timer
             (run-at-time t proced-auto-update-interval
@@ -1889,17 +1936,10 @@ proced-update
   (if (consp buffer-undo-list)
       (setq buffer-undo-list nil))
   (let ((buffer-undo-list t)
-        ;; If point is on a field, we try to return point to that field.
-        ;; Otherwise we try to return to the same column
-        (old-pos (let ((pid (proced-pid-at-point))
-                       (key (get-text-property (point) 'proced-key)))
-                   (list pid key ; can both be nil
-                         (if key
-                             (if (get-text-property (1- (point)) 'proced-key)
-                                 (- (point) (previous-single-property-change
-                                             (point) 'proced-key))
-                               0)
-                           (current-column)))))
+        (window-pos-infos
+         (mapcar (lambda (w) `(,w . ,(proced--position-info (window-point w))))
+                 (get-buffer-window-list (current-buffer) nil t)))
+        (old-pos (proced--position-info (point)))
         buffer-read-only mp-list)
     ;; remember marked processes (whatever the mark was)
     (goto-char (point-min))
@@ -1932,7 +1972,8 @@ proced-update
     ;; Sometimes this puts point in the middle of the proced buffer
     ;; where it is not interesting.  Is there a better / more flexible solution?
     (goto-char (point-min))
-    (let (pid mark new-pos)
+
+    (let (pid mark new-pos win-points)
       (if (or mp-list (car old-pos))
           (while (not (eobp))
             (setq pid (proced-pid-at-point))
@@ -1941,28 +1982,25 @@ proced-update
               (delete-char 1)
               (beginning-of-line))
             (when (eq (car old-pos) pid)
-              (if (nth 1 old-pos)
-                  (let ((limit (line-end-position)) pos)
-                    (while (and (not new-pos)
-                                (setq pos (next-property-change (point) nil limit)))
-                      (goto-char pos)
-                      (when (eq (nth 1 old-pos)
-                                (get-text-property (point) 'proced-key))
-                        (forward-char (min (nth 2 old-pos)
-                                           (- (next-property-change (point))
-                                              (point))))
-                        (setq new-pos (point))))
-                    (unless new-pos
-                      ;; we found the process, but the field of point
-                      ;; is not listed anymore
-                      (setq new-pos (proced-move-to-goal-column))))
-                (setq new-pos (min (+ (line-beginning-position) (nth 2 old-pos))
-                                   (line-end-position)))))
+              (setq new-pos (proced--determine-pos (nth 1 old-pos)
+                                                   (nth 2 old-pos))))
+            (mapc (lambda (w-pos)
+                    (when (eq (cadr w-pos) pid)
+                      (push `(,(car w-pos) . ,(proced--determine-pos
+                                               (nth 1 (cdr w-pos))
+                                               (nth 2 (cdr w-pos))))
+                            win-points)))
+                  window-pos-infos)
             (forward-line)))
-      (if new-pos
-          (goto-char new-pos)
-        (goto-char (point-min))
-        (proced-move-to-goal-column)))
+      (let ((fallback (save-excursion (goto-char (point-min))
+                                      (proced-move-to-goal-column)
+                                      (point))))
+        (goto-char (or new-pos fallback))
+        ;; Update window points
+        (mapc (lambda (w-pos)
+                (set-window-point (car w-pos)
+                                  (alist-get (car w-pos) win-points fallback)))
+              window-pos-infos)))
     ;; update mode line
     ;; Does the long `mode-name' clutter the mode line?  It would be nice
     ;; to have some other location for displaying the values of the various
diff --git a/test/lisp/proced-tests.el b/test/lisp/proced-tests.el
index 3c1f5493e7..1f47566529 100644
--- a/test/lisp/proced-tests.el
+++ b/test/lisp/proced-tests.el
@@ -101,5 +101,22 @@ proced-refine-with-update-test
        (should (string= pid (word-at-point)))
        (forward-line)))))
 
+(ert-deftest proced-update-preserves-pid-at-point-test ()
+  (proced--within-buffer
+   'medium
+   'user
+   (goto-char (point-min))
+   (search-forward (number-to-string (emacs-pid)))
+   (proced--move-to-column "PID")
+   (save-window-excursion
+     (let ((pid (proced-pid-at-point))
+           (new-window (split-window))
+           (old-window (get-buffer-window)))
+       (select-window new-window)
+       (with-current-buffer "*Proced*"
+         (proced-update t t))
+       (select-window old-window)
+       (should (= pid (proced-pid-at-point)))))))
+
 (provide 'proced-tests)
 ;;; proced-tests.el ends here
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#60381: [PATCH] Preserve Window Position with Proced
  2023-01-07 17:23                       ` Laurence Warne
@ 2023-01-14  8:40                         ` Eli Zaretskii
  0 siblings, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2023-01-14  8:40 UTC (permalink / raw)
  To: Laurence Warne; +Cc: 60381-done

> From: Laurence Warne <laurencewarne@gmail.com>
> Date: Sat, 7 Jan 2023 17:23:36 +0000
> Cc: 60381@debbugs.gnu.org
> 
> Cool, I think I've now fixed the second issue (preserving the position in proced buffers which are not
> displayed in any window) using your suggestion of setting switch-to-buffer-preserve-window-point locally to
> nil in proced buffers.  I've attached a new patch, the same as the previous one but with one line change for
> the above.

Thanks, installed, and closing the bug.





^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-01-14  8:40 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-28 15:55 bug#60381: [PATCH] Preserve Window Position with Proced Laurence Warne
2022-12-28 17:14 ` Eli Zaretskii
2022-12-28 20:30   ` Laurence Warne
2022-12-29  6:09     ` Eli Zaretskii
2022-12-29 12:52       ` Laurence Warne
2022-12-29 14:09         ` Eli Zaretskii
     [not found]           ` <CAE2oLqh5i-fFVeYwyRufWhFZzrxDCfO+VrWFpe3tRLW9OJKUbg@mail.gmail.com>
2022-12-29 17:37             ` Eli Zaretskii
2023-01-05 15:59               ` Laurence Warne
2023-01-07  9:28                 ` Eli Zaretskii
2023-01-07 11:58                   ` Laurence Warne
2023-01-07 13:28                     ` Eli Zaretskii
2023-01-07 17:23                       ` Laurence Warne
2023-01-14  8:40                         ` Eli Zaretskii

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.