Support editors with arguments, using shlex

[davidparsson]

Fixes #11.

[fmoo]

As-is, this breaks editors with spaces in their names or paths. What if we wrap the Popen call with try/catch (re-raising if it's not ENOENT or the editor has no spaces), and try other variations inside the catch?

[davidparsson]

@fmoo, is editors with (unescaped) spaces a thing that need to be supported? I don't think it should be, at least not on *nix.

Even Bash's CTRL+x, CTRL+e does not support EDITOR='some editor', but supports EDITOR='some\ editor'. The same goes for git. Or are there other cases where it makes sense to support this?

[eugene-eeo]

What about:

def edit(filename=None, args=(), contents=None, use_tty=None):
    editor = get_editor()
    args = [editor] + (args or get_editor_args(...))
    ...

We assume that if you're gonna pass in additional arguments args then you know what you're doing. This solves issues such as "Where do the additional arguments go".

[davidparsson]

Okay, let's make sure we don't misunderstand each other, because I see that #11 may be misinterpreted. When I opened #11 I did not mean to add support for binaries with spaces in their names, but to allow passing arguments in the $EDITOR variable. This conforms with how the $EDITOR variable is handled by other tools I've used, and has the (intentional) side-effect of breaking editors with (unescaped) spaces in their names.

But if you're still convinced that support for both cases are needed, despite it not being the "standard way", please let me know.

[eugene-eeo]

Ok that makes a ton of sense now. 👍 My bad for rushing into it; can you verify the behaviour of most tools when we pass in arguments to the $EDITOR variable? I.e. are there any additional arguments passed? Thanks.

[davidparsson]

Apart from bash and git (above), a few Googles found this:

• The GNU Emacs Manual mentions EDITOR="emacsclient --alternate-editor vi +%d %s"
• Sublime Text for OS X recommends export EDITOR='subl -w'

Bash, git and emacs are all pretty prominent software projects, but apart from that, if you're not convinced, I'm not really sure what more tools to test. What else uses $EDITOR?

[fmoo]

Yeah, I came across the Emacs one which imho is the most compelling argument for supporting this. The one thing I want to consider specially (and not break) is the "default args" behavior for various text editors (this is similar to what hg has implemented). It's not clear to me whether args in $EDITOR should override or augment the default args we provide though I am thinking augment. I'm curious about others' thoughts on this. If the current behavior of the two patches you sent David seem reasonable with this, I'd like to take both revs and do a major version bump to 2.0 since they're technically "breaking" API changes.

…

On Thu, Oct 19, 2017 at 7:20 AM, David Pärsson ***@***.***> wrote: Apart from bash and git (above), a few Googles found this: - The GNU Emacs Manual <http://ftp.gnu.org/old-gnu/Manuals/emacs-21.2/html_node/emacs_457.html> mentions EDITOR="emacsclient --alternate-editor vi +%d %s" - Sublime Text for OS X <https://www.sublimetext.com/docs/3/osx_command_line.html> recommends export EDITOR='subl -w' Bash, git and emacs are all pretty prominent software projects, but apart from that, if you're not convinced, I'm not really sure what more tools to test. What else uses $EDITOR? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIwkGyP480wXRCaRwiR73XBqpRbRl1Fks5st1rHgaJpZM4P6OHM> .

-- Peter Ruibal

[davidparsson]

I'm not sure what all current default arguments for all editors do, but I'm a big fan of sane defaults, so I think the augment approach seems like a reasonable way to go. Do you know how hg handles this case? Is it likely that any user would want to remove those default arguments?

I fully agree about the major version bump as well

[josh-gree]

Hi any updates on this issue? Would like to be able to open vscode which requires -w -n as args. Currently just hacked this by adding code with those args to the get_editor_args function...

    elif editor == "code":
        return ["-w", "-n"]

Don't really want to create a fork just to support this...

[davidparsson]

@josh-gree, my workaround for this was to create a scripts that opens the editor with arguments.

My /usr/bin/editor could contain something like:

#!/bin/sh
code -w -n $1

and then I've set $EDITOR to /usr/bin/editor.

Disclaimer: I've not tested this example, but you probably get it.

[fmoo]

@josh-gree if you put up a PR for get_editor_args defaults update for vscode, I'll take it.

[fmoo]

(I'd do a quick release as 1.0.4 and then look at pulling this change in as 2.0)

[josh-gree]

@fmoo PR added #26

[23Skidoo]

Any news on the 2.0 release?