Skip to content

Commit

Permalink
Merge branch 'shellquote'
Browse files Browse the repository at this point in the history
  • Loading branch information
rhysd committed Nov 14, 2018
2 parents aee1aaa + 03bd667 commit cba5295
Show file tree
Hide file tree
Showing 10 changed files with 155 additions and 76 deletions.
4 changes: 2 additions & 2 deletions cmd_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,14 @@ func (cmd *ConfigCmd) Do() error {
"HOME=%s\nGIT=%s\nEDITOR=%s\n",
cmd.Config.HomePath,
cmd.Config.GitPath,
cmd.Config.EditorPath,
cmd.Config.EditorCmd,
)
case "home":
fmt.Fprintln(cmd.Out, cmd.Config.HomePath)
case "git":
fmt.Fprintln(cmd.Out, cmd.Config.GitPath)
case "editor":
fmt.Fprintln(cmd.Out, cmd.Config.EditorPath)
fmt.Fprintln(cmd.Out, cmd.Config.EditorCmd)
default:
return errors.Errorf("Unknown config name '%s'", cmd.Name)
}
Expand Down
12 changes: 6 additions & 6 deletions cmd_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import (

func TestConfigCmd(t *testing.T) {
cfg := &Config{
HomePath: "/path/to/home",
GitPath: "/path/to/git",
EditorPath: "vim",
HomePath: "/path/to/home",
GitPath: "/path/to/git",
EditorCmd: "vim",
}
for _, tc := range []struct {
name string
Expand Down Expand Up @@ -57,9 +57,9 @@ func TestConfigCmd(t *testing.T) {

func TestConfigCmdError(t *testing.T) {
cfg := &Config{
HomePath: "/path/to/home",
GitPath: "/path/to/git",
EditorPath: "vim",
HomePath: "/path/to/home",
GitPath: "/path/to/git",
EditorCmd: "vim",
}
c := ConfigCmd{
Config: cfg,
Expand Down
4 changes: 3 additions & 1 deletion cmd_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"fmt"
"github.com/fatih/color"
"github.com/kballard/go-shellquote"
"github.com/rhysd/go-fakeio"
"os"
"os/exec"
Expand Down Expand Up @@ -660,9 +661,10 @@ func TestListCmdEditOption(t *testing.T) {

exe, err := exec.LookPath("echo")
panicIfErr(err)
exe = shellquote.Join(exe) // On Windows it may contain 'Program Files' so quoting is necessary

cfg := testNewConfigForListCmd("normal")
cfg.EditorPath = exe
cfg.EditorCmd = exe

var buf bytes.Buffer
cmd := &ListCmd{
Expand Down
17 changes: 8 additions & 9 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ type Config struct {
// Otherwise, `git` is used by default. This is optional and can be empty. When empty, some command
// and functionality which require Git don't work
GitPath string
// EditorPath is a file path to executable of your favorite editor. If $NOTES_CLI_EDITOR is set, it is used.
// Otherwise, this value will be empty. When empty, some functionality which requires an editor to open note
// doesn't work
EditorPath string
// EditorCmd is a command of your favorite editor. If $NOTES_CLI_EDITOR is set, it is used. This value is
// similar to $EDITOR environment variable and can contain command arguments like "vim -g". Otherwise,
// this value will be empty. When empty, some functionality which requires an editor to open note doesn't
// work
EditorCmd string
}

func homePath() (string, error) {
Expand Down Expand Up @@ -62,12 +63,10 @@ func gitPath() string {
return exe
}

func editorPath() string {
func editorCmd() string {
for _, key := range []string{"NOTES_CLI_EDITOR", "EDITOR"} {
if env := os.Getenv(key); env != "" {
if exe, err := exec.LookPath(env); err == nil {
return exe
}
return env
}
}
return ""
Expand All @@ -87,5 +86,5 @@ func NewConfig() (*Config, error) {
return nil, errors.Wrapf(err, "Could not create home '%s'", h)
}

return &Config{h, gitPath(), editorPath()}, nil
return &Config{h, gitPath(), editorCmd()}, nil
}
29 changes: 12 additions & 17 deletions config_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package notes

import (
"github.com/kballard/go-shellquote"
"github.com/rhysd/go-tmpenv"
"os"
"os/exec"
Expand Down Expand Up @@ -49,8 +50,8 @@ func TestNewDefaultConfig(t *testing.T) {
t.Fatal("Git path should not be detected:", c.GitPath)
}
}
if c.EditorPath != "" {
t.Fatal("Editor path should be empty by default:", c.EditorPath)
if c.EditorCmd != "" {
t.Fatal("Editor path should be empty by default:", c.EditorCmd)
}
}

Expand All @@ -59,11 +60,10 @@ func TestNewConfigCustomizeBinaryPaths(t *testing.T) {
defer func() { panicIfErr(g.Restore()) }()

ls, err := exec.LookPath("ls")
if err != nil {
panic(err)
}
panicIfErr(err)
qls := shellquote.Join(ls) // On Windows, it may contain 'Program Files'
os.Setenv("NOTES_CLI_GIT", ls)
os.Setenv("NOTES_CLI_EDITOR", ls)
os.Setenv("NOTES_CLI_EDITOR", qls)

c, err := NewConfig()
if err != nil {
Expand All @@ -74,20 +74,20 @@ func TestNewConfigCustomizeBinaryPaths(t *testing.T) {
t.Fatal("git path is unexpected:", c.GitPath, "wanted:", ls)
}

if c.EditorPath != ls {
t.Fatal("Editor is unexpected:", c.EditorPath, "wanted:", ls)
if c.EditorCmd != qls {
t.Fatal("Editor is unexpected:", c.EditorCmd, "wanted:", qls)
}

os.Unsetenv("NOTES_CLI_EDITOR")
os.Setenv("EDITOR", ls)
os.Setenv("EDITOR", qls)

c, err = NewConfig()
if err != nil {
t.Fatal(err)
}

if c.EditorPath != ls {
t.Fatal("Editor is unexpected:", c.EditorPath, "wanted:", ls)
if c.EditorCmd != qls {
t.Fatal("Editor is unexpected:", c.EditorCmd, "wanted:", qls)
}
}

Expand Down Expand Up @@ -145,12 +145,11 @@ func TestNewConfigCustomizeHome(t *testing.T) {
}
}

func TestNewConfigGitAndEditorNotFound(t *testing.T) {
func TestNewConfigGitNotFound(t *testing.T) {
g := testNewConfigEnvGuard()
defer func() { panicIfErr(g.Restore()) }()

panicIfErr(os.Setenv("NOTES_CLI_GIT", "/path/to/unknown-command"))
panicIfErr(os.Setenv("NOTES_CLI_EDITOR", "/path/to/unknown-command"))

c, err := NewConfig()
if err != nil {
Expand All @@ -160,8 +159,4 @@ func TestNewConfigGitAndEditorNotFound(t *testing.T) {
if c.GitPath != "" {
t.Fatal("git path should be empty:", c.GitPath)
}

if c.EditorPath != "" {
t.Fatal("editor path should be empty:", c.EditorPath)
}
}
23 changes: 18 additions & 5 deletions editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,30 @@ import (
"os"
"os/exec"

"github.com/kballard/go-shellquote"
"github.com/pkg/errors"
)

func openEditor(config *Config, args ...string) error {
if config.EditorPath == "" {
return errors.New("Editor is not set. To open note in editor, please set $NOTES_CLI_EDITOR")
func openEditor(config *Config, paths ...string) error {
if config.EditorCmd == "" {
return errors.New("Editor is not set. To open note in editor, please set $NOTES_CLI_EDITOR or $EDITOR")
}
c := exec.Command(config.EditorPath, args...)

cmdline, err := shellquote.Split(config.EditorCmd)
if err != nil {
return errors.Wrap(err, "Cannot parse editor command line. Please check $NOTES_CLI_EDITOR or $EDITOR")
}

editor := cmdline[0]
args := make([]string, 0, len(cmdline)-1+len(paths))
args = append(args, cmdline[1:]...)
args = append(args, paths...)

c := exec.Command(editor, args...)
c.Stdout = os.Stdout
c.Stderr = os.Stderr
c.Stdin = os.Stdin
c.Dir = config.HomePath
return errors.Wrap(c.Run(), "Editor command did not exit successfully")

return errors.Wrapf(c.Run(), "Editor command '%s' did not exit successfully", editor)
}
130 changes: 97 additions & 33 deletions editor_test.go
Original file line number Diff line number Diff line change
@@ -1,57 +1,121 @@
package notes

import (
"github.com/kballard/go-shellquote"
"github.com/rhysd/go-fakeio"
"os/exec"
"strings"
"testing"
)

func TestOpenEditor(t *testing.T) {
fake := fakeio.Stdout()
defer fake.Restore()

exe, err := exec.LookPath("echo")
echo, err := exec.LookPath("echo")
panicIfErr(err)

cfg := &Config{
EditorPath: exe,
HomePath: ".",
}
for _, tc := range []struct {
what string
editor string
want string
}{
{
what: "executable",
editor: "echo",
want: "foo bar\n",
},
{
what: "executable path",
editor: shellquote.Join(echo), // On Windows, path would contain 'Program Files'
want: "foo bar\n",
},
{
what: "multiple args",
editor: "echo 'aaa' bbb",
want: "aaa bbb foo bar\n",
},
{
what: "arg contains white space",
editor: "echo 'aaa bbb' 'ccc\tddd'",
want: "aaa bbb ccc\tddd foo bar\n",
},
{
what: "arg contains multiple white space",
editor: "echo aaa bbb ",
want: "aaa bbb foo bar\n",
},
} {
t.Run(tc.what, func(t *testing.T) {
fake := fakeio.Stdout()
defer fake.Restore()

if err := openEditor(cfg, "foo", "bar"); err != nil {
t.Fatal(err)
}
cfg := &Config{
EditorCmd: tc.editor,
HomePath: ".",
}

have, err := fake.String()
if err != nil {
panic(err)
}
if err := openEditor(cfg, "foo", "bar"); err != nil {
t.Fatal(err)
}

if have != "foo bar\n" {
t.Fatal("Arguments are unexpected:", have)
}
}
have, err := fake.String()
panicIfErr(err)

func TestEditorPathNotSet(t *testing.T) {
err := openEditor(&Config{})
if err == nil {
t.Fatal("Error did not occur")
}
if !strings.Contains(err.Error(), "Editor is not set") {
t.Fatal("Unexpected error:", err)
if have != tc.want {
t.Fatalf("Output from %q is unexpected. Wanted %q but have %q", tc.editor, tc.want, have)
}
})
}
}

func TestEditorExitError(t *testing.T) {
func TestEditorInvalidEditor(t *testing.T) {
exe, err := exec.LookPath("false")
panicIfErr(err)
exe = shellquote.Join(exe)

err = openEditor(&Config{EditorPath: exe})
if err == nil {
t.Fatal("Error did not occur")
}
if !strings.Contains(err.Error(), "Editor command did not exit successfully") {
t.Fatal("Unexpected error:", err)
for _, tc := range []struct {
what string
value string
want string
}{
{
what: "empty",
value: "",
want: "Editor is not set",
},
{
what: "empty command",
value: "''",
want: "did not exit successfully",
},
{
what: "unterminated single quote",
value: "vim '-g",
want: "Cannot parse editor command line",
},
{
what: "unterminated double quote",
value: "vim \"-g",
want: "Cannot parse editor command line",
},
{
what: "executable exit failure",
value: exe,
want: "did not exit successfully",
},
{
what: "executable does not exist",
value: "unknown-command-which-does-not-exist",
want: "did not exit successfully",
},
} {
t.Run(tc.want, func(t *testing.T) {
cfg := &Config{EditorCmd: tc.value}
err := openEditor(cfg)
if err == nil {
t.Fatal("Error did not occur")
}
if !strings.Contains(err.Error(), tc.want) {
t.Fatal("Unexpected error:", err)
}
})
}
}
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/fatih/color v1.7.0
github.com/google/go-cmp v0.2.0
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
github.com/mattn/go-colorable v0.0.9
github.com/mattn/go-isatty v0.0.4 // indirect
github.com/pkg/errors v0.8.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI=
github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=
github.com/inconshreveable/go-update v0.0.0-20160112193335-8152e7eb6ccf h1:WfD7VjIE6z8dIvMsI4/s+1qr5EL+zoIGev1BQj1eoJ8=
github.com/inconshreveable/go-update v0.0.0-20160112193335-8152e7eb6ccf/go.mod h1:hyb9oH7vZsitZCiBt0ZvifOrB+qc8PS5IiilCIb87rg=
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs=
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8=
github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
Expand Down
Loading

0 comments on commit cba5295

Please sign in to comment.