6
\$\begingroup\$

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","<br>")

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 <module>
    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/Versions/2.7/lib/python2.7/subprocess.py", line 524, in call
    return Popen(*popenargs, **kwargs).wait()
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 711, in __init__
    errread, errwrite)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 1308, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory
\$\endgroup\$
2
  • \$\begingroup\$ Isn't specifying core editor in git config good enough? Just trying to think a use-case or purpose of this. Thanks. \$\endgroup\$ – andHapp Feb 2 '14 at 11:52
  • \$\begingroup\$ the purpose is different, this function just launches for email message to compose in default editor set in git config! \$\endgroup\$ – Ciasto piekarz Feb 2 '14 at 12:10
3
\$\begingroup\$

1. Comments on your code

  1. 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 <br>. 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.

  2. There are missing imports (os and subprocess).

  3. 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.

  4. 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.)

  5. 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.

  6. 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:
    
  7. Specifying stdin=open('/dev/tty', 'r') is unnecessary. Let the editor decide how it wants to get input from the user.

  8. 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.)

  9. 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.

2. 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()
\$\endgroup\$
4
  • \$\begingroup\$ hello @Gareth Rees: this raises error, please see above I have edited and pasted the error. \$\endgroup\$ – Ciasto piekarz Feb 4 '14 at 17:04
  • \$\begingroup\$ Did you read the error message? It says "No such file or directory", so it looks as if the value of editor is wrong. \$\endgroup\$ – Gareth Rees Feb 4 '14 at 17:10
  • \$\begingroup\$ no, it is not which is why i printed the path of the editor and use os.path.existis that evaluates to True. \$\endgroup\$ – Ciasto piekarz Feb 4 '14 at 17:19
  • \$\begingroup\$ tried with your code again: and this time i got this error ` File "/Users/sanjeevkumar/Development/executables//git-ipush", line 79, in launchEditor with open(f.name) as g: IOError: [Errno 2] No such file or directory: '/var/folders/43/m1qv9zf53q19sqh6h9kg9pz80000gn/T/tmp2R8gs4'` \$\endgroup\$ – Ciasto piekarz Feb 4 '14 at 17:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.