From 48be3a7eafd9534f210ff9c534d6b506127f3bc4 Mon Sep 17 00:00:00 2001
From: ababyduck <kassisaf@gmail.com>
Date: Wed, 24 Apr 2019 18:43:50 -0700
Subject: [PATCH] Make requested changes for PR 3236

- Refactored convert and identify command names to an ArtResizer
member variable, set on ArtResizer init. Functions that use this info
will now access it from there.
- Changed the way `cmd` variables are written so that the command name
and command args are assigned directly to `cmd`, rather than doing
`command_output(cmd + args)`
- `get_im_version()` will now always return a tuple containing two
values: a tuple representing the version, and either a bool or None
flag representing whether we should send legacy commands to ImageMagick
- Improved readability of successful return value in `get_im_version()`
---
 beets/util/artresizer.py | 47 ++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index 5e3e983867..427a5411d9 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -93,13 +93,12 @@ def im_resize(maxwidth, path_in, path_out=None):
     # than the given width while maintaining the aspect ratio
     # with regards to the height.
     try:
-        cmds = (['magick'], ['convert'])
-        cmd = cmds[0] if not ArtResizer.shared.im_legacy else cmds[1]
-        args = [util.syspath(path_in, prefix=False),
+        cmd = ArtResizer.shared.im_convert_cmd + \
+            [util.syspath(path_in, prefix=False),
                 '-resize', '{0}x>'.format(maxwidth),
                 util.syspath(path_out, prefix=False)]
 
-        util.command_output(cmd + args)
+        util.command_output(cmd)
     except subprocess.CalledProcessError:
         log.warning(u'artresizer: IM convert failed for {0}',
                     util.displayable_path(path_in))
@@ -124,12 +123,11 @@ def pil_getsize(path_in):
 
 
 def im_getsize(path_in):
-    cmds = (['magick', 'identify'], ['identify'])
-    cmd = cmds[0] if not ArtResizer.shared.im_legacy else cmds[1]
-    args = ['-format', '%w %h', util.syspath(path_in, prefix=False)]
-
     try:
-        out = util.command_output(cmd + args)
+        cmd = ArtResizer.shared.im_identify_cmd + \
+            ['-format', '%w %h', util.syspath(path_in, prefix=False)]
+
+        out = util.command_output(cmd)
     except subprocess.CalledProcessError as exc:
         log.warning(u'ImageMagick size query failed')
         log.debug(
@@ -180,6 +178,12 @@ def __init__(self):
 
         if self.method[0] == IMAGEMAGICK:
             self.im_legacy = self.method[2]
+            if self.im_legacy:
+                self.im_convert_cmd = ['convert']
+                self.im_identify_cmd = ['identify']
+            else:
+                self.im_convert_cmd = ['magick']
+                self.im_identify_cmd = ['magick', 'identify']
 
     def resize(self, maxwidth, path_in, path_out=None):
         """Manipulate an image file according to the method, returning a
@@ -246,30 +250,31 @@ def get_im_version():
     Try invoking ImageMagick's "magick". If "magick" is unavailable,
     as with older versions, fall back to "convert"
 
-    Our iterator `im_legacy` will be non-zero when the first command
-    fails, and will be returned in a tuple along with the version
+    Our iterator will be non-zero when the first command fails, and will
+    be returned in a tuple along with the version.
     """
-    cmds = ('magick', 'convert')
-    for im_legacy, cmd in enumerate(cmds):
+    cmd_names = (['magick'],
+                 ['convert'])
+    for i, cmd_name in enumerate(cmd_names):
 
         try:
-            out = util.command_output([cmd, '--version'])
+            cmd = cmd_name + ['--version']
+            out = util.command_output(cmd)
 
             if b'imagemagick' in out.lower():
                 pattern = br".+ (\d+)\.(\d+)\.(\d+).*"
                 match = re.search(pattern, out)
+                version = (int(match.group(1)),
+                           int(match.group(2)),
+                           int(match.group(3)))
+                legacy = bool(i)
                 if match:
-                    return ((int(match.group(1)),
-                            int(match.group(2)),
-                            int(match.group(3))),
-                            bool(im_legacy)
-                            )
+                    return (version, legacy)
 
         except (subprocess.CalledProcessError, OSError) as exc:
             log.debug(u'ImageMagick version check failed: {}', exc)
-            return None
 
-        return (0,)
+        return (0, None)
 
 
 def get_pil_version():