bug#64406: [PATCH] Improve commands to manage Python imports

[Matthias Meulien <orontee@gmail.com>]

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

Matthias Meulien <orontee@gmail.com> writes:

> Augusto Stoffel <arstoffel@gmail.com> writes:
>> (...)
>> This change implies that the "success" branch may run if the exit code
>> is nonzero (without knowing all the isort internals, it can't be
>> excluded that an exit code > 2 is used somewhere).  I suggest instead a
>> (pcase status ...) to construct the " (maybe... ?)" segment of the error
>> message.
>
> Thanks, I'll fix this.

>> We could also take this opportunity to distinguish between random
>> exceptions happening in the script (which likely leads to exit code 1)
>> and the ModuleNotFoundError case.
>
> Good point, I'll improve this too.

Here are two patches.

The first one revert part of the original patch where I introduced a
useless detailed check of isort exit status itself.

The second one relates to your remarks: It shift the custom exit status
of the Python script and make sure that an error is reported assoon as
exit code >0.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-Improve-Python-imports-management-commands.patch --]
[-- Type: text/x-diff, Size: 1276 bytes --]

From 2463a33c5c3b8eb38da8f064459900159330bb47 Mon Sep 17 00:00:00 2001
From: Matthias Meulien <orontee@gmail.com>
Date: Sun, 9 Jul 2023 07:45:57 +0200
Subject: [PATCH 1/2] Fix "Improve Python imports management commands"

* lisp/progmodes/python.el (python--do-isort): The isort library don't
check for its version itself!
---
 lisp/progmodes/python.el | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 4291ab03ca6..8ce10621ed3 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -6556,13 +6556,9 @@ python--do-isort
                                nil (list temp nil) nil
                                "-m" "isort" "-" args))
                 (tick (buffer-chars-modified-tick)))
-            (cond
-             ((eq 1 status)
+            (unless (eq 0 status)
               (error "%s exited with status %s (maybe isort is missing?)"
                      python-interpreter status))
-             ((eq 2 status)
-              (error "%s exited with status %s (maybe isort version is <5.7.0?)"
-                     python-interpreter status)))
             (replace-buffer-contents temp)
             (not (eq tick (buffer-chars-modified-tick)))))))))
 
-- 
2.39.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Improve-Python-imports-management-commands.patch --]
[-- Type: text/x-diff, Size: 2282 bytes --]

From 65e5229e62fffc96251e28b90a56c8d79c53703c Mon Sep 17 00:00:00 2001
From: Matthias Meulien <orontee@gmail.com>
Date: Sun, 9 Jul 2023 07:48:57 +0200
Subject: [PATCH 2/2] Improve Python imports management commands

* lisp/progmodes/python.el (python--list-imports): Prefer to use an
exit status >1.
(python--list-imports-check-status): Check Python script status.
---
 lisp/progmodes/python.el | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 8ce10621ed3..ea68d7ee59d 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -6451,9 +6451,9 @@ python--list-imports
 try:
     from isort import find_imports_in_stream, find_imports_in_paths
 except ModuleNotFoundError:
-    exit(1)
-except ImportError:
     exit(2)
+except ImportError:
+    exit(3)
 
 query, files, result = argv[1] or None, argv[2:], {}
 
@@ -6484,6 +6484,17 @@ python--import-sources
                   (project-files proj))
     (list default-directory)))
 
+(defun python--list-imports-check-status (status)
+  (unless (eq 0 status)
+    (let* ((details
+            (cond
+             ((eq 2 status) " (maybe isort is missing?)")
+             ((eq 3 status) " (maybe isort version is less than 5.7.0?)")
+             (t "")))
+           (msg
+            (concat "%s exited with status %s" details)))
+      (error msg python-interpreter status))))
+
 (defun python--list-imports (name source)
   "List all Python imports matching NAME in SOURCE.
 If NAME is nil, list all imports.  SOURCE can be a buffer or a
@@ -6507,13 +6518,7 @@ python--list-imports
                                 (or name "")
                                 (mapcar #'file-local-name source)))))
              lines)
-        (cond
-         ((eq 1 status)
-          (error "%s exited with status %s (maybe isort is missing?)"
-                 python-interpreter status))
-         ((eq 2 status)
-          (error "%s exited with status %s (maybe isort version is <5.7.0?)"
-                 python-interpreter status)))
+        (python--list-imports-check-status status)
         (goto-char (point-min))
         (while (not (eobp))
 	  (push (buffer-substring-no-properties (point) (pos-eol))
-- 
2.39.2