unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: master f450798: Don't move point in vc-dir on vc-register/vc-checkin (bug#43188)
       [not found] ` <20200904211738.DA94720667@vcs0.savannah.gnu.org>
@ 2020-09-04 21:44   ` Stefan Monnier
  2020-09-05 19:34     ` Andrii Kolomoiets
  0 siblings, 1 reply; 14+ messages in thread
From: Stefan Monnier @ 2020-09-04 21:44 UTC (permalink / raw)
  To: emacs-devel; +Cc: Andrii Kolomoiets

> -		      (ewoc-invalidate vc-ewoc node))
> +                      ;; `ewoc-invalidate' will kill line and insert new text,
> +                      ;; let's keep point column.
> +                      (let ((p (point)))
> +		        (ewoc-invalidate vc-ewoc node)
> +                        (goto-char p)))

Shouldn't this be done in `ewoc-invalidate` instead?


        Stefan




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

* Re: master f450798: Don't move point in vc-dir on vc-register/vc-checkin (bug#43188)
  2020-09-04 21:44   ` master f450798: Don't move point in vc-dir on vc-register/vc-checkin (bug#43188) Stefan Monnier
@ 2020-09-05 19:34     ` Andrii Kolomoiets
  2020-09-05 19:41       ` Stefan Monnier
  0 siblings, 1 reply; 14+ messages in thread
From: Andrii Kolomoiets @ 2020-09-05 19:34 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> -		      (ewoc-invalidate vc-ewoc node))
>> +                      ;; `ewoc-invalidate' will kill line and insert new text,
>> +                      ;; let's keep point column.
>> +                      (let ((p (point)))
>> +		        (ewoc-invalidate vc-ewoc node)
>> +                        (goto-char p)))
>
> Shouldn't this be done in `ewoc-invalidate` instead?

Good point!  According 'ewoc-invalidate' docstring, refreshing is done
by deleting the text and inserting the new one thus the 'save-excursion'
is not really helpful.

Please see attached patch.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Save-and-restore-point-in-ewoc-invalidate.patch --]
[-- Type: text/x-patch, Size: 1990 bytes --]

From 6f07dca7bff37c6ca7e98f38e8613445d3e8ea32 Mon Sep 17 00:00:00 2001
From: Andrii Kolomoiets <andreyk.mad@gmail.com>
Date: Sat, 5 Sep 2020 22:18:59 +0300
Subject: [PATCH] Save and restore point in ewoc-invalidate

