make ExternalCommand work on command lists

instead of command strings. subprocess.Popen
takes a string list that determines the subcommand to call.
This makes ExternalCommand directly accept string lists to pass on to Popen.
This change obsoletes the 'path' parameter, as callers can now simply
construct the command list accordingly.

OpenAttachmentCommand and EditCommand are updated to match this API change.

2 files changed, 51 insertions, 42 deletions

diff --git a/alot/commands/globals.py b/alot/commands/globals.py
index 07d2caa5..bc1e779a 100644
--- a/alot/commands/globals.py
+++ b/alot/commands/globals.py
@@ -134,13 +134,11 @@ class RefreshCommand(Command):
 )
 class ExternalCommand(Command):
     """run external command"""
-    def __init__(self, cmd, path=None, stdin=None, shell=False,spawn=False,
+    def __init__(self, cmdlist, stdin=None, shell=False, spawn=False,
                  refocus=True, thread=False, on_success=None, **kwargs):
         """
-        :param cmd: the command to call
-        :type cmd: str
-        :param path: a path to a file (or None)
-        :type path: str
+        :param cmdlist: the command to call
+        :type cmdlist: list of str
         :param stdin: input to pipe to the process
         :type stdin: file or str
         :param spawn: run command in a new terminal
@@ -154,8 +152,7 @@ class ExternalCommand(Command):
         :param on_success: code to execute after command successfully exited
         :type on_success: callable
         """
-        self.commandstring = cmd
-        self.path = path
+        self.cmdlist = cmdlist
         self.stdin = stdin
         self.shell = shell
         self.spawn = spawn
@@ -165,6 +162,7 @@ class ExternalCommand(Command):
         Command.__init__(self, **kwargs)
 
     def apply(self, ui):
+        logging.debug('cmdlist: %s' % self.cmdlist)
         callerbuffer = ui.current_buffer
 
         #set standard input for subcommand
@@ -186,27 +184,22 @@ class ExternalCommand(Command):
                 logging.info('refocussing')
                 ui.buffer_focus(callerbuffer)
 
-        def thread_code(*args):
-            if self.path:
-                if '{}' in self.commandstring:
-                    cmd = self.commandstring.replace('{}',
-                            helper.shell_quote(self.path))
-                else:
-                    cmd = '%s %s' % (self.commandstring,
-                                     helper.shell_quote(self.path))
-            else:
-                cmd = self.commandstring
+        if self.spawn:
+            term_cmd = settings.get('terminal_cmd', '')
+            term_cmd = term_cmd.encode('utf-8', errors='ignore')
+            logging.info('spawn in terminal: %s' % term_cmd)
+            termcmdlist = shlex.split(term_cmd)
+            logging.info('term cmdlist: %s' % termcmdlist)
+            self.cmdlist = termcmdlist + self.cmdlist
 
-            if self.spawn:
-                cmd = '%s %s' % (settings.get('terminal_cmd'), cmd)
-            cmd = cmd.encode('utf-8', errors='ignore')
-            logging.info('calling external command: %s' % cmd)
+        logging.info('calling external command: %s' % self.cmdlist)
+
+        def thread_code(*args):
             try:
-                cmdlist = shlex.split(cmd)
                 if stdin == None:
-                    ret = subprocess.call(cmdlist, shell=self.shell)
+                    ret = subprocess.call(self.cmdlist, shell=self.shell)
                 else:
-                    proc = subprocess.Popen(cmdlist, shell=self.shell,
+                    proc = subprocess.Popen(self.cmdlist, shell=self.shell,
                                             stdin=subprocess.PIPE)
                     out, err = proc.communicate(stdin.read())
                     ret = proc.wait()
@@ -241,26 +234,37 @@ class EditCommand(ExternalCommand):
         :param thread: run asynchronously, don't block alot
         :type thread: bool
         """
-        self.path = path
         self.spawn = settings.get('editor_spawn') or spawn
         if thread != None:
             self.thread = thread
         else:
             self.thread = settings.get('editor_in_thread')
 
-        self.editor_cmd = None
+        editor_cmdstring = None
         if os.path.isfile('/usr/bin/editor'):
-            self.editor_cmd = '/usr/bin/editor'
-        self.editor_cmd = os.environ.get('EDITOR', self.editor_cmd)
-        self.editor_cmd = settings.get('editor_cmd') or self.editor_cmd
-        logging.debug('using editor_cmd: %s' % self.editor_cmd)
+            editor_cmdstring = '/usr/bin/editor'
+        editor_cmdstring = os.environ.get('EDITOR', editor_cmdstring)
+        editor_cmdstring = settings.get('editor_cmd') or editor_cmdstring
+        logging.debug('using editor_cmd: %s' % editor_cmdstring)
+
+        self.cmdlist = None
+        if '%s' in editor_cmdstring:
+            cmdstring = editor_cmdstring.replace('%s',
+                                                 helper.shell_quote(path))
+            cmdstring = cmdstring.encode('utf-8', errors='ignore')
+            self.cmdlist = shlex.split(cmdstring)
+        else:
+            cmdstring = editor_cmdstring.encode('utf-8', errors='ignore')
+            self.cmdlist = shlex.split(cmdstring) + [path]
+
+        logging.debug(self.cmdlist)
 
-        ExternalCommand.__init__(self, self.editor_cmd, path=self.path,
+        ExternalCommand.__init__(self, self.cmdlist,
                                  spawn=self.spawn, thread=self.thread,
                                  **kwargs)
 
     def apply(self, ui):
-        if self.editor_cmd == None:
+        if self.cmdlist == None:
             ui.notify('no editor set', priority='error')
         else:
             return ExternalCommand.apply(self, ui)
diff --git a/alot/commands/thread.py b/alot/commands/thread.py
index 9319fb2c..c304fa0c 100644
--- a/alot/commands/thread.py
+++ b/alot/commands/thread.py
@@ -666,19 +666,21 @@ class OpenAttachmentCommand(Command):
         logging.info('open attachment')
         mimetype = self.attachment.get_content_type()
 
-        handler, entry = settings.mailcap_find_match(mimetype)
-        if handler:
-            afterwards = None
+        # returns pair of preliminary command string and entry dict containing
+        # more info. We only use the dict and construct the command ourselves
+        _, entry = settings.mailcap_find_match(mimetype)
+        if entry:
+            afterwards = None  # callback, will rm tempfile if used
             handler_stdin = None
             tempfile_name = None
-            handler_commandstring = entry['view']
+            handler_raw_commandstring = entry['view']
             # read parameter
             part = self.attachment.get_mime_representation()
             parms = tuple(map('='.join, part.get_params()))
 
             # in case the mailcap defined command contains no '%s',
             # we pipe the files content to the handling command via stdin
-            if '%s' in handler_commandstring:
+            if '%s' in handler_raw_commandstring:
                 nametemplate = entry.get('nametemplate', '%s')
                 prefix, suffix = parse_mailcap_nametemplate(nametemplate)
                 tmpfile = tempfile.NamedTemporaryFile(delete=False,
@@ -688,23 +690,26 @@ class OpenAttachmentCommand(Command):
                 tempfile_name = tmpfile.name
                 self.attachment.write(tmpfile)
                 tmpfile.close()
+
                 def afterwards():
                     os.remove(tempfile_name)
             else:
                 handler_stdin = StringIO()
                 self.attachment.write(handler_stdin)
 
+            # create handler command list
+            handler_cmd = mailcap.subst(handler_raw_commandstring, mimetype,
+                                        filename=tempfile_name, plist=parms)
 
-            # create and call external command
-            handler = mailcap.subst(handler_commandstring, mimetype,
-                                    filename=tempfile_name, plist=parms)
-
+            handler_cmd = handler_cmd.encode('utf-8', errors='ignore')
+            handler_cmdlist = shlex.split(handler_cmd)
 
             # 'needsterminal' makes handler overtake the terminal
             nt = entry.get('needsterminal', None)
             overtakes = (nt is None)
 
-            ui.apply_command(ExternalCommand(handler, stdin=handler_stdin,
+            ui.apply_command(ExternalCommand(handler_cmdlist,
+                                             stdin=handler_stdin,
                                              on_success=afterwards,
                                              thread=overtakes))
         else: