Function launching default editor

I have written a function that launches the default editor set in git config, right now I have managed to get it working for Sublime, nano and Vim.

def launchEditor(editor):
    """ this function launches the default editor
        for user to compose message to be sent
        along with git diff.

        Args:
            editor(str): name or path of editor
        Returns:
            msg(str): html formatted message
    """

    filePath = os.path.join(os.getcwd(), "compose.txt")
    wfh = open(filePath, 'w')
    wfh.close()

    if os.path.exists(filePath):
        # using sublime
        if re.search(r'ubl', editor):
            diff = subprocess.Popen(['cat',filePath], stdout=subprocess.PIPE)
            pr = subprocess.Popen(
                editor,
                stdout=subprocess.PIPE,
                stderr=subprocess.PIPE,
                shell=True,
                stdin=diff.stdout
                )
            pr.wait()
            if pr.returncode == 0:
                msg = pr.stdout.read()
        else:
            # using vim or nano
            pr = subprocess.Popen([editor, filePath], stdin=open('/dev/tty', 'r'))
            pr.wait()
            if pr.returncode == 0:
                with open(filePath, 'r') as fh:
                    msg = fh.readlines()
    os.remove(filePath)
    return "".join(msg).replace("\n","")

Any suggestion on improvement and adding support to other text editors is welcome!!

Revision update:

```
Traceback (most recent call last):
File "/Users/san/Development/executables//git-ipush", line 268, in
sys.exit(main())
File "/Users/san/Development/executables//git-ipush", line 46, in main
preCheck(args)
File "/Users/sanjeevkumar/Development/executables//git-ipush", line 156, in preCheck
message = launchEditor(editor)
File "/Users/san/Development/executables//git-ipush", line 77, in launchEditor
if subprocess.call([editor, f.name]) != 0:
File "/Library/Frameworks/Python.framework/Ver

1. Comments on your code

-
The function is poorly specified in that it combines two tasks: (i) it gets input from the user via an editor; (ii) it replaces newlines in the input with
. But what I just want the input and don't want any HTML conversion (especially not a half-baked conversion like this one)?

It would be better to decompose this function into two pieces. In what follows I'm just going to discuss piece (i), launching the editor and capturing the input.

-
There are missing imports (os and subprocess).

-
The function always uses the file compose.txt in the current directory. This means that if I already had a file with that name, it would be erased and then deleted, which would be really annoying.

It would be better to create a temporary file for this purpose, using Python's tempfile.NamedTemporaryFile.

-
This code:

diff = subprocess.Popen(['cat',filePath], stdout=subprocess.PIPE)
pr = subprocess.Popen(..., stdin=diff.stdout)

is a classic "useless use of cat". If you read the documentation for subprocess.Popen, you'll see:


stdin, stdout and stderr specify the executed program’s standard input, standard output and standard error file handles, respectively. Valid values are PIPE, DEVNULL, an existing file descriptor (a positive integer), an existing file object, and None.

(my emphasis) so you can write:

pr = subprocess.Popen(..., stdin=open(filePath))

and save a process. (But actually this is unnecessary, as explained below.)

-
This code:

if re.search(r'ubl', editor):

doesn't seem like a very robust way to ensure that editor is Sublime Text. I mean, for all you know I could have just run:

$ ln /usr/bin/vi ubl

It's best to treat all the editors the same. After all, git commit doesn't have any special case for Sublime Text, so neither should you.

In particular, you don't need to go through all that subprocess.Popen shenanigans to open a file in Sublime. I find that

subprocess.call(['subl', filename])

works fine.

-
If all you are going to do with a subprocess is wait for it to exit:

pr = subprocess.Popen([editor, filePath], stdin=open('/dev/tty', 'r'))
pr.wait()
if pr.returncode == 0:

then use subprocess.call instead of subprocess.Popen:

if subprocess.call([editor, filePath]) == 0:

-
Specifying stdin=open('/dev/tty', 'r') is unnecessary. Let the editor decide how it wants to get input from the user.

-
If the editor returned with a non-zero code, your function doesn't report an error, it just continues running (but with nothing assigned to msg) until it reaches "".join(msg) which fails with a mysterious TypeError. Better to raise an exception if the editor returned an error code.

(I submitted a bug report for the mysterious TypeError: see Python issue 20507.)

-
Your function splits the input into lines by calling msg = fh.readlines(), and then it joins these lines back together again with "".join(msg). This is pointless: if you just want the contents of the file as a string, write msg = fh.read() instead.

1. Revised code

import subprocess
import tempfile

def input_via_editor(editor):
    """Launch editor on an empty temporary file, wait for it to exit, and
    if it exited successfully, return the contents of the file.

    """
    with tempfile.NamedTemporaryFile() as f:
        f.close()
        try:
            subprocess.check_call([editor, f.name])
        except subprocess.CalledProcessError as e:
            raise IOError("{} exited with code {}.".format(editor, e.returncode))
        with open(f.name) as g:
            return g.read()

StackExchange Code Review Q#40652, answer score: 4