* lisp/emacs-lisp/ewoc.el (ewoc-invalidate): Use 'goto-char' instead of
'save-excursion'.
* lisp/vc/vc-dir.el (vc-dir-update): Don't save/restore point on calling
'ewoc-invalidate'.
---
 lisp/emacs-lisp/ewoc.el | 5 +++--
 lisp/vc/vc-dir.el       | 6 +-----
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/lisp/emacs-lisp/ewoc.el b/lisp/emacs-lisp/ewoc.el
index 78ada3e076..7244fd9359 100644
--- a/lisp/emacs-lisp/ewoc.el
+++ b/lisp/emacs-lisp/ewoc.el
@@ -461,9 +461,10 @@ ewoc-invalidate
 Delete current text first, thus effecting a \"refresh\"."
   (ewoc--set-buffer-bind-dll-let* ewoc
       ((pp (ewoc--pretty-printer ewoc)))
-    (save-excursion
+    (let ((p (point)))
       (dolist (node nodes)
-        (ewoc--refresh-node pp node dll)))))
+        (ewoc--refresh-node pp node dll))
+      (goto-char p))))
 
 (defun ewoc-goto-prev (ewoc arg)
   "Move point to the ARGth previous element in EWOC.
diff --git a/lisp/vc/vc-dir.el b/lisp/vc/vc-dir.el
index 6c219005ce..cdf8ab984e 100644
--- a/lisp/vc/vc-dir.el
+++ b/lisp/vc/vc-dir.el
@@ -451,11 +451,7 @@ vc-dir-update
 		      (setf (vc-dir-fileinfo->state (ewoc-data node)) (nth 1 entry))
 		      (setf (vc-dir-fileinfo->extra (ewoc-data node)) (nth 2 entry))
 		      (setf (vc-dir-fileinfo->needs-update (ewoc-data node)) nil)
-                      ;; `ewoc-invalidate' will kill line and insert new text,
-                      ;; let's keep point column.
-                      (let ((p (point)))
-		        (ewoc-invalidate vc-ewoc node)
-                        (goto-char p)))
+		      (ewoc-invalidate vc-ewoc node))
 		  ;; If the state is nil, the file does not exist
 		  ;; anymore, so remember the entry so we can remove
 		  ;; it after we are done inserting all ENTRIES.
-- 
2.15.1


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

* Re: master f450798: Don't move point in vc-dir on vc-register/vc-checkin (bug#43188)
  2020-09-05 19:34     ` Andrii Kolomoiets
@ 2020-09-05 19:41       ` Stefan Monnier
  2020-09-13 18:58         ` Andrii Kolomoiets
  0 siblings, 1 reply; 14+ messages in thread
From: Stefan Monnier @ 2020-09-05 19:41 UTC (permalink / raw)
  To: Andrii Kolomoiets; +Cc: emacs-devel

> -    (save-excursion
> +    (let ((p (point)))
>        (dolist (node nodes)
> -        (ewoc--refresh-node pp node dll)))))
> +        (ewoc--refresh-node pp node dll))
> +      (goto-char p))))

Hmm... I think this is an improvement when point is within the refreshed
node, but a regression when it's further down in the buffer.


        Stefan




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

