bug#32390: 26.1; python.el: cleanup font lock buffer after input is sent

bug#32390: 26.1; python.el: cleanup font lock buffer after input is sent

[Carlos Pita]

font-locking gets often confused because of unclosed string delimiters.
For example, type " and then press return, start typing in the new input
prompt and the input will be identified as a docstring. It's ok to keep
the font lock buffer while doing multiline edition, but after input is
sent to the underlying process the buffer should be cleaned up so that
each input is independently colorized.

---

In GNU Emacs 26.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.30)
 of 2018-07-05 built on juergen
Windowing system distributor 'The X.Org Foundation', version 11.0.12000000
Recent messages:
Loading ido-completing-read+...done
Loading paren...done
Loading winner...done
Loading xclip...done
Source file ‘/home/carlos/.emacs.d/elpa/elpy-20180720.155/elpy-shell.el’ newer than byte-compiled file
[yas] Prepared just-in-time loading of snippets successfully.
For information about GNU Emacs and the GNU system, type C-h C-a.
ido-read-internal: Command attempted to use minibuffer while in minibuffer
You can run the command ‘run-python’ with M-x r-py RET
Shell native completion is disabled, using fallback

Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --with-x-toolkit=gtk3 --with-xft --with-modules
 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong
 -fno-plt' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS NOTIFY
ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS
GTK3 X11 MODULES THREADS LIBSYSTEMD LCMS2

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Inferior Python

Minor modes in effect:
  pdf-occur-global-minor-mode: t
  diff-auto-refine-mode: t
  pyvenv-mode: t
  shell-dirtrack-mode: t
  compilation-shell-minor-mode: t
  xclip-mode: t
  winner-mode: t
  show-paren-mode: t
  ido-ubiquitous-mode: t
  ido-everywhere: t
  global-company-mode: t
  company-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny rfc822 mml mml-sec epa
derived epg mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail display-line-numbers checkdoc pdf-occur
ibuf-ext ibuffer ibuffer-loaddefs tablist tablist-filter
semantic/wisent/comp semantic/wisent semantic/wisent/wisent
semantic/util-modes semantic/util semantic semantic/tag semantic/lex
semantic/fw mode-local cedet dired dired-loaddefs pdf-isearch let-alist
pdf-misc imenu pdf-tools pdf-view bookmark pp jka-compr pdf-cache
pdf-info tq pdf-util image-mode org-protocol org-element avl-tree
generator org org-macro org-footnote org-pcomplete org-list org-faces
org-entities noutline outline org-version ob-emacs-lisp ob ob-tangle
org-src ob-ref ob-lob ob-table ob-keys ob-exp ob-comint ob-core ob-eval
org-compat org-macs org-loaddefs find-func cal-menu calendar
cal-loaddefs cl-extra yasnippet elec-pair highlight-indentation
flymake-proc flymake warnings help-fns radix-tree help-mode elpy
find-file-in-project ivy delsel colir color ivy-overlay ffap thingatpt
windmove diff-mode easy-mmode elpy-shell pyvenv esh-var esh-io esh-cmd
esh-opt esh-ext esh-proc esh-arg esh-groups eshell esh-module esh-mode
esh-util elpy-profile elpy-django elpy-refactor subr-x python tramp-sh
tramp tramp-compat tramp-loaddefs trampver ucs-normalize shell pcomplete
parse-time format-spec advice json map grep compile comint ansi-color
files-x doom-themes-org doom-tomorrow-night-theme doom-themes
doom-themes-common company-oddmuse company-keywords company-etags etags
xref project company-gtags company-dabbrev-code company-dabbrev
company-files company-capf company-cmake company-xcode company-clang
company-semantic company-eclim company-template company-bbdb xclip
winner ring paren ido-completing-read+ memoize s cus-edit minibuf-eldef
ido gnus nnheader gnus-util rmail rmail-loaddefs rfc2047 rfc2045
ietf-drums mail-utils mm-util mail-prsvr wid-edit company edmacro kmacro
pcase cus-start cus-load finder-inf info package easymenu epg-config
url-handlers url-parse auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache url-vars seq byte-opt gv bytecomp
byte-compile cconv cl-loaddefs cl-lib time-date mule-util tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win
x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript charprop case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 556542 20830)
 (symbols 48 43787 5)
 (miscs 40 1025 265)
 (strings 32 128269 3450)
 (string-bytes 1 3715198)
 (vectors 16 66976)
 (vector-slots 8 1097024 18728)
 (floats 8 516 401)
 (intervals 56 385 0)
 (buffers 992 16))

bug#32390:

[Carlos Pita]

The buffer is indeed being cleaned up in the output filter:

