bug#7138: 24.0.50; Markers lost with minibuffer completion

bug#7138: 24.0.50; Markers lost with minibuffer completion

[Aaron S. Hawley]

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

In GNU Emacs 24.0.50.1 (i386-mingw-nt6.1.7600)
 of 2010-09-20 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 6.1.7600
configured using `configure --with-gcc (4.4) --no-opt --cflags
-Ic:/imagesupport/include'

I've found that setting markers with C-SPC in the minibuffer for C-x
C-f and then completing some file name with TAB, the markers will be
deleted and so C-u C-SPC won't work.  This is a corner case of
completion and markers, but seems like it should be done right.  I
have proposed a patch.  The comment in the function I changed suggests
that marks should be preserved.  That doesn't seem to be the case at
all.

2010-09-30  Aaron S. Hawley  <aaron.s.hawley@gmail.com>

	* minibuffer.el (completion--replace): Try to preserve markers by
	only replacing the changed text.

--- minibuffer.el	2010-09-21 14:10:35.139030100 -0400
+++ minibuffer.el	2010-09-30 08:48:11.327387800 -0400
@@ -490,6 +490,13 @@
   ;; to keep markers "at the right place" and be robust in the face of
   ;; after-change-functions that may themselves modify the buffer.
   (goto-char beg)
-  (insert newtext)
-  (delete-region (point) (+ (point) (- end beg))))
+  (let ((pos (compare-strings (buffer-substring-no-properties beg end)
+                              0 (- end beg)
+                              newtext 0 (length newtext))))
+    (if (numberp pos)
+        (setq pos (1- (abs pos)))
+      (setq pos 0))
+    (forward-char pos)
+    (delete-region (point) end)
+    (insert (substring newtext pos))))


-- 
In general, we reserve the right to have a poor
memory--the computer, however, is supposed to
remember!  Poor computer.  -- Guy Lewis Steele Jr.

[-- Attachment #2: minibuffer-complete-replace-marks.diff --]
[-- Type: application/octet-stream, Size: 875 bytes --]

2010-09-30  Aaron S. Hawley  <aaron.s.hawley@gmail.com>

	* minibuffer.el (completion--replace): Try to preserve markers by
	only replacing the changed text.

--- minibuffer.el	2010-09-21 14:10:35.139030100 -0400
+++ minibuffer.el	2010-09-30 08:48:11.327387800 -0400
@@ -490,6 +490,13 @@
   ;; to keep markers "at the right place" and be robust in the face of
   ;; after-change-functions that may themselves modify the buffer.
   (goto-char beg)
-  (insert newtext)
-  (delete-region (point) (+ (point) (- end beg))))
+  (let ((pos (compare-strings (buffer-substring-no-properties beg end)
+                              0 (- end beg)
+                              newtext 0 (length newtext))))
+    (if (numberp pos)
+        (setq pos (1- (abs pos)))
+      (setq pos 0))
+    (forward-char pos)
+    (delete-region (point) end)
+    (insert (substring newtext pos))))
 

bug#7138: 24.0.50; Markers lost with minibuffer completion

[Stefan Monnier]

> I've found that setting markers with C-SPC in the minibuffer for C-x
> C-f and then completing some file name with TAB, the markers will be
> deleted and so C-u C-SPC won't work.  This is a corner case of
> completion and markers, but seems like it should be done right.  I
> have proposed a patch.

Looks pretty good, thank you.

> The comment in the function I changed suggests that marks should be
> preserved.  That doesn't seem to be the case at all.

It is talking about surrounding markers, not markers within the
replaced text.  Your patch changes the code to "delete first and insert
afterwards", which is not as good w.r.t preserving surrounding markers
(because after the delete, the markers that were after and before the
deleted text end up together and the subsequent insert can't tell which
to push to which side).

> +    (if (numberp pos)
> +        (setq pos (1- (abs pos)))
> +      (setq pos 0))

Aka  (setq pos (if (numberp pos) (1- (abs pos)) 0))

In any case, I've installed in the emacs-23 branch a slightly different
change, which additionally tries to find a common suffix.
Thanks,


        Stefan