* Re: master f450798: Don't move point in vc-dir on vc-register/vc-checkin (bug#43188)
  2020-09-05 19:41       ` Stefan Monnier
@ 2020-09-13 18:58         ` Andrii Kolomoiets
  2020-09-13 20:27           ` Stefan Monnier
  0 siblings, 1 reply; 14+ messages in thread
From: Andrii Kolomoiets @ 2020-09-13 18:58 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> -    (save-excursion
>> +    (let ((p (point)))
>>        (dolist (node nodes)
>> -        (ewoc--refresh-node pp node dll)))))
>> +        (ewoc--refresh-node pp node dll))
>> +      (goto-char p))))
>
> Hmm... I think this is an improvement when point is within the refreshed
> node, but a regression when it's further down in the buffer.

You are right.

Take a look at the new patch.  ewoc-invalidate now save and restore
point line and column offset in the current node.

testewoc.el is the code I used to test the patch:

1. emacs -Q
2. M-x load-file testewoc.el
3. M-x testewoc
4. move point somewhere and M-x testewoc-invalidate
Point must remain in the same line and column within the node.  The
nodes is represented with characters "0", "1" and "2".


[-- Attachment #2: testewoc.el --]
[-- Type: application/emacs-lisp, Size: 1006 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Save-and-restore-point-in-ewoc-invalidate.patch --]
[-- Type: text/x-patch, Size: 3321 bytes --]

From 0b43d95581c36d43995fc27fdb965327937455ae Mon Sep 17 00:00:00 2001
From: Andrii Kolomoiets <andreyk.mad@gmail.com>
Date: Sat, 5 Sep 2020 22:18:59 +0300
Subject: [PATCH] Save and restore point in ewoc-invalidate

* lisp/emacs-lisp/ewoc.el (ewoc-invalidate): Save and restore point line and
column offset.
* lisp/vc/vc-dir.el (vc-dir-update): Don't save/restore point on calling
'ewoc-invalidate'.
---
 lisp/emacs-lisp/ewoc.el | 37 ++++++++++++++++++++++++++++++++++---
 lisp/vc/vc-dir.el       |  6 +-----
 2 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/lisp/emacs-lisp/ewoc.el b/lisp/emacs-lisp/ewoc.el
index 78ada3e076..6a37309eb1 100644
--- a/lisp/emacs-lisp/ewoc.el
+++ b/lisp/emacs-lisp/ewoc.el
@@ -461,9 +461,40 @@ ewoc-invalidate
 Delete current text first, thus effecting a \"refresh\"."
   (ewoc--set-buffer-bind-dll-let* ewoc
       ((pp (ewoc--pretty-printer ewoc)))
-    (save-excursion
-      (dolist (node nodes)
-        (ewoc--refresh-node pp node dll)))))
+    (let ((point (point))
+	  (footer-start (ewoc-location (ewoc--footer ewoc)))
+	  (refresh-nodes (lambda ()
+			   (dolist (node nodes)
+			     (ewoc--refresh-node pp node dll)))))
+      (cond
+       ((= point footer-start)
+	(funcall refresh-nodes))
+       ((or (> point footer-start)
+            (<= point (ewoc-location (ewoc-nth ewoc 0))))
+	(save-excursion
+	  (funcall refresh-nodes)))
+       (t
+	(let* ((node-start (ewoc-location (ewoc-locate ewoc)))
+	       (line (- (line-number-at-pos point)
+			(line-number-at-pos node-start)))
+	       (column (min (- point node-start)
+                            (- point (line-beginning-position)))))
+	  (funcall refresh-nodes)
+	  (goto-char node-start)
+	  (let* ((node-end (1- (ewoc-location
+                                (ewoc--node-right (ewoc-locate ewoc)))))
+		 (line-start (line-number-at-pos node-start))
+		 (line-end (line-number-at-pos node-end))
+		 (line (min line (- line-end line-start)))
+                 (line-beginning-position (line-beginning-position))
+                 (line-end-position (line-end-position)))
+            (when (> line 0)
+	      (forward-line line))
+	    (forward-char (min column
+			       (- node-end node-start)
+			       (- node-end line-beginning-position)
+			       (- line-end-position line-beginning-position)
+			       (- line-end-position node-start))))))))))
 
 (defun ewoc-goto-prev (ewoc arg)
   "Move point to the ARGth previous element in EWOC.
diff --git a/lisp/vc/vc-dir.el b/lisp/vc/vc-dir.el
index 6c219005ce..cdf8ab984e 100644
--- a/lisp/vc/vc-dir.el
+++ b/lisp/vc/vc-dir.el
@@ -451,11 +451,7 @@ vc-dir-update
 		      (setf (vc-dir-fileinfo->state (ewoc-data node)) (nth 1 entry))
 		      (setf (vc-dir-fileinfo->extra (ewoc-data node)) (nth 2 entry))
 		      (setf (vc-dir-fileinfo->needs-update (ewoc-data node)) nil)
-                      ;; `ewoc-invalidate' will kill line and insert new text,
-                      ;; let's keep point column.
-                      (let ((p (point)))
-		        (ewoc-invalidate vc-ewoc node)
-                        (goto-char p)))
+		      (ewoc-invalidate vc-ewoc node))
 		  ;; If the state is nil, the file does not exist
 		  ;; anymore, so remember the entry so we can remove
 		  ;; it after we are done inserting all ENTRIES.
-- 
2.15.1


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

