From c585225eb3294bcdaa647fd72b39d061a1a0cbfb Mon Sep 17 00:00:00 2001 From: Luke Cowell Date: Mon, 3 Aug 2015 11:32:27 -0700 Subject: [PATCH 1/2] don't ignore errors generated in config.Update Update is not guaranteed to work, we should handle any returned errors. --- cmd/configure.go | 6 +++++- config/config.go | 10 ++++++++-- config/config_test.go | 9 ++++++--- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/cmd/configure.go b/cmd/configure.go index e48e92257..eefebebfd 100644 --- a/cmd/configure.go +++ b/cmd/configure.go @@ -22,7 +22,11 @@ func Configure(ctx *cli.Context) { host := ctx.String("host") dir := ctx.String("dir") api := ctx.String("api") - c.Update(key, host, dir, api) + + err = c.Update(key, host, dir, api) + if err != nil { + log.Fatalf("Error updating your configuration %s\n", err) + } if err := os.MkdirAll(c.Dir, os.ModePerm); err != nil { log.Fatalf("Error creating exercism directory %s\n", err) diff --git a/config/config.go b/config/config.go index 0fbe271a6..e98ae0258 100644 --- a/config/config.go +++ b/config/config.go @@ -76,7 +76,8 @@ func New(path string) (*Config, error) { } // Update sets new values where given. -func (c *Config) Update(key, host, dir, xapi string) { +func (c *Config) Update(key, host, dir, xapi string) error { + var err error key = strings.TrimSpace(key) if key != "" { c.APIKey = key @@ -89,13 +90,18 @@ func (c *Config) Update(key, host, dir, xapi string) { dir = strings.TrimSpace(dir) if dir != "" { - c.SetDir(dir) + err = c.SetDir(dir) + if err != nil { + return err + } } xapi = strings.TrimSpace(xapi) if xapi != "" { c.XAPI = xapi } + + return nil } // Write saves the config as JSON. diff --git a/config/config_test.go b/config/config_test.go index 23b2c95ef..9508be2b2 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -184,22 +184,25 @@ func TestUpdateConfig(t *testing.T) { } // Test the blank values don't overwrite existing values - c.Update("", "", "", "") + err = c.Update("", "", "", "") assert.Equal(t, "MyKey", c.APIKey) assert.Equal(t, "localhost", c.API) assert.Equal(t, "/exercism/directory", c.Dir) assert.Equal(t, "localhost", c.XAPI) + assert.NoError(t, err) // Test that each value can be overwritten - c.Update("NewKey", "http://example.com", "/tmp/exercism", "http://x.example.org") + err = c.Update("NewKey", "http://example.com", "/tmp/exercism", "http://x.example.org") assert.Equal(t, "NewKey", c.APIKey) assert.Equal(t, "http://example.com", c.API) assert.Equal(t, "/tmp/exercism", c.Dir) assert.Equal(t, "http://x.example.org", c.XAPI) + assert.NoError(t, err) // Test home is expanded on update - c.Update("", "", "~/myexercism", "") + err = c.Update("", "", "~/myexercism", "") assert.Equal(t, filepath.Join(tmpDir, "myexercism"), c.Dir) + assert.NoError(t, err) } func fixturePath(t *testing.T, filename string) string { From 8449f6a7cbaa2fdec35991f5ea29aaa9620031e9 Mon Sep 17 00:00:00 2001 From: Luke Cowell Date: Mon, 3 Aug 2015 11:34:02 -0700 Subject: [PATCH 2/2] prepend non-absolute paths with cwd --- cmd/configure.go | 3 +-- config/config.go | 26 ++++++++++++++++++++------ config/config_test.go | 5 ++++- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/cmd/configure.go b/cmd/configure.go index eefebebfd..00f07f938 100644 --- a/cmd/configure.go +++ b/cmd/configure.go @@ -23,8 +23,7 @@ func Configure(ctx *cli.Context) { dir := ctx.String("dir") api := ctx.String("api") - err = c.Update(key, host, dir, api) - if err != nil { + if err = c.Update(key, host, dir, api); err != nil { log.Fatalf("Error updating your configuration %s\n", err) } diff --git a/config/config.go b/config/config.go index e98ae0258..d42064833 100644 --- a/config/config.go +++ b/config/config.go @@ -77,7 +77,6 @@ func New(path string) (*Config, error) { // Update sets new values where given. func (c *Config) Update(key, host, dir, xapi string) error { - var err error key = strings.TrimSpace(key) if key != "" { c.APIKey = key @@ -90,8 +89,7 @@ func (c *Config) Update(key, host, dir, xapi string) error { dir = strings.TrimSpace(dir) if dir != "" { - err = c.SetDir(dir) - if err != nil { + if err := c.SetDir(dir); err != nil { return err } } @@ -259,13 +257,29 @@ func (c *Config) SetDir(path string) error { if err != nil { return err } + + var dir string + if path == "" { - c.Dir = filepath.Join(home, DirExercises) + dir = filepath.Join(home, DirExercises) } else { - c.Dir = path + dir = path } - c.Dir = expandHome(c.Dir, home) + dir = expandHome(dir, home) + + // if the user has provided us with a relative path, make it absolute so + // it will always work + if !filepath.IsAbs(dir) { + wd, err := os.Getwd() + if err != nil { + return err + } + dir = filepath.Join(wd, dir) + } + + c.Dir = dir + return nil } diff --git a/config/config_test.go b/config/config_test.go index 9508be2b2..d112f6403 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -12,6 +12,9 @@ import ( ) func TestSetDir(t *testing.T) { + dir, err := os.Getwd() + assert.NoError(t, err) + testCases := []struct { givenPath string expectedPath string @@ -20,7 +23,7 @@ func TestSetDir(t *testing.T) { {"~/foobar", "/test/home/foobar"}, {"/foobar/~/noexpand", "/foobar/~/noexpand"}, {"/no/modification", "/no/modification"}, - {"nomodification", "nomodification"}, + {"relativePath", filepath.Join(dir, "relativePath")}, } for _, testCase := range testCases {