bug#19650: 25.0.50; python-shell-font-lock-post-command-hook breaks hippie-expand

bug#19650: 25.0.50; python-shell-font-lock-post-command-hook breaks hippie-expand

[Carlos Pita]

hippie-expand sucessive completions depends on the marker
he-string-end having the same value the previous hippie-expand
invocation left it. But the delete-region call in
python-shell-font-lock-post-command-hook changes the value of that
marker after each hippie-expand invocation, so hippie-expand gets
stucked at the first candidate for ever.

bug#19650:

[Carlos Pita]

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

I've been doing my homework:

1) Changing insert for insert-before-markers is problematic because it
somehow wreaks havoc with font locking. I guess some other markers are
being moved around besides he-string-end, maybe the one pointing to
the end of the prompt or something like that (since everything gets
colored the same the prompt is). Moreover, the he-string-beg marker
would still point to the wrong place no matter what its type were.

2) This will work for the end marker: (set-marker-insertion-type
he-string-end t), but it's too invasive regarding hippie-expand. Also,
it assumes the user is indeed using hippie-expand. But maybe changing
the insertion type of he-string-end just around the
delete-region/insert and just in case hippie-expand is currently
provided will do the dirty job. Again, this doesn't fix the
he-string-beg marker at all.

3) Same as (2) above, but saving/restoring the *positions*  of both
he-string-beg and he-string-end. This is the best solution I could
figure out till now. Take a look at the attached patch, please.

What do you think?

[-- Attachment #2: py.diff --]
[-- Type: text/plain, Size: 2459 bytes --]

--- py1.el	2015-01-22 13:34:02.863974265 -0300
+++ py2.el	2015-01-22 13:34:18.143826873 -0300
@@ -16,21 +16,28 @@
             (buffer-undo-list t))
         ;; When input hasn't changed, do nothing.
         (when (not (string= input old-input))
-          (delete-region (cdr (python-util-comint-last-prompt)) (point-max))
-          (insert
-           (python-shell-font-lock-with-font-lock-buffer
-             (delete-region (line-beginning-position)
-                            (line-end-position))
-             (insert input)
-             ;; Ensure buffer is fontified, keeping it
-             ;; compatible with Emacs < 24.4.
-             (if (fboundp 'font-lock-ensure)
-                 (funcall 'font-lock-ensure)
-               (font-lock-default-fontify-buffer))
-             ;; Replace FACE text properties with FONT-LOCK-FACE so
-             ;; they are not overwritten by comint buffer's font lock.
-             (python-util-text-properties-replace-name
-              'face 'font-lock-face)
-             (buffer-substring (line-beginning-position)
-                               (line-end-position))))
-          (goto-char current-point))))))
+          (let ((beg) (end))
+            (when (featurep 'hippie-exp)
+              (setq beg (marker-position he-string-beg)
+                    end (marker-position he-string-end)))
+            (delete-region (cdr (python-util-comint-last-prompt)) (point-max))
+            (insert
+             (python-shell-font-lock-with-font-lock-buffer
+               (delete-region (line-beginning-position)
+                              (line-end-position))
+               (insert input)
+               ;; Ensure buffer is fontified, keeping it
+               ;; compatible with Emacs < 24.4.
+               (if (fboundp 'font-lock-ensure)
+                   (funcall 'font-lock-ensure)
+                 (font-lock-default-fontify-buffer))
+               ;; Replace FACE text properties with FONT-LOCK-FACE so
+               ;; they are not overwritten by comint buffer's font lock.
+               (python-util-text-properties-replace-name
+                'face 'font-lock-face)
+               (buffer-substring (line-beginning-position)
+                                 (line-end-position))))
+            (when (featurep 'hippie-exp)
+              (set-marker he-string-beg beg)
+              (set-marker he-string-end end))
+            (goto-char current-point)))))))

bug#19650:

[Carlos Pita]

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

Patch updated against current master (954ca0f017f62f52ee679f2aa68effb20c917278).