* Re: master f450798: Don't move point in vc-dir on vc-register/vc-checkin (bug#43188)
  2020-09-13 18:58         ` Andrii Kolomoiets
@ 2020-09-13 20:27           ` Stefan Monnier
  2020-09-15 19:14             ` Andrii Kolomoiets
  0 siblings, 1 reply; 14+ messages in thread
From: Stefan Monnier @ 2020-09-13 20:27 UTC (permalink / raw)
  To: Andrii Kolomoiets; +Cc: emacs-devel

> Take a look at the new patch.  ewoc-invalidate now save and restore
> point line and column offset in the current node.

Wow!  That seems more complex than we'd like it to be.
Hmm...

Maybe doing it inside ewoc--refresh-node makes it easier, as in the
100% untested patch below.


        Stefan


diff --git a/lisp/emacs-lisp/ewoc.el b/lisp/emacs-lisp/ewoc.el
index fcb3d5a29e..f5ff41491d 100644
--- a/lisp/emacs-lisp/ewoc.el
+++ b/lisp/emacs-lisp/ewoc.el
@@ -205,15 +205,22 @@ ewoc--insert-new-node
 
 (defun ewoc--refresh-node (pp node dll)
   "Redisplay the element represented by NODE using the pretty-printer PP."
-  (let ((inhibit-read-only t)
-        (m (ewoc--node-start-marker node))
-        (R (ewoc--node-right node)))
-    ;; First, remove the string from the buffer:
-    (delete-region m (ewoc--node-start-marker R))
-    ;; Calculate and insert the string.
-    (goto-char m)
-    (funcall pp (ewoc--node-data node))
-    (ewoc--adjust m (point) R dll)))
+  (let* ((m (ewoc--node-start-marker node))
+         (R (ewoc--node-right node))
+         (end (ewoc--node-start-marker R))
+         (inhibit-read-only t)
+         (percent (when (and (<= m (point) end) (< m end))
+                    (/ (- (point) m) 1.0 (- end m)))))
+    (save-excursion
+      ;; First, remove the string from the buffer:
+      (delete-region m end)
+      ;; Calculate and insert the string.
+      (goto-char m)
+      (funcall pp (ewoc--node-data node))
+      (setq end (point))
+      (ewoc--adjust m (point) R dll))
+    (when percent
+      (goto-char (+ m (* percent (- end m)))))))
 
 (defun ewoc--wrap (separator func)
   (lambda (data)
@@ -352,11 +359,10 @@ ewoc-map
       ((footer (ewoc--footer ewoc))
        (pp (ewoc--pretty-printer ewoc))
        (node (ewoc--node-nth dll 1)))
-    (save-excursion
-      (while (not (eq node footer))
-        (if (apply map-function (ewoc--node-data node) args)
-            (ewoc--refresh-node pp node dll))
-        (setq node (ewoc--node-next dll node))))))
+    (while (not (eq node footer))
+      (if (apply map-function (ewoc--node-data node) args)
+          (ewoc--refresh-node pp node dll))
+      (setq node (ewoc--node-next dll node)))))
 
 (defun ewoc-delete (ewoc &rest nodes)
   "Delete NODES from EWOC."
@@ -471,9 +477,8 @@ ewoc-invalidate
 Delete current text first, thus effecting a \"refresh\"."
   (ewoc--set-buffer-bind-dll-let* ewoc
       ((pp (ewoc--pretty-printer ewoc)))
-    (save-excursion
-      (dolist (node nodes)
-        (ewoc--refresh-node pp node dll)))))
+    (dolist (node nodes)
+      (ewoc--refresh-node pp node dll))))
 
 (defun ewoc-goto-prev (ewoc arg)
   "Move point to the ARGth previous element in EWOC.
@@ -576,9 +581,8 @@ ewoc-set-hf
        (hf-pp (ewoc--hf-pp ewoc)))
     (setf (ewoc--node-data head) header
           (ewoc--node-data foot) footer)
-    (save-excursion
-      (ewoc--refresh-node hf-pp head dll)
-      (ewoc--refresh-node hf-pp foot dll))))
+    (ewoc--refresh-node hf-pp head dll)
+    (ewoc--refresh-node hf-pp foot dll)))
 
 \f
 (provide 'ewoc)




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

* Re: master f450798: Don't move point in vc-dir on vc-register/vc-checkin (bug#43188)
  2020-09-13 20:27           ` Stefan Monnier
@ 2020-09-15 19:14             ` Andrii Kolomoiets
  2020-09-15 20:32               ` Stefan Monnier
  0 siblings, 1 reply; 14+ messages in thread
From: Andrii Kolomoiets @ 2020-09-15 19:14 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Wow!  That seems more complex than we'd like it to be.
> Hmm...
>
> Maybe doing it inside ewoc--refresh-node makes it easier, as in the
> 100% untested patch below.

The complexity came from attempt to save the line and column offset
within node.  That code is not needed considering that there are not so
many multiline ewoc nodes.

The reason the save-point code was added to 'ewoc-invalidate' is to
avoid point movements on refreshing each node and make one movement once
all the nodes are refreshed.  As I can see 'ewoc-invalidate' is used to
refresh single node, so placing that code inside 'ewoc--refresh-node' is
more appropriate.

Your code works great except:

> +      (goto-char (+ m (* percent (- end m)))))))
                         ^ this must be wrapped with (round)

Also I would move point by offset rather than percent to save exactly
the same position.

Imagine the line in vc-dir (with point under |):

    edited   |foo.txt

After invalidation some text is added after filename, e.g.:

    edited   |foo.txt  (resolved conflict or something)

Isn't it better to leave point where it was and not move it to the right
because line length is increased?



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

* Re: master f450798: Don't move point in vc-dir on vc-register/vc-checkin (bug#43188)
  2020-09-15 19:14             ` Andrii Kolomoiets
@ 2020-09-15 20:32               ` Stefan Monnier
  2020-09-18 14:57                 ` Andrii Kolomoiets
  0 siblings, 1 reply; 14+ messages in thread
From: Stefan Monnier @ 2020-09-15 20:32 UTC (permalink / raw)
  To: Andrii Kolomoiets; +Cc: emacs-devel

> Your code works great except:
>
>> +      (goto-char (+ m (* percent (- end m)))))))
>                          ^ this must be wrapped with (round)
>
> Also I would move point by offset rather than percent to save exactly
> the same position.

Feel free to do that, yes (of course taking care of the case where the
new text is shorter than the original position).
I only used `percent` as an example.


        Stefan




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

* Re: master f450798: Don't move point in vc-dir on vc-register/vc-checkin (bug#43188)
  2020-09-15 20:32               ` Stefan Monnier
@ 2020-09-18 14:57                 ` Andrii Kolomoiets
  2020-09-18 15:23                   ` Stefan Monnier
  0 siblings, 1 reply; 14+ messages in thread
From: Andrii Kolomoiets @ 2020-09-18 14:57 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Also I would move point by offset rather than percent to save exactly
>> the same position.
>
> Feel free to do that, yes (of course taking care of the case where the
> new text is shorter than the original position).

Done.  Please see attached patch.

Thanks!

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Save-and-restore-point-in-ewoc-invalidate.patch --]
[-- Type: text/x-patch, Size: 4362 bytes --]

From b630aa0f63312585c65847e1cec4a61d57face98 Mon Sep 17 00:00:00 2001
From: Andrii Kolomoiets <andreyk.mad@gmail.com>
Date: Sat, 5 Sep 2020 22:18:59 +0300
Subject: [PATCH] Save and restore point in ewoc-invalidate

* lisp/emacs-lisp/ewoc.el (ewoc--refresh-node): Save and restore point line
and column offset.
(eowc-map) (ewoc--invalidate) (ewoc-set-hf): Don't use save-excursion
* lisp/vc/vc-dir.el (vc-dir-update): Don't save/restore point on calling
'ewoc-invalidate'.
---
 lisp/emacs-lisp/ewoc.el | 48 ++++++++++++++++++++++++++++--------------------
 lisp/vc/vc-dir.el       |  6 +-----
 2 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/lisp/emacs-lisp/ewoc.el b/lisp/emacs-lisp/ewoc.el