(defun python-shell-font-lock-comint-output-filter-function (output)
  (if (and (not (string= "" output))
      ...
      (python-shell-font-lock-cleanup-buffer)

The problem is that an unclosed string is an error and in that case
output is "", so that the cleanup part is not reached. I think it
should be made unconditional.

bug#32390:

[Carlos Pita]

Sorry, it's not the first condition that is failing but the second one:

           (not (member
                 (python-shell-comint-end-of-output-p
                  (ansi-color-filter-apply output))
                 '(nil 0))))

(ansi-color-filter-apply output) is removing the error message so that
only the next input prompt remains. The entire expression evaluates to
nil for error output, at least when:

 '(python-shell-interpreter "ipython")
 '(python-shell-interpreter-args "-i --simple-prompt")

bug#32390:

[Carlos Pita]

Why not simply:

           (python-shell-comint-end-of-output-p
            (ansi-color-filter-apply output)))

?

What is the problem when just an input prompt is retrieved (that is,
the 0 case in (nil 0))?

bug#32390:

[Carlos Pita]

The current condition is also failing for multiline input. Consider:


In [154]: def f():
     ...:   'ewewe
     ...:

output is "     ...:" so it's not considered just an input prompt
(because of the preceding whitespace) and so the font lock buffer is
wrongly cleaned up (indeed, this is the only case I'm able to figure
out for which you don't want to cleanup the buffer).

I think the condition should be reformulated to match any expression
that ends with an input prompt excluding a continuation prompt. This
would fix both problems:

1. An expression that is just an input prompt (for example, ansi
filtered errors) will trigger a cleanup.
2. A continuation input prompt will not be considered the start of
a new input.

bug#32390: 26.1; python.el: cleanup font lock buffer after input is sent

[Noam Postavsky]

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

Carlos Pita <carlosjosepita@gmail.com> writes:

> font-locking gets often confused because of unclosed string delimiters.
> For example, type " and then press return, start typing in the new input
> prompt and the input will be identified as a docstring.

I can't reproduce this.  The next input looks normal to me.


[-- Attachment #2: screenshot of bug reproduction attempt --]
[-- Type: image/png, Size: 31093 bytes --]

bug#32390: 26.1; python.el: cleanup font lock buffer after input is sent

[Carlos Pita]

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

My report was for ipython. I've attached a screenshot showing the problem.

The way I'm fixing this issue at the moment is by patching
python-shell-font-lock-comint-output-filter-function like this:

(defun python-shell-font-lock-comint-output-filter-function (output)
  "Clean up the font-lock buffer after any OUTPUT."
  (if (and (python-shell-comint-end-of-output-p
            (ansi-color-filter-apply output))
           (not (string-match "\\.\\.\\.: $" output)))
      ;; If output is other than an input prompt then "real" output has
      ;; been received and the font-lock buffer must be cleaned up.
      (python-shell-font-lock-cleanup-buffer)
    ;; Otherwise just add a newline.
    (python-shell-font-lock-with-font-lock-buffer
      (goto-char (point-max))
      (newline)))
  output)

[-- Attachment #2: Screenshot_2018-08-31_08:54:00.png --]
[-- Type: image/png, Size: 14329 bytes --]

bug#32390: 26.1; python.el: cleanup font lock buffer after input is sent

[Noam Postavsky]

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

Carlos Pita <carlosjosepita@gmail.com> writes:

> My report was for ipython. I've attached a screenshot showing the problem.

I still can't reproduce.  What is your ipython version (mine is 5.1.0)?


[-- Attachment #2: screenshot of (failed) bug reproduction attempt --]
[-- Type: image/png, Size: 38120 bytes --]

bug#32390: 26.1; python.el: cleanup font lock buffer after input is sent

[Carlos Pita]

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

Please, look at the version information in the attached screenshot.

My shell config is:

 '(python-shell-interpreter "ipython")
 '(python-shell-interpreter-args "-i --simple-prompt")

[-- Attachment #2: Screenshot_2018-09-05_12:38:24.png --]
[-- Type: image/png, Size: 26922 bytes --]

bug#32390: 26.1; python.el: cleanup font lock buffer after input is sent

[Noam Postavsky]

retitle 32390 [ipython 6.x] unmatched quotes break fontification in run-python
tags 32390 = confirmed
quit

Carlos Pita <carlosjosepita@gmail.com> writes:

> Please, look at the version information in the attached screenshot.

Aha, ipython 6.x strikes again.

> My shell config is:
>
>  '(python-shell-interpreter "ipython")
>  '(python-shell-interpreter-args "-i --simple-prompt")

I can reproduce it when running with ipython 6.x, although it seems to
only happen the first time.  If enter 'x = "' again, then it works as
expected.

bug#32390: [ipython 6.x] unmatched quotes break fontification in run-python

[Noam Postavsky]

Carlos Pita <carlosjosepita@gmail.com> writes:

> Why not simply:
>
>            (python-shell-comint-end-of-output-p
>             (ansi-color-filter-apply output)))
>
> ?
>
> What is the problem when just an input prompt is retrieved (that is,
> the 0 case in (nil 0))?

I guess that's the "not just a prompt" in the comment:

           ;; Is end of output and is not just a prompt.

But it seems that python.el is assuming a certain chunking of output
from the subprocess, which is really not a safe assumptiong.  And
ipython 6 breaks it.

> The current condition is also failing for multiline input. Consider:
>
>
> In [154]: def f():
>      ...:   'ewewe
>      ...:
>
> output is "     ...:" so it's not considered just an input prompt
> (because of the preceding whitespace) and so the font lock buffer is
> wrongly cleaned up (indeed, this is the only case I'm able to figure
> out for which you don't want to cleanup the buffer).
>
> I think the condition should be reformulated to match any expression
> that ends with an input prompt excluding a continuation prompt. This
> would fix both problems:
>
> 1. An expression that is just an input prompt (for example, ansi
> filtered errors) will trigger a cleanup.
> 2. A continuation input prompt will not be considered the start of
> a new input.

This sounds quite sensible to me.

bug#32390: 26.1; python.el: cleanup font lock buffer after input is sent

[Carlos Pita]

> I can reproduce it when running with ipython 6.x, although it seems to
> only happen the first time.  If enter 'x = "' again, then it works as
> expected.


I think this is because the second time you're closing the previously
opened string. But try one more time and it should begin alternating
between string / non-string faces.

bug#32390: 26.1; python.el: cleanup font lock buffer after input is sent

[Carlos Pita]

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

Hi Noam,

I've been carefully studying this issue and below I explain in detail
what is really happening and what different cases are being (sometimes
wrongly) covered. There is a patch attached, I hope you consider it
worth of being applied.

The condition for cleaning up of the font lock buffer is:

-------------
  (if (and (not (string= "" output))                             (1)
           ;; Is end of output and is not just a prompt.
           (not (member
                 (python-shell-comint-end-of-output-p
                  (ansi-color-filter-apply output))
                 '(nil
         (2)
                   0))))
        (3)
-------------

First nota that (1) is really irrelevant since
(python-shell-comint-end-of-output-p "") returns nil anyway.

Now, the problem originating this report was:

-------------
In [15]: "
  File "<ipython-input-15-3b7a06bb1102>", line 1
    "
     ^
SyntaxError: EOL while scanning string literal


In [16]:   string face still here"
-------------

This happens because
python-shell-font-lock-comint-output-filter-function is called twice,
first for the error output and then for the "In [16]: " part (I assume
this is because one part is coming from stderr and the other from
stdout, but that's just a guess). The first time (2) applies since
we're *not* at the end of an input prompt. The second time (3) applies
since we're at the end of *just* and input prompt. So in any case the
buffer is cleaned up.

Now, my first reaction was to ignore the *just* part: what damage
could it do to just check if we're at the end of an input prompt
disregarding the fact that it could be the only thing there? Well, the
problem is with multiline input, of course. Nevertheless the current
code is relying in a very weak rule: it considers "just an input
prompt" to be a continuation prompt. Another unreliable aspect of the
current rule is that sometimes (python-shell-comint-end-of-output-p
(ansi-color-filter-apply output)) returns 1 and not 0 for continuation
prompts. In short, the rule does a very poor job identifying
continuations.

So, all in all I had rewritten (in a previous post) the condition above as:

-------------
  (if (and (python-shell-comint-end-of-output-p
            (ansi-color-filter-apply output))
           (not (string-match "\\.\\.\\.: $" output)))        (4)
-------------

Where:
- Clause (1) is disregarded because it's redundant.
- Clause (2) is taken into account.
- Clause (3) is disregarded because it's unreliable.
- Clause (4) was added to address the multiline input case
(continuation prompt).

Now, it's a sad fact that python-shell-prompt-input-regexps doesn't
distinguish between initial and continuation input prompts, so I
explicitly had to put that particular regexp there. I assume this is
the main reason while my fix was not yet accepted, isn't it?

At this point we have at least two alternatives:

a) Consider that an input prompt that includes the pattern "\\.\\.\\."
is a continuation prompt. This is a heuristic but I think it's robust
enough. I favor this solution and the attached patch implements it.

b) Add a new customization option with a list of continuation prompts.
I believe this would be too much.

So the attached patch implements this new rule:

-------------
    (if (let ((output (ansi-color-filter-apply output)))
        (and (python-shell-comint-end-of-output-p output)
             (not (string-match "\\.\\.\\." output))))       (5)
-----------

The difference between (4) and (5) is that (5) relaxes the match to
just include three sequential dots (because we already know we have an
input prompt at the end of the output!). I've been more careful by
matching on the filtered string instead of the raw one also.

Please let me know if you prefer option (b) instead.

Best regards
--
Carlos

[-- Attachment #2: font-lock-cleanup-filter.diff --]
[-- Type: text/x-patch, Size: 1181 bytes --]

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index c7bb2d9..218c6e4 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -2556,14 +2556,11 @@ python-shell-font-lock-cleanup-buffer
 
 (defun python-shell-font-lock-comint-output-filter-function (output)
   "Clean up the font-lock buffer after any OUTPUT."
-  (if (and (not (string= "" output))
-           ;; Is end of output and is not just a prompt.
-           (not (member
-                 (python-shell-comint-end-of-output-p
-                  (ansi-color-filter-apply output))
-                 '(nil 0))))
-      ;; If output is other than an input prompt then "real" output has
-      ;; been received and the font-lock buffer must be cleaned up.
+  (if (let ((output (ansi-color-filter-apply output)))
+        (and (python-shell-comint-end-of-output-p output)
+             (not (string-match "\\.\\.\\." output))))
+      ;; If output ends with an initial (not continuation) input prompt
+      ;; then the font-lock buffer must be cleaned up.
       (python-shell-font-lock-cleanup-buffer)
     ;; Otherwise just add a newline.
     (python-shell-font-lock-with-font-lock-buffer

bug#32390: 26.1; python.el: cleanup font lock buffer after input is sent

[Carlos Pita]

> stdout, but that's just a guess). The first time (2) applies since
> we're *not* at the end of an input prompt. The second time (3) applies
> since we're at the end of *just* and input prompt. So in any case the
> buffer is cleaned up.

Sorry, this must have said "in NO case the buffer is cleaned up".

bug#32390: 26.1; python.el: cleanup font lock buffer after input is sent

[Carlos Pita]

Please take a look at the related
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=33959 since I've added a
patch there that fixes both this and other issue. The patch is
essentially the one above plus a fix for a strange interaction with
company that makes the output filter populate the font lock buffer
with spurious empty lines. Since both changes overlap I combined them
into a single patch (I provided a separated one also, if you prefer).

bug#32390: (no subject)

[Carlos Pita]

retitle 32390 [ipython 6.x] unmatched quotes break fontification in python.el (superseded by #33959)
tags 32390 = confirmed
quit

Retitled in order to make it match "python" in the search interface and
to notify that #33959 contains and overlapping patch for this and other
issue.

bug#32390: 26.1; python.el: cleanup font lock buffer after input is sent

[Noam Postavsky]

tags 32390 fixed
close 32390 27.1
quit

Carlos Pita <carlosjosepita@gmail.com> writes:

> Please take a look at the related
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=33959 since I've added a
> patch there that fixes both this and other issue. The patch is
> essentially the one above plus a fix for a strange interaction with
> company that makes the output filter populate the font lock buffer
> with spurious empty lines. Since both changes overlap I combined them
> into a single patch (I provided a separated one also, if you prefer).

I think these changes are confusing enough on their own, without trying
to combine things.  The patch you posted here seems okay, so I've
applied to master now.

[1: 7acc621e37]: 2019-10-14 20:09:38 -0400
  Fix python-shell font-lock cleanup for unclosed quotes (Bug#32390)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=7acc621e373ba1371495e15e5e78aa6ce948a9a6

bug#32390: 26.1; python.el: cleanup font lock buffer after input is sent

[Carlos Pita]

Thanks Noam,

do you want me to provide a separate patch for the other issue that
applies on top of this one?

Remember that I had provided a separate but independent one in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=33959;att=1;msg=18;filename=0001-Avoid-spurious-empty-lines-in-font-lock-buffer-fixes.patch,
which will surely result in conflicts.

bug#32390: 26.1; python.el: cleanup font lock buffer after input is sent

[Carlos Pita]

Also, shouldn't this be applied to emacs-26 besides of master? I mean,
it's a bugfix.

bug#32390: 26.1; python.el: cleanup font lock buffer after input is sent

[Lars Ingebrigtsen]

Carlos Pita <carlosjosepita@gmail.com> writes:

> Also, shouldn't this be applied to emacs-26 besides of master? I mean,
> it's a bugfix.

There won't be any more Emacs 26 releases -- nothing is applied to that
branch at present.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#32390: 26.1; python.el: cleanup font lock buffer after input is sent

[Noam Postavsky]

Carlos Pita <carlosjosepita@gmail.com> writes:

> Also, shouldn't this be applied to emacs-26 besides of master? I mean,
> it's a bugfix.

emacs-26 is pretty much closed.

https://debbugs.gnu.org/37527#95