bug#58389: 29.0.50; Python completion fails under certain conditions

bug#58389: 29.0.50; Python completion fails under certain conditions

[kobarity]

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

Hi,

Completion in Python mode buffer fails under certain conditions:

- non-native completion is used,
- ElDoc mode is used, or
- FFAP is used.

Here is how to reproduce the problem of non-native completion:

1. emacs -Q
2. Open the following Python file.

#+begin_src python
import sys
#+end_src

3. Use M-x customize-variable to disable
   python-shell-completion-native-enable
4. C-u -1 M-x eldoc-mode (Disable ElDoc mode)
5. C-c C-p (M-x run-python)
6. C-x o (Back to the Python mode buffer)
7. C-c C-c (M-x python-shell-send-buffer)
8. C-n (M-x next-line)
9. Type "sys."
10. C-M-i (M-x completion-at-point)
11. Completions are shown in *Completions* buffer.
12. Type "e"
13. C-M-i (M-x completion-at-point)
14. "No match" is shown in the minibuffer.

This does not happen when native completion is enabled and ElDoc mode
is disabled.

The following steps show the problem of ElDoc mode:

1. emacs -Q
2. Open the following Python file.

#+begin_src python
import sys
#+end_src

3. Make sure that ElDoc mode is enabled.
4. C-c C-p (M-x run-python)
5. Make sure that "Shell native completion is enabled." is shown in
   the minibuffer.
6. C-x o (Back to the Python mode buffer)
7. C-c C-c (M-x python-shell-send-buffer)
8. C-n (M-x next-line)
9. Type "sys."
10. Wait for 1-2 seconds to make sure ElDoc function is run by the
    idle timer.
11. C-M-i (M-x completion-at-point)
12. "No match" is shown in the minibuffer.

The problem with FFAP can be reproduced with similar steps.

The direct cause of these problems is that
`python-util-comint-last-prompt' returns nil, which is not supposed in
`python-shell-completion-at-point'.

`python-util-comint-last-prompt' is a wrapper to return
`comint-last-prompt'.  `comint-last-prompt' is initialized to nil in
`comint-snapshot-last-prompt' and is set in `comint-output-filter'.
However, `python-shell-send-string-no-output' which is used by the
features mentioned above temporarily changes the `process-filter' to
redirect the output.  This causes `comint-output-filter' to not be
called and `comint-last-prompt' to remain nil.

I think `comint-snapshot-last-prompt' should not be called as long as 
`comint-output-filter' is not called and the Inferior Python Shell
buffer is not changed.  Attached is a patch to fix this issue.

Best Regards,

--

In GNU Emacs 29.0.50 (build 5, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw scroll bars) of 2022-10-09 built on ubuntu
Repository revision: 9d47364634f96b2f2651a7f3942f8f87b920f02d
Repository branch: master
System Description: Ubuntu 22.04.1 LTS

Configured using:
 'configure --without-libgmp'

Configured features:
ACL CAIRO FREETYPE GIF GLIB GNUTLS GSETTINGS HARFBUZZ JPEG LIBSELINUX
LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG SECCOMP SOUND SQLITE3 THREADS
TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM LUCID ZLIB

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

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search time-date subr-x mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils term/screen term/xterm xterm byte-opt gv bytecomp
byte-compile cconv rmc iso-transl tooltip eldoc paren electric uniquify
ediff-hook vc-hooks lisp-float-type elisp-mode 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 lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
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 emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs faces cus-face macroexp files window
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget keymap hashtable-print-readable backquote threads inotify
dynamic-setting system-font-setting font-render-setting cairo x-toolkit
x multi-tty make-network-process emacs)

Memory information:
((conses 16 46148 9345)
 (symbols 48 5790 0)
 (strings 32 15049 1660)
 (string-bytes 1 427012)
 (vectors 16 8356)
 (vector-slots 8 105418 10463)
 (floats 8 24 349)
 (intervals 56 216 0)
 (buffers 1000 11))