index 78ada3e076..5112322cfd 100644
--- a/lisp/emacs-lisp/ewoc.el
+++ b/lisp/emacs-lisp/ewoc.el
@@ -205,15 +205,26 @@ ewoc--insert-new-node
 
 (defun ewoc--refresh-node (pp node dll)
   "Redisplay the element represented by NODE using the pretty-printer PP."
-  (let ((inhibit-read-only t)
-        (m (ewoc--node-start-marker node))
-        (R (ewoc--node-right node)))
-    ;; First, remove the string from the buffer:
-    (delete-region m (ewoc--node-start-marker R))
-    ;; Calculate and insert the string.
-    (goto-char m)
-    (funcall pp (ewoc--node-data node))
-    (ewoc--adjust m (point) R dll)))
+  (let* ((m (ewoc--node-start-marker node))
+         (R (ewoc--node-right node))
+         (end (ewoc--node-start-marker R))
+         (inhibit-read-only t)
+         (offset (if (= (point) end)
+                     'end
+                   (when (< m (point) end)
+                     (- (point) m)))))
+    (save-excursion
+      ;; First, remove the string from the buffer:
+      (delete-region m end)
+      ;; Calculate and insert the string.
+      (goto-char m)
+      (funcall pp (ewoc--node-data node))
+      (setq end (point))
+      (ewoc--adjust m (point) R dll))
+    (when offset
+      (goto-char (if (eq offset 'end)
+                     end
+                   (min (+ m offset) (1- end)))))))
 
 (defun ewoc--wrap (func)
   (lambda (data)
@@ -342,11 +353,10 @@ ewoc-map
       ((footer (ewoc--footer ewoc))
        (pp (ewoc--pretty-printer ewoc))
        (node (ewoc--node-nth dll 1)))
-    (save-excursion
-      (while (not (eq node footer))
-        (if (apply map-function (ewoc--node-data node) args)
-            (ewoc--refresh-node pp node dll))
-        (setq node (ewoc--node-next dll node))))))
+    (while (not (eq node footer))
+      (if (apply map-function (ewoc--node-data node) args)
+          (ewoc--refresh-node pp node dll))
+      (setq node (ewoc--node-next dll node)))))
 
 (defun ewoc-delete (ewoc &rest nodes)
   "Delete NODES from EWOC."
@@ -461,9 +471,8 @@ ewoc-invalidate
 Delete current text first, thus effecting a \"refresh\"."
   (ewoc--set-buffer-bind-dll-let* ewoc
       ((pp (ewoc--pretty-printer ewoc)))
-    (save-excursion
-      (dolist (node nodes)
-        (ewoc--refresh-node pp node dll)))))
+    (dolist (node nodes)
+      (ewoc--refresh-node pp node dll))))
 
 (defun ewoc-goto-prev (ewoc arg)
   "Move point to the ARGth previous element in EWOC.
@@ -566,9 +575,8 @@ ewoc-set-hf
        (hf-pp (ewoc--hf-pp ewoc)))
     (setf (ewoc--node-data head) header
           (ewoc--node-data foot) footer)
