Subject: bug#57184: [PATCH] Assorted improvements to python.el

From: Augusto Stoffel <[email-masked]>
Date: Sun, 14 Aug 2022 10:42:11 +0200

Hi Stefan,

I've attached new patches, see below for comments on your comments.  (As
to the smaller style changes, feel free to copyedit my patches before
merging in case I still let something slip.)

>From 34a28cf6cfca66751de2e0f8269e685e9638d8ea Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <[email-masked]>
Date: Sat, 13 Aug 2022 17:04:17 +0200
Subject: [PATCH 1/4] python-check-command: Don't use absolute file names

Absolute executable file names are incompatible with Tramp and
packages that switch between virtualenvs.

* lisp/progmodes/python.el (python-check-command): Don't use absolute
file names.
---
 lisp/progmodes/python.el | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 96f9d14832..3176d12bd1 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -4702,9 +4702,9 @@ ffap-alist
 ;;; Code check
 
 (defcustom python-check-command
-  (or (executable-find "pyflakes")
-      (executable-find "epylint")
-      "install pyflakes, pylint or something else")
+  (cond ((executable-find "pyflakes") "pyflakes")
+        ((executable-find "epylint") "epylint")
+        (t "pyflakes"))
   "Command used to check a Python file."
   :type 'string)
 
-- 
2.37.1

>From 86ed1979e39a6ee211259912b764b8b755ee129e Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <[email-masked]>
Date: Sat, 13 Aug 2022 17:39:57 +0200
Subject: [PATCH 2/4] python.el: Adjustments to Flymake backend

* lisp/progmodes/python (python-flymake-command): Advertise possiblity
to use pylint.
(python-flymake-command-output-pattern): Make compatible with recent
versions of pyflakes.
---
 lisp/progmodes/python.el            | 12 ++++++----
 test/lisp/progmodes/python-tests.el | 34 +++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 3176d12bd1..f473fc0369 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -5625,9 +5625,13 @@ python-flymake-command
 This is a non empty list of strings, the checker tool possibly followed by
 required arguments.  Once launched it will receive the Python source to be
 checked as its standard input.
-To use `flake8' you would set this to (\"flake8\" \"-\")."
+To use `flake8' you would set this to (\"flake8\" \"-\").
+To use `pylint' you would set this to (\"pylint\" \"--from-stdin\" \"stdin\")."
   :version "26.1"
-  :type '(repeat string))
+  :type '(choice (const :tag "Pyflakes" ("pyflakes"))
+                 (const :tag "Flake8" ("flake8" "-"))
+                 (const :tag "Pylint" ("pylint" "--from-stdin" "stdin"))
+                 (repeat :tag "Custom command" string)))
 
 ;; The default regexp accommodates for older pyflakes, which did not
 ;; report the column number, and at the same time it's compatible with
@@ -5635,7 +5639,7 @@ python-flymake-command
 ;; TYPE
 (defcustom python-flymake-command-output-pattern
   (list
-   "^\\(?:<?stdin>?\\):\\(?1:[0-9]+\\):\\(?:\\(?2:[0-9]+\\):\\)? \\(?3:.*\\)$"
+   "^\\(?:<?stdin>?\\):\\(?1:[0-9]+\\):\\(?:\\(?2:[0-9]+\\):?\\)? \\(?3:.*\\)$"
    1 2 nil 3)
   "Specify how to parse the output of `python-flymake-command'.
 The value has the form (REGEXP LINE COLUMN TYPE MESSAGE): if
@@ -5647,7 +5651,7 @@ python-flymake-command-output-pattern
 If COLUMN or TYPE are nil or that index didn't match, that
 information is not present on the matched line and a default will
 be used."
-  :version "26.1"
+  :version "29.1"
   :type '(list regexp
                (integer :tag "Line's index")
                (choice
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index d303050fad..0bfdf1e76b 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -6152,6 +6152,40 @@ python-tests--fill-long-first-line
     a = 1
 ")))
 
+
+;;; Flymake
+
+(ert-deftest python-tests--flymake-command-output-pattern ()
+  (pcase-let ((`(,patt ,line ,col ,type ,msg)
+               python-flymake-command-output-pattern))
+    ;; Pyflakes output as of version 2.4.0
+    (let ((output "<stdin>:12:34 'a.b.c as d' imported but unused"))
+      (string-match patt output)
+      (should (equal (match-string line output) "12"))
+      (when col (should (equal (match-string col output) "34")))
+      (should (equal (match-string msg output)
+                     "'a.b.c as d' imported but unused")))
+    ;; Flake8 output as of version 4.0.1
+    (let ((output "stdin:12:34: F401 'a.b.c as d' imported but unused"))
+      (string-match patt output)
+      (should (equal (match-string line output) "12"))
+      (when col (should (equal (match-string col output) "34")))
+      (when type (should (equal (match-string type output) "F401")))
+      (should (equal (match-string msg output)
+                     (if type
+                         "'a.b.c as d' imported but unused"
+                       "F401 'a.b.c as d' imported but unused"))))
+    ;; Pylint output as of version 2.14.5
+    (let ((output "stdin:12:34: W0611: Unused import a.b.c (unused-import)"))
+      (string-match patt output)
+      (should (equal (match-string line output) "12"))
+      (when col (should (equal (match-string col output) "34")))
+      (when type (should (equal (match-string type output) "W0611")))
+      (should (equal (match-string msg output)
+                     (if type
+                         "Unused import a.b.c (unused-import)"
+                       "W0611: Unused import a.b.c (unused-import)"))))))
+
 (provide 'python-tests)
 
 ;;; python-tests.el ends here
-- 
2.37.1

>From b50964dd81be4e45c999fcfc617c1d4603c34668 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <[email-masked]>
Date: Sat, 13 Aug 2022 18:10:14 +0200
Subject: [PATCH 3/4] python.el: Add completion-predicate symbol property to
 commands

* lisp/progmodes/python.el: Add a completion-predicate property to
most commands defined in this file; some are only useful in
python-mode, others in inferior-python mode as well.
(python-skeleton-define, python-define-auxiliary-skeleton): Add
appropriate completion-predicate properties.
---
 lisp/progmodes/python.el | 75 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 67 insertions(+), 8 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index f473fc0369..a373390d94 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -4546,8 +4546,13 @@ python-skeleton-define
 be added to `python-mode-skeleton-abbrev-table'."
   (declare (indent 2))
   (let* ((name (symbol-name name))
-         (function-name (intern (concat "python-skeleton-" name))))
+         (function-name (intern (concat "python-skeleton-" name)))
+         (completion-predicate (lambda (_ buffer)
+                                 (provided-mode-derived-p
+                                  (buffer-local-value 'major-mode buffer)
+                                  'python-mode))))
     `(progn
+       (put ',function-name 'completion-predicate ,completion-predicate)
        (define-abbrev python-mode-skeleton-abbrev-table
          ,name "" ',function-name :system t)
        (setq python-skeleton-available
@@ -4573,13 +4578,15 @@ python-define-auxiliary-skeleton
       (setq skel
             `(< ,(format "%s:" name) \n \n
                 > _ \n)))
-    `(define-skeleton ,function-name
-       ,(or doc
-            (format "Auxiliary skeleton for %s statement." name))
-       nil
-       (unless (y-or-n-p ,msg)
-         (signal 'quit t))
-       ,@skel)))
+    `(progn
+       (put ',function-name 'completion-predicate #'ignore)
+       (define-skeleton ,function-name
+         ,(or doc
+              (format "Auxiliary skeleton for %s statement." name))
+         nil
+         (unless (y-or-n-p ,msg)
+           (signal 'quit t))
+         ,@skel))))
 
 (python-define-auxiliary-skeleton else)
 
@@ -5864,6 +5871,58 @@ python-mode
 
   (add-hook 'flymake-diagnostic-functions #'python-flymake nil t))
 
+;;; Completion predicates for M-x
+(dolist (sym '(python-check
+               python-fill-paragraph
+               python-indent-dedent-line
+               python-indent-dedent-line-backspace
+               python-indent-guess-indent-offset
+               python-indent-shift-left
+               python-indent-shift-right
+               python-mark-defun
+               python-nav-backward-block
+               python-nav-backward-defun
+               python-nav-backward-sexp
+               python-nav-backward-sexp-safe
+               python-nav-backward-statement
+               python-nav-backward-up-list
+               python-nav-beginning-of-block
+               python-nav-beginning-of-statement
+               python-nav-end-of-block
+               python-nav-end-of-defun
+               python-nav-end-of-statement
+               python-nav-forward-block
+               python-nav-forward-defun
+               python-nav-forward-sexp
+               python-nav-forward-sexp-safe
+               python-nav-forward-statement
+               python-nav-if-name-main
+               python-nav-up-list
+               python-shell-send-buffer
+               python-shell-send-defun
+               python-shell-send-statement))
+  (put sym 'completion-predicate (lambda (_ buffer)
+                                   (provided-mode-derived-p
+                                    (buffer-local-value 'major-mode buffer)
+                                    'python-mode))))
+
+(dolist (sym '(python-describe-at-point
+               python-eldoc-at-point
+               python-shell-completion-native-toggle
+               python-shell-completion-native-turn-off
+               python-shell-completion-native-turn-on
+               python-shell-completion-native-turn-on-maybe
+               python-shell-font-lock-cleanup-buffer
+               python-shell-font-lock-toggle
+               python-shell-font-lock-turn-off
+               python-shell-font-lock-turn-on
+               python-shell-package-enable
+               python-shell-completion-complete-or-indent  ))
+  (put sym 'completion-predicate (lambda (_ buffer)
+                                   (provided-mode-derived-p
+                                    (buffer-local-value 'major-mode buffer)
+                                    'python-mode 'inferior-python-mode))))
+
 (provide 'python)
 
 ;;; python.el ends here
-- 
2.37.1

>From fbd4cc631f1986a631da8619f58db2da62cbf578 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <[email-masked]>
Date: Sat, 13 Aug 2022 18:55:48 +0200
Subject: [PATCH 4/4] python-mode: Remove special outline-heading-end-regexp

It doesn't work well with the new type annotation syntax introduced in
Python 3.5, see bug#53913.

* lisp/progmodes/python.el (python-mode): Remove buffer-local setting
of outline-heading-end-regexp.
---
 lisp/progmodes/python.el | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index a373390d94..8eb28eb19e 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -5854,7 +5854,6 @@ python-mode
      nil))
 
   (setq-local outline-regexp (python-rx (* space) block-start))
-  (setq-local outline-heading-end-regexp ":[^\n]*\n")
   (setq-local outline-level
               (lambda ()
                 "`outline-level' function for Python mode."
-- 
2.37.1

On Sat, 13 Aug 2022 at 11:26, Stefan Kangas <[email-masked]> wrote:

>> Subject: [PATCH 2/4] python.el: Adjustments to Flymake back-end
>
> How about:
>
>     python.el: Support pyflakes 2.4+ in flymake
>

This commit includes a second change advertising the possibility to use
the pylint program.  Moreover, I'm not claiming that pyflakes changed in
version 2.4; the change probably happened further in the past.  So I
made this statement less specific.

(BTW, instead of writing things like "To use `flake8' you would set this
to ..." in a docstring, wouldn't it make sense for describe-variable to
list the suggested customization values from the :type specification?)

> Maybe include an example of the old/new pattern in comments?  Or even
> better actually: add a test for it.

I've added some tests.

>> From 95675a3f63bc420813992d7b599e99b303e4fd63 Mon Sep 17 00:00:00 2001
>> From: Augusto Stoffel <[email-masked]>
>> Date: Sat, 13 Aug 2022 18:10:14 +0200
>> Subject: [PATCH 3/4] python.el: Add completion-predicate symbol property to
>>  commands
>>
>> * lisp/progmodes/python.el: Add a completion-predicate property to
>> most commands defined in this file; some are only useful in
>> python-mode, others in inferior-python mode as well.
>> (python-skeleton-define, python-define-auxiliary-skeleton): Add
>> appropriate completion-predicate properties.
>
> This makes sense, but how about just creating a new mode
> python-common-mode that both python-mode and inferior-python-mode could
> inherit from?  Would that simplify things here?

In any case there are two cases to consider anyway: commands that only
make sense in Python source code, and commands that make sense in code
or in the REPL (we might eventually have code that makes sense just in
the REPL, that's not the case now).

So for the current purposes this suggestion wouldn't really simplify
anything.

> I'm fine with this as `outline-heading-end-regexp' doesn't seem to see
> much use in any case (and folding to one line somehow seems nicer
> anyways).
>
> However, it seems like we might be able to do find a fix here with just
> a small number of assumptions, if we really wanted to.

I guess the fix would be not to look for the next colon, but instead to
the next colon skipping over pairs of delimiters.  But then we couldn't
use regexps, and moreover things wouldn't work on incomplete code during
editing.

And I also think folding on one line makes just as much sense, if it
isn't better.  So it looks like removing this is the best option.