Skip to content

Commit

Permalink
[cmd] Stop the surprising behavior of shell-splitting in cmd.New()
Browse files Browse the repository at this point in the history
`cmd.New(programWithSpaces)` will now work for executables that have
spaces in the path. The only place where we relied on shell-splitting
was in openTextEditor, which now performs its own splitting.
  • Loading branch information
mislav committed Jun 28, 2019
1 parent 6a13adf commit 312ccd8
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 14 deletions.
18 changes: 8 additions & 10 deletions cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import (
"syscall"

"github.com/github/hub/ui"
"github.com/github/hub/utils"
"github.com/kballard/go-shellquote"
)

type Cmd struct {
Expand Down Expand Up @@ -123,14 +121,14 @@ func (cmd *Cmd) Exec() error {
return syscall.Exec(binary, args, os.Environ())
}

func New(cmd string) *Cmd {
cmds, err := shellquote.Split(cmd)
utils.Check(err)

name := cmds[0]
args := cmds[1:]

return &Cmd{Name: name, Args: args, Stdin: os.Stdin, Stdout: os.Stdout, Stderr: os.Stderr}
func New(name string) *Cmd {
return &Cmd{
Name: name,
Args: []string{},
Stdin: os.Stdin,
Stdout: os.Stdout,
Stderr: os.Stderr,
}
}

func NewWithArray(cmd []string) *Cmd {
Expand Down
5 changes: 2 additions & 3 deletions cmd/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ import (

func TestNew(t *testing.T) {
execCmd := New("vim --noplugin")
assert.Equal(t, "vim", execCmd.Name)
assert.Equal(t, 1, len(execCmd.Args))
assert.Equal(t, "--noplugin", execCmd.Args[0])
assert.Equal(t, "vim --noplugin", execCmd.Name)
assert.Equal(t, 0, len(execCmd.Args))
}

func TestWithArg(t *testing.T) {
Expand Down
7 changes: 6 additions & 1 deletion github/editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/github/hub/cmd"
"github.com/github/hub/git"
"github.com/kballard/go-shellquote"
)

const Scissors = "------------------------ >8 ------------------------"
Expand Down Expand Up @@ -137,7 +138,11 @@ func (e *Editor) readContent() (content []byte, err error) {
}

func openTextEditor(program, file string) error {
editCmd := cmd.New(program)
programArgs, err := shellquote.Split(program)
if err != nil {
return err
}
editCmd := cmd.NewWithArray(programArgs)
r := regexp.MustCompile(`\b(?:[gm]?vim)(?:\.exe)?$`)
if r.MatchString(editCmd.Name) {
editCmd.WithArg("--cmd")
Expand Down

0 comments on commit 312ccd8

Please sign in to comment.