-    (save-excursion
-      (ewoc--refresh-node hf-pp head dll)
-      (ewoc--refresh-node hf-pp foot dll))))
+    (ewoc--refresh-node hf-pp head dll)
+    (ewoc--refresh-node hf-pp foot dll)))
 
 \f
 (provide 'ewoc)
diff --git a/lisp/vc/vc-dir.el b/lisp/vc/vc-dir.el
index 6c219005ce..cdf8ab984e 100644
--- a/lisp/vc/vc-dir.el
+++ b/lisp/vc/vc-dir.el
@@ -451,11 +451,7 @@ vc-dir-update
 		      (setf (vc-dir-fileinfo->state (ewoc-data node)) (nth 1 entry))
 		      (setf (vc-dir-fileinfo->extra (ewoc-data node)) (nth 2 entry))
 		      (setf (vc-dir-fileinfo->needs-update (ewoc-data node)) nil)
-                      ;; `ewoc-invalidate' will kill line and insert new text,
-                      ;; let's keep point column.
-                      (let ((p (point)))
-		        (ewoc-invalidate vc-ewoc node)
-                        (goto-char p)))
+		      (ewoc-invalidate vc-ewoc node))
 		  ;; If the state is nil, the file does not exist
 		  ;; anymore, so remember the entry so we can remove
 		  ;; it after we are done inserting all ENTRIES.
-- 
2.15.1


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

* Re: master f450798: Don't move point in vc-dir on vc-register/vc-checkin (bug#43188)
  2020-09-18 14:57                 ` Andrii Kolomoiets
@ 2020-09-18 15:23                   ` Stefan Monnier
  2020-09-18 15:47                     ` Andrii Kolomoiets
  0 siblings, 1 reply; 14+ messages in thread
From: Stefan Monnier @ 2020-09-18 15:23 UTC (permalink / raw)
  To: Andrii Kolomoiets; +Cc: emacs-devel

> Done.  Please see attached patch.

Looks perfect, thanks.  Do you have write access or should I push this for you?


        Stefan




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

* Re: master f450798: Don't move point in vc-dir on vc-register/vc-checkin (bug#43188)
  2020-09-18 15:23                   ` Stefan Monnier
@ 2020-09-18 15:47                     ` Andrii Kolomoiets
  2020-09-18 15:53                       ` Stefan Monnier
  0 siblings, 1 reply; 14+ messages in thread
From: Andrii Kolomoiets @ 2020-09-18 15:47 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Do you have write access or should I push this for you?

I don't have write access, so please install it for me.

Thanks!



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

* Re: master f450798: Don't move point in vc-dir on vc-register/vc-checkin (bug#43188)
  2020-09-18 15:47                     ` Andrii Kolomoiets
@ 2020-09-18 15:53                       ` Stefan Monnier
  2020-09-24 20:21                         ` Andrii Kolomoiets
  0 siblings, 1 reply; 14+ messages in thread
From: Stefan Monnier @ 2020-09-18 15:53 UTC (permalink / raw)
  To: Andrii Kolomoiets; +Cc: emacs-devel

>> Do you have write access or should I push this for you?
> I don't have write access, so please install it for me.

Done,


        Stefan




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

* Re: master f450798: Don't move point in vc-dir on vc-register/vc-checkin (bug#43188)
  2020-09-18 15:53                       ` Stefan Monnier
@ 2020-09-24 20:21                         ` Andrii Kolomoiets
  2020-09-28 17:13                           ` Stefan Monnier
  0 siblings, 1 reply; 14+ messages in thread
From: Andrii Kolomoiets @ 2020-09-24 20:21 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Done,

There are still the case when point position is not saved on
ewoc-invalidate - when the buffer is displayed in the other window:

1. mkdir gittest && cd gittest && git init && touch foo
2. emacs -Q
3. C-x v d
4. n
5. o
6. o
7. C-x C-s
Point is moved to BOL. Because
8. M-: (save-current-buffer (set-buffer "*vc-dir*") (goto-char 1))
will not move point in the *vc-dir*.

Is it worth it to restore the point position in each window where it
will be affected by 'ewoc--refresh-node'?



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

* Re: master f450798: Don't move point in vc-dir on vc-register/vc-checkin (bug#43188)
  2020-09-24 20:21                         ` Andrii Kolomoiets
@ 2020-09-28 17:13                           ` Stefan Monnier
  2020-09-30 20:08                             ` Andrii Kolomoiets
  0 siblings, 1 reply; 14+ messages in thread
From: Stefan Monnier @ 2020-09-28 17:13 UTC (permalink / raw)
  To: Andrii Kolomoiets; +Cc: emacs-devel

> There are still the case when point position is not saved on
> ewoc-invalidate - when the buffer is displayed in the other window:

Plus when the buffer is "displayed in a window configuration saved in
a register", plus when invalidate is called from within
`save-excursion`, plus ....

> Is it worth it to restore the point position in each window where it
> will be affected by 'ewoc--refresh-node'?

Down this path lies madness, I think.

Arguably Emacs should provide some way to better handle those issues
(e.g. provide a hook that can be used to influence the way markers move
in response to text removal+insertion), but until it does, I recommend
you live with the occasional suboptimal behavior.


        Stefan




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

* Re: master f450798: Don't move point in vc-dir on vc-register/vc-checkin (bug#43188)
  2020-09-28 17:13                           ` Stefan Monnier
@ 2020-09-30 20:08                             ` Andrii Kolomoiets
  0 siblings, 0 replies; 14+ messages in thread
From: Andrii Kolomoiets @ 2020-09-30 20:08 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> There are still the case when point position is not saved on
>> ewoc-invalidate - when the buffer is displayed in the other window:
>
> Plus when the buffer is "displayed in a window configuration saved in
> a register", plus when invalidate is called from within
> `save-excursion`, plus ....
>
> Arguably Emacs should provide some way to better handle those issues
> (e.g. provide a hook that can be used to influence the way markers move
> in response to text removal+insertion), but until it does, I recommend
> you live with the occasional suboptimal behavior.

Ok, I will.  But take a look at the attached patch, please.  Few
additional lines and the point is saved in all windows!  Not sure about
those additional cases you mention though, but if the behavior will be
suboptimal, I'll be ready :)

Thanks!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ewoc-save-point-in-windows.patch --]
[-- Type: text/x-patch, Size: 1790 bytes --]