[-- Attachment #2: 0001-Fix-Python-completion-failure-under-certain-conditio.patch --]
[-- Type: application/octet-stream, Size: 5239 bytes --]

From 6bd4d7fdf16cda18a921c2b63221fc2aac84fb8c Mon Sep 17 00:00:00 2001
From: kobarity <kobarity@gmail.com>
Date: Sun, 9 Oct 2022 15:39:52 +0900
Subject: [PATCH] Fix Python completion failure under certain conditions

* lisp/progmodes/python.el (python-shell-send-string-no-output):
Invalidate `comint-snapshot-last-prompt'.
* test/lisp/progmodes/python-tests.el (python-tests-shell-wait-for-prompt):
New helper function.
(python-tests-with-temp-buffer-with-shell): New helper macro.
(python-shell-completion-1, python-shell-completion-native-1)
(python-shell-completion-native-with-ffap-1)
(python-shell-completion-native-with-eldoc-1): New tests.
---
 lisp/progmodes/python.el            |  2 +
 test/lisp/progmodes/python-tests.el | 90 +++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 80c5b31b6e..9f29eb417e 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -3417,6 +3417,8 @@ python-shell-send-string-no-output
              (lambda (_proc str)
                (with-current-buffer (process-buffer process)
                  (python-shell-output-filter str))))
+            ((symbol-function 'comint-snapshot-last-prompt)
+             #'ignore)
             (python-shell-output-filter-in-progress t)
             (inhibit-quit t))
     (or
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index fdaedb5fd7..eed9c18ede 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -43,6 +43,34 @@ python-tests-with-temp-buffer
        (goto-char (point-min))
        ,@body)))
 
+(defun python-tests-shell-wait-for-prompt ()
+  "Wait for the prompt in the shell buffer."
+   (with-current-buffer (python-shell-get-buffer)
+     (while (not (python-util-comint-last-prompt))
+       (sit-for 0.1))))
+
+(defmacro python-tests-with-temp-buffer-with-shell (contents &rest body)
+  "Create a `python-mode' enabled temp buffer with CONTENTS and `run-python'.
+BODY is code to be executed within the temp buffer.  Point is
+always located at the beginning of buffer.  Native completion is
+turned off.  Shell buffer will be killed on exit."
+  (declare (indent 1) (debug t))
+  `(with-temp-buffer
+     (let ((python-indent-guess-indent-offset nil)
+           (python-shell-completion-native-enable nil))
+       (python-mode)
+       (unwind-protect
+           (progn
+             (run-python nil t)
+             (insert ,contents)
+             (goto-char (point-min))
+             (python-tests-shell-wait-for-prompt)
+             ,@body)
+         (when (python-shell-get-buffer)
+           (python-shell-with-shell-buffer
+             (let (kill-buffer-hook kill-buffer-query-functions)
+               (kill-buffer))))))))
+
 (defmacro python-tests-with-temp-file (contents &rest body)
   "Create a `python-mode' enabled file with CONTENTS.
 BODY is code to be executed within the temp buffer.  Point is
@@ -4365,6 +4393,68 @@ python-shell-completion-native-interpreter-disabled-p-1
          (python-shell-interpreter "/some/path/to/bin/pypy"))
     (should (python-shell-completion-native-interpreter-disabled-p))))
 
+(ert-deftest python-shell-completion-1 ()
+  (skip-unless (executable-find python-tests-shell-interpreter))
+  (python-tests-with-temp-buffer-with-shell
+   "
+import abc
+"
+   (let ((inhibit-message t))
+     (python-shell-send-buffer)
+     (python-tests-shell-wait-for-prompt)
+     (goto-char (point-max))
+     (insert "abc.")
+     (should (completion-at-point))
+     (insert "A")
+     (should (completion-at-point)))))
+
+(ert-deftest python-shell-completion-native-1 ()
+  (skip-unless (executable-find python-tests-shell-interpreter))
+  (python-tests-with-temp-buffer-with-shell
+   "
+import abc
+"
+   (let ((inhibit-message t))
+     (python-shell-completion-native-turn-on)
+     (python-shell-send-buffer)
+     (python-tests-shell-wait-for-prompt)
+     (goto-char (point-max))
+     (insert "abc.")
+     (should (completion-at-point))
+     (insert "A")
+     (should (completion-at-point)))))
+
+(ert-deftest python-shell-completion-native-with-ffap-1 ()
+  (skip-unless (executable-find python-tests-shell-interpreter))
+  (python-tests-with-temp-buffer-with-shell
+   "
+import abc
+"
+   (let ((inhibit-message t))
+     (python-shell-completion-native-turn-on)
+     (python-shell-send-buffer)
+     (python-tests-shell-wait-for-prompt)
+     (goto-char (point-max))
+     (insert "abc.")
+     ;; This is called when FFAP is enabled and a find-file function is called.
+     (python-ffap-module-path "abc.")
+     (should (completion-at-point)))))
+
+(ert-deftest python-shell-completion-native-with-eldoc-1 ()
+  (skip-unless (executable-find python-tests-shell-interpreter))
+  (python-tests-with-temp-buffer-with-shell
+   "
+import abc
+"
+   (let ((inhibit-message t))
+     (python-shell-completion-native-turn-on)
+     (python-shell-send-buffer)
+     (python-tests-shell-wait-for-prompt)
+     (goto-char (point-max))
+     (insert "abc.")
+     ;; This is called by idle-timer when ElDoc is enabled.
+     (python-eldoc-function)
+     (should (completion-at-point)))))
 
 
 \f