[-- Attachment #2: hippie.patch --]
[-- Type: text/x-patch, Size: 3050 bytes --]

From ff40570650c060b91c3939c80b5172ac4b01be82 Mon Sep 17 00:00:00 2001
From: memeplex <carlosjosepita@gmail.com>
Date: Thu, 22 Jan 2015 17:18:34 -0300
Subject: [PATCH] python.el: fix
 http://debbugs.gnu.org/cgi/bugreport.cgi?bug=19650

---
 .emacs.d/lisp/python.el | 43 +++++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/.emacs.d/lisp/python.el b/.emacs.d/lisp/python.el
index d340550..2963706 100644
--- a/.emacs.d/lisp/python.el
+++ b/.emacs.d/lisp/python.el
@@ -2364,24 +2364,31 @@ goes wrong and syntax highlighting in the shell gets messed up."
             (buffer-undo-list t))
         ;; When input hasn't changed, do nothing.
         (when (not (string= input old-input))
-          (delete-region (cdr (python-util-comint-last-prompt)) (point-max))
-          (insert
-           (python-shell-font-lock-with-font-lock-buffer
-             (delete-region (line-beginning-position)
-                            (line-end-position))
-             (insert input)
-             ;; Ensure buffer is fontified, keeping it
-             ;; compatible with Emacs < 24.4.
-             (if (fboundp 'font-lock-ensure)
-                 (funcall 'font-lock-ensure)
-               (font-lock-default-fontify-buffer))
-             ;; Replace FACE text properties with FONT-LOCK-FACE so
-             ;; they are not overwritten by comint buffer's font lock.
-             (python-util-text-properties-replace-name
-              'face 'font-lock-face)
-             (buffer-substring (line-beginning-position)
-                               (line-end-position))))
-          (goto-char current-point))))))
+          (let ((beg) (end))
+            (when (featurep 'hippie-exp)
+              (setq beg (marker-position he-string-beg)
+                    end (marker-position he-string-end)))
+            (delete-region (cdr (python-util-comint-last-prompt)) (point-max))
+            (insert
+             (python-shell-font-lock-with-font-lock-buffer
+               (delete-region (line-beginning-position)
+                              (line-end-position))
+               (insert input)
+               ;; Ensure buffer is fontified, keeping it
+               ;; compatible with Emacs < 24.4.
+               (if (fboundp 'font-lock-ensure)
+                   (funcall 'font-lock-ensure)
+                 (font-lock-default-fontify-buffer))
+               ;; Replace FACE text properties with FONT-LOCK-FACE so
+               ;; they are not overwritten by comint buffer's font lock.
+               (python-util-text-properties-replace-name
+                'face 'font-lock-face)
+               (buffer-substring (line-beginning-position)
+                                 (line-end-position))))
+            (when (featurep 'hippie-exp)
+              (set-marker he-string-beg beg)
+              (set-marker he-string-end end))
+            (goto-char current-point)))))))
 
 (defun python-shell-font-lock-turn-on (&optional msg)
   "Turn on shell font-lock.
-- 
2.2.1

bug#19650: 25.0.50; python-shell-font-lock-post-command-hook breaks hippie-expand

[Fabián Ezequiel Gallina]

Fixed at revno 52df70a in the master branch.

The approach used here is different than the patch provided as
`insert-before-markers` did the trick.  Thanks for such detailed
explanation of the problem.


Cheers,
Fabián.

bug#19650: closed (25.0.50; python-shell-font-lock-post-command-hook breaks hippie-expand)

[Carlos Pita]

> The approach used here is different than the patch provided as
> `insert-before-markers` did the trick.  Thanks for such detailed
> explanation of the problem.

Fabián, this approach shares a problem with my first
set-marker-insertion-type attempt in that it won't preserve
he-string-beg.

The problem manifests itself when you try something like:

> blah blah blah p<hippie-expand here>

It results in:

> p<hippie-expand completion here>

So the previous part of the line gets lost.

I don't think it's possible to fix that by just setting the marker or
insert operation types.

bug#19650: 25.0.50; python-shell-font-lock-post-command-hook breaks hippie-expand

[Fabián Ezequiel Gallina]

Alternate approach installed at revno 859e865 in the master branch.


Thanks,
Fabián.

bug#19650:

[Stefan Monnier]

> 1) Changing insert for insert-before-markers is problematic because it
> somehow wreaks havoc with font locking. I guess some other markers are
> being moved around besides he-string-end, maybe the one pointing to
> the end of the prompt or something like that (since everything gets
> colored the same the prompt is). Moreover, the he-string-beg marker
> would still point to the wrong place no matter what its type were.

Indeed, the issue is not "insert-before" vs "insert-after" but that we
delete+insert the exact same text, so the markers should simply not
move, but Emacs can't know that.

Basically, the delete+insert only changes the text's properties, so "the
right way" to fix this is to not delete and not insert, but instead to
take the result string, extract its text-properties and apply them
carefully to the corresponding of the buffer, so as not to affect
any marker.

Another "right way" is to do the font-locking "in situ" instead of doing
it in a side buffer.  This would be good, but IIUC it's not as
straightforward as it sounds.

> 3) Same as (2) above, but saving/restoring the *positions*  of both
> he-string-beg and he-string-end. This is the best solution I could
> figure out till now. Take a look at the attached patch, please.
> What do you think?

I think it's hideous, but maybe it's OK as a temporary workaround.
I'll let Fabian decide if he wants it in his code.  But if he accepts
it, the code should have some extra comments explaining what's going on.


        Stefan