diff --git a/lisp/emacs-lisp/ewoc.el b/lisp/emacs-lisp/ewoc.el
index 5112322cfd..b71ae5864c 100644
--- a/lisp/emacs-lisp/ewoc.el
+++ b/lisp/emacs-lisp/ewoc.el
@@ -209,10 +209,19 @@ ewoc--refresh-node
          (R (ewoc--node-right node))
          (end (ewoc--node-start-marker R))
          (inhibit-read-only t)
-         (offset (if (= (point) end)
-                     'end
-                   (when (< m (point) end)
-                     (- (point) m)))))
+         (get-offset (lambda (point)
+                       (if (= point end)
+                           'end
+                         (when (< m point end)
+                           (- point m)))))
+         (offset (funcall get-offset (point)))
+         (windows (mapcar (lambda (w)
+                            (unless (eq w (selected-window))
+                              (let ((offset (funcall get-offset
+                                                     (window-point w))))
+                                (when offset
+                                  (cons w offset)))))
+                          (get-buffer-window-list nil nil t))))
     (save-excursion
       ;; First, remove the string from the buffer:
       (delete-region m end)
@@ -224,7 +233,14 @@ ewoc--refresh-node
     (when offset
       (goto-char (if (eq offset 'end)
                      end
-                   (min (+ m offset) (1- end)))))))
+                   (min (+ m offset) (1- end)))))
+    (mapc (lambda (w)
+            (when (window-live-p (car w))
+              (set-window-point (car w)
+                                (if (eq (cdr w) 'end)
+                                    end
+                                  (min (+ m (cdr w)) (1- end))))))
+          windows)))
 
 (defun ewoc--wrap (func)
   (lambda (data)

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

end of thread, other threads:[~2020-09-30 20:08 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200904211737.15548.63989@vcs0.savannah.gnu.org>
     [not found] ` <20200904211738.DA94720667@vcs0.savannah.gnu.org>
2020-09-04 21:44   ` master f450798: Don't move point in vc-dir on vc-register/vc-checkin (bug#43188) Stefan Monnier
2020-09-05 19:34     ` Andrii Kolomoiets
2020-09-05 19:41       ` Stefan Monnier
2020-09-13 18:58         ` Andrii Kolomoiets
2020-09-13 20:27           ` Stefan Monnier
2020-09-15 19:14             ` Andrii Kolomoiets
2020-09-15 20:32               ` Stefan Monnier
2020-09-18 14:57                 ` Andrii Kolomoiets
2020-09-18 15:23                   ` Stefan Monnier
2020-09-18 15:47                     ` Andrii Kolomoiets
2020-09-18 15:53                       ` Stefan Monnier
2020-09-24 20:21                         ` Andrii Kolomoiets
2020-09-28 17:13                           ` Stefan Monnier
2020-09-30 20:08                             ` Andrii Kolomoiets

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