-- 
2.34.1

bug#58389: 29.0.50; Python completion fails under certain conditions

[kobarity]

I wrote:
> I think `comint-snapshot-last-prompt' should not be called as long as 
> `comint-output-filter' is not called and the Inferior Python Shell
> buffer is not changed.  Attached is a patch to fix this issue.

Sorry, I was not aware of other processes using comint.  It might
affect other processes.  I will reconsider the patch.

Best Regards,

bug#58389: 29.0.50; Python completion fails under certain conditions

[kobarity]

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

I wrote:
> 
> I wrote:
> > I think `comint-snapshot-last-prompt' should not be called as long as 
> > `comint-output-filter' is not called and the Inferior Python Shell
> > buffer is not changed.  Attached is a patch to fix this issue.
> 
> Sorry, I was not aware of other processes using comint.  It might
> affect other processes.  I will reconsider the patch.

Attached is the revised patch to fix this issue.  Instead of
invalidating `comint-snapshot-last-prompt', I added codes to save and
restore the buffer local variable `comint-last-prompt-overlay' or
`comint-last-prompt'.

Best Regards,

[-- Attachment #2: 0001-Fix-Python-completion-failure-under-certain-conditio.patch --]
[-- Type: application/octet-stream, Size: 7044 bytes --]

From 5962306d13d77aac25e0d991e8aaaab34d499c86 Mon Sep 17 00:00:00 2001
From: kobarity <kobarity@gmail.com>
Date: Mon, 10 Oct 2022 22:24:17 +0900
Subject: [PATCH] Fix Python completion failure under certain conditions

* lisp/progmodes/python.el (python-shell-send-string-no-output):
Save and restore `comint-last-prompt-overlay' or `comint-last-prompt'.
* test/lisp/progmodes/python-tests.el (python-tests-shell-wait-for-prompt):
New helper function.
(python-tests-with-temp-buffer-with-shell): New helper macro.
(python-shell-completion-1, python-shell-completion-native-1)
(python-shell-completion-native-with-ffap-1)
(python-shell-completion-native-with-eldoc-1): New tests (bug#58389).
---
 lisp/progmodes/python.el            | 26 +++++---
 test/lisp/progmodes/python-tests.el | 93 +++++++++++++++++++++++++++++
 2 files changed, 111 insertions(+), 8 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 80c5b31b6e..680b57fc3e 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -3413,15 +3413,25 @@ python-shell-send-string-no-output
   "Send STRING to PROCESS and inhibit output.
 Return the output."
   (or process (setq process (python-shell-get-process-or-error)))
-  (cl-letf (((process-filter process)
-             (lambda (_proc str)
-               (with-current-buffer (process-buffer process)
-                 (python-shell-output-filter str))))
-            (python-shell-output-filter-in-progress t)
-            (inhibit-quit t))
+  (cl-letf* (((process-filter process)
+              (lambda (_proc str)
+                (with-current-buffer (process-buffer process)
+                  (python-shell-output-filter str))))
+             (python-shell-output-filter-in-progress t)
+             (inhibit-quit t)
+             (buffer (process-buffer process))
+             (last-prompt (cond ((boundp 'comint-last-prompt-overlay)
+                                 'comint-last-prompt-overlay)
+                                ((boundp 'comint-last-prompt)
+                                 'comint-last-prompt)))
+             (last-prompt-value (buffer-local-value last-prompt buffer)))
     (or
      (with-local-quit
-       (python-shell-send-string string process)
+       (unwind-protect
+           (python-shell-send-string string process)
+         (when (not (null last-prompt))
+           (with-current-buffer buffer
+             (set last-prompt last-prompt-value))))
        (while python-shell-output-filter-in-progress
          ;; `python-shell-output-filter' takes care of setting
          ;; `python-shell-output-filter-in-progress' to NIL after it
@@ -3430,7 +3440,7 @@ python-shell-send-string-no-output
        (prog1
            python-shell-output-filter-buffer
          (setq python-shell-output-filter-buffer nil)))
-     (with-current-buffer (process-buffer process)
+     (with-current-buffer buffer
        (comint-interrupt-subjob)))))
 
 (defun python-shell-internal-send-string (string)
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index fdaedb5fd7..60ff9bb613 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -43,6 +43,37 @@ python-tests-with-temp-buffer
        (goto-char (point-min))
        ,@body)))
 
+(defun python-tests-shell-wait-for-prompt ()
+  "Wait for the prompt in the shell buffer."
+  (python-shell-with-shell-buffer
+    (while (not (if-let ((prompt (python-util-comint-last-prompt)))
+                    (python-shell-comint-end-of-output-p
+                     (buffer-substring-no-properties
+                      (car prompt) (cdr prompt)))))
+      (sit-for 0.1))))
+
+(defmacro python-tests-with-temp-buffer-with-shell (contents &rest body)
+  "Create a `python-mode' enabled temp buffer with CONTENTS and `run-python'.
+BODY is code to be executed within the temp buffer.  Point is
+always located at the beginning of buffer.  Native completion is
+turned off.  Shell buffer will be killed on exit."
+  (declare (indent 1) (debug t))
+  `(with-temp-buffer
+     (let ((python-indent-guess-indent-offset nil)
+           (python-shell-completion-native-enable nil))
+       (python-mode)
+       (unwind-protect
+           (progn
+             (run-python nil t)
+             (insert ,contents)
+             (goto-char (point-min))
+             (python-tests-shell-wait-for-prompt)
+             ,@body)
+         (when (python-shell-get-buffer)
+           (python-shell-with-shell-buffer
+             (let (kill-buffer-hook kill-buffer-query-functions)
+               (kill-buffer))))))))
+
 (defmacro python-tests-with-temp-file (contents &rest body)
   "Create a `python-mode' enabled file with CONTENTS.
 BODY is code to be executed within the temp buffer.  Point is
@@ -4365,6 +4396,68 @@ python-shell-completion-native-interpreter-disabled-p-1
          (python-shell-interpreter "/some/path/to/bin/pypy"))
     (should (python-shell-completion-native-interpreter-disabled-p))))
 
+(ert-deftest python-shell-completion-1 ()
+  (skip-unless (executable-find python-tests-shell-interpreter))
+  (python-tests-with-temp-buffer-with-shell
+   "
+import abc
+"
+   (let ((inhibit-message t))
+     (python-shell-send-buffer)
+     (python-tests-shell-wait-for-prompt)
+     (goto-char (point-max))
+     (insert "abc.")
+     (should (completion-at-point))
+     (insert "A")
+     (should (completion-at-point)))))
+
+(ert-deftest python-shell-completion-native-1 ()
+  (skip-unless (executable-find python-tests-shell-interpreter))
+  (python-tests-with-temp-buffer-with-shell
+   "
+import abc
+"
+   (let ((inhibit-message t))
+     (python-shell-completion-native-turn-on)
+     (python-shell-send-buffer)
+     (python-tests-shell-wait-for-prompt)
+     (goto-char (point-max))
+     (insert "abc.")
+     (should (completion-at-point))
+     (insert "A")
+     (should (completion-at-point)))))
+
+(ert-deftest python-shell-completion-native-with-ffap-1 ()
+  (skip-unless (executable-find python-tests-shell-interpreter))
+  (python-tests-with-temp-buffer-with-shell
+   "
+import abc
+"
+   (let ((inhibit-message t))
+     (python-shell-completion-native-turn-on)
+     (python-shell-send-buffer)
+     (python-tests-shell-wait-for-prompt)
+     (goto-char (point-max))
+     (insert "abc.")
+     ;; This is called when FFAP is enabled and a find-file function is called.
+     (python-ffap-module-path "abc.")
+     (should (completion-at-point)))))
+
+(ert-deftest python-shell-completion-native-with-eldoc-1 ()
+  (skip-unless (executable-find python-tests-shell-interpreter))
+  (python-tests-with-temp-buffer-with-shell
+   "
+import abc
+"
+   (let ((inhibit-message t))
+     (python-shell-completion-native-turn-on)
+     (python-shell-send-buffer)
+     (python-tests-shell-wait-for-prompt)
+     (goto-char (point-max))
+     (insert "abc.")
+     ;; This is called by idle-timer when ElDoc is enabled.
+     (python-eldoc-function)
+     (should (completion-at-point)))))
 
 
 \f
-- 
2.34.1

bug#58389: 29.0.50; Python completion fails under certain conditions

[Lars Ingebrigtsen]

kobarity <kobarity@gmail.com> writes:

> Attached is the revised patch to fix this issue.  Instead of
> invalidating `comint-snapshot-last-prompt', I added codes to save and
> restore the buffer local variable `comint-last-prompt-overlay' or
> `comint-last-prompt'.

Thanks; pushed to Emacs 29.