From f984c7d858651323f14fd405f25406882cecb2ec Mon Sep 17 00:00:00 2001 From: yuki2006 Date: Sat, 4 Jun 2016 00:03:14 +0900 Subject: [PATCH 1/5] Distinguishes that there is an analysis error or there is not found app.conf. --- context.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/context.go b/context.go index ffe91ca..a2915b3 100644 --- a/context.go +++ b/context.go @@ -17,6 +17,8 @@ package config import ( "path" "strings" + "os" + "errors" ) // Context structure handles the parsing of app.conf @@ -36,10 +38,14 @@ func LoadContext(confName string, confPaths []string) (*Context, error) { var err error var conf *Config for _, confPath := range confPaths { - conf, err = ReadDefault(path.Join(confPath, confName)) + filePath := path.Join(confPath, confName) + conf, err = ReadDefault(filePath) if err == nil { return &Context{config: conf}, nil + } else if _, isPathErr := err.(*os.PathError); !isPathErr { + return nil, errors.New(filePath + " " + err.Error()) } + } return nil, err } @@ -134,8 +140,8 @@ func stripQuotes(s string) string { return s } - if s[0] == '"' && s[len(s)-1] == '"' { - return s[1 : len(s)-1] + if s[0] == '"' && s[len(s) - 1] == '"' { + return s[1 : len(s) - 1] } return s From 437ddfcfc9fbc67d1e70d22cd3a10af84271dde5 Mon Sep 17 00:00:00 2001 From: Jeevanandam M Date: Fri, 3 Jun 2016 14:38:19 -0700 Subject: [PATCH 2/5] load context method fix --- all_test.go | 74 ++++++++++++++++++++++++++++++++++++ context.go | 26 +++++++++---- read.go | 6 ++- testdata/app-invalid.conf | 17 +++++++++ testdata/conf-path1/app.conf | 16 ++++++++ testdata/conf-path2/app.conf | 18 +++++++++ 6 files changed, 148 insertions(+), 9 deletions(-) create mode 100644 testdata/app-invalid.conf create mode 100644 testdata/conf-path1/app.conf create mode 100644 testdata/conf-path2/app.conf diff --git a/all_test.go b/all_test.go index 571f99e..c02adf1 100644 --- a/all_test.go +++ b/all_test.go @@ -398,3 +398,77 @@ func TestMerge(t *testing.T) { t.Errorf("Expected '[X] x.four' to be 'x4' but instead it was '%s'", result) } } + +func TestLoadContextOneConf(t *testing.T) { + ctx, err := LoadContext("app.conf", []string{"testdata/conf-path1"}) + if err != nil { + t.Errorf("Error: %v", err) + t.FailNow() + } + + ctx.SetSection("X") + result, found := ctx.String("x.three") + if !strings.EqualFold("conf1-sourcex3", result) { + t.Errorf("Expected '[X] x.three' to be 'conf1-sourcex3' but instead it was '%s'", result) + } + + _, found = ctx.String("x.notexists") + if found { + t.Error("Config 'x.notexists' shouldn't found") + } + + ctx.SetSection("Y") + result, found = ctx.String("y.one") + if !strings.EqualFold("conf1-sourcey1", result) { + t.Errorf("Expected '[Y] y.one' to be 'conf1-sourcey1' but instead it was '%s'", result) + } + + _, found = ctx.String("y.notexists") + if found { + t.Error("Config 'y.notexists' shouldn't found") + } +} + +func TestLoadContextMultipleConfWithPriority(t *testing.T) { + ctx, err := LoadContext("app.conf", []string{"testdata/conf-path1", "testdata/conf-path2"}) + if err != nil { + t.Errorf("Error: %v", err) + t.FailNow() + } + + ctx.SetSection("X") + result, found := ctx.String("x.two") + if !strings.EqualFold("override-conf2-sourcex2", result) { + t.Errorf("Expected '[X] x.two' to be 'override-conf2-sourcex2' but instead it was '%s'", result) + } + + _, found = ctx.String("x.notexists") + if found { + t.Error("Config 'x.notexists' shouldn't be found") + } + + ctx.SetSection("Y") + result, found = ctx.String("y.three") + if !strings.EqualFold("override-conf2-sourcey3", result) { + t.Errorf("Expected '[Y] y.three' to be 'override-conf2-sourcey3' but instead it was '%s'", result) + } + + _, found = ctx.String("y.notexists") + if found { + t.Error("Config 'y.notexists' shouldn't be found") + } +} + +func TestLoadContextConfNotFound(t *testing.T) { + _, err := LoadContext("notfound.conf", []string{"testdata/conf-path1"}) + if err != nil && !strings.EqualFold("open testdata/conf-path1/notfound.conf: no such file or directory", err.Error()) { + t.Errorf("This is not expected error: %v", err) + } +} + +func TestLoadContextInvalidConf(t *testing.T) { + _, err := LoadContext("app-invalid.conf", []string{"testdata"}) + if err != nil && !strings.EqualFold("testdata/app-invalid.conf: could not parse line #7: %(two)s + %(four)s", err.Error()) { + t.Errorf("This is not expected error: %v", err) + } +} diff --git a/context.go b/context.go index ffe91ca..89cc7b1 100644 --- a/context.go +++ b/context.go @@ -15,7 +15,9 @@ package config import ( - "path" + "fmt" + "os" + "path/filepath" "strings" ) @@ -33,15 +35,25 @@ func NewContext() *Context { } func LoadContext(confName string, confPaths []string) (*Context, error) { - var err error - var conf *Config + ctx := NewContext() for _, confPath := range confPaths { - conf, err = ReadDefault(path.Join(confPath, confName)) - if err == nil { - return &Context{config: conf}, nil + filePath := filepath.Join(confPath, confName) + currentConf, err := ReadDefault(filePath) + if err != nil { + if _, isPathErr := err.(*os.PathError); isPathErr { + // TODO don't ignore config not found + // due exists flow, ignore is required else it breaks the App + // LoadContext needs revist + fmt.Println(err) + continue + } + + return nil, fmt.Errorf("%v: %v", filePath, err) } + + ctx.config.Merge(currentConf) } - return nil, err + return ctx, nil } func (c *Context) Raw() *Config { diff --git a/read.go b/read.go index 595d6d9..a1d5263 100644 --- a/read.go +++ b/read.go @@ -16,7 +16,7 @@ package config import ( "bufio" - "errors" + "fmt" "os" "strings" "unicode" @@ -58,8 +58,10 @@ func ReadDefault(fname string) (*Config, error) { func (c *Config) read(buf *bufio.Reader) (err error) { var section, option string var scanner = bufio.NewScanner(buf) + lineNo := 0 for scanner.Scan() { l := strings.TrimRightFunc(stripComments(scanner.Text()), unicode.IsSpace) + lineNo++ // Switch written for readability (not performance) switch { @@ -92,7 +94,7 @@ func (c *Config) read(buf *bufio.Reader) (err error) { c.AddOption(section, option, value) default: - return errors.New("could not parse line: " + l) + return fmt.Errorf("could not parse line #%v: %v", lineNo, l) } } } diff --git a/testdata/app-invalid.conf b/testdata/app-invalid.conf new file mode 100644 index 0000000..8774d35 --- /dev/null +++ b/testdata/app-invalid.conf @@ -0,0 +1,17 @@ +one=source1 +two=source2 +three=source3 +four=4 + +#invalid spot +%(two)s + %(four)s + +[X] +x.one=conf1-sourcex1 +x.two=conf1-sourcex2 +x.three=conf1-sourcex3 + +[Y] +y.one=conf1-sourcey1 +y.two=conf1-sourcey2 +y.three=conf1-sourcey3 diff --git a/testdata/conf-path1/app.conf b/testdata/conf-path1/app.conf new file mode 100644 index 0000000..d7027f2 --- /dev/null +++ b/testdata/conf-path1/app.conf @@ -0,0 +1,16 @@ +one=source1 +two=source2 +three=source3 +four=4 + +two_+_four=%(two)s + %(four)s + +[X] +x.one=conf1-sourcex1 +x.two=conf1-sourcex2 +x.three=conf1-sourcex3 + +[Y] +y.one=conf1-sourcey1 +y.two=conf1-sourcey2 +y.three=conf1-sourcey3 diff --git a/testdata/conf-path2/app.conf b/testdata/conf-path2/app.conf new file mode 100644 index 0000000..32561d4 --- /dev/null +++ b/testdata/conf-path2/app.conf @@ -0,0 +1,18 @@ +one=source1 +two=source2 +three=source3 +four=4 + +two_+_four=%(two)s + %(four)s + +[X] +x.one=conf2-sourcex1 +x.two=override-conf2-sourcex2 +x.three=conf2-sourcex3 + +x.four=exists here only + +[Y] +y.one=conf2-sourcey1 +y.two=conf2-sourcey2 +y.three=override-conf2-sourcey3 From 891a0e7652150dc481464ae51436b2c294d00ce4 Mon Sep 17 00:00:00 2001 From: yuki2006 Date: Sat, 4 Jun 2016 15:45:41 +0900 Subject: [PATCH 3/5] change path to file path --- context.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/context.go b/context.go index a2915b3..0af7350 100644 --- a/context.go +++ b/context.go @@ -15,10 +15,10 @@ package config import ( - "path" - "strings" - "os" "errors" + "os" + "path/filepath" + "strings" ) // Context structure handles the parsing of app.conf @@ -38,12 +38,12 @@ func LoadContext(confName string, confPaths []string) (*Context, error) { var err error var conf *Config for _, confPath := range confPaths { - filePath := path.Join(confPath, confName) - conf, err = ReadDefault(filePath) + path := filepath.Join(confPath, confName) + conf, err = ReadDefault(path) if err == nil { return &Context{config: conf}, nil } else if _, isPathErr := err.(*os.PathError); !isPathErr { - return nil, errors.New(filePath + " " + err.Error()) + return nil, errors.New(path + " " + err.Error()) } } @@ -140,8 +140,8 @@ func stripQuotes(s string) string { return s } - if s[0] == '"' && s[len(s) - 1] == '"' { - return s[1 : len(s) - 1] + if s[0] == '"' && s[len(s)-1] == '"' { + return s[1 : len(s)-1] } return s From 85a123061070899a82f59c5ef6187e8fb4457f64 Mon Sep 17 00:00:00 2001 From: Jeevanandam M Date: Sat, 4 Jun 2016 12:41:56 -0700 Subject: [PATCH 4/5] Added line no in ini file read error info --- read.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/read.go b/read.go index 595d6d9..a1d5263 100644 --- a/read.go +++ b/read.go @@ -16,7 +16,7 @@ package config import ( "bufio" - "errors" + "fmt" "os" "strings" "unicode" @@ -58,8 +58,10 @@ func ReadDefault(fname string) (*Config, error) { func (c *Config) read(buf *bufio.Reader) (err error) { var section, option string var scanner = bufio.NewScanner(buf) + lineNo := 0 for scanner.Scan() { l := strings.TrimRightFunc(stripComments(scanner.Text()), unicode.IsSpace) + lineNo++ // Switch written for readability (not performance) switch { @@ -92,7 +94,7 @@ func (c *Config) read(buf *bufio.Reader) (err error) { c.AddOption(section, option, value) default: - return errors.New("could not parse line: " + l) + return fmt.Errorf("could not parse line #%v: %v", lineNo, l) } } } From 6821dd3db5d28e764fec2a68c6aa569aaa96e77e Mon Sep 17 00:00:00 2001 From: Jeevanandam M Date: Sat, 4 Jun 2016 13:30:32 -0700 Subject: [PATCH 5/5] #4 improvements --- context.go | 1 + 1 file changed, 1 insertion(+) diff --git a/context.go b/context.go index 5cf74b2..8d7f57b 100644 --- a/context.go +++ b/context.go @@ -43,6 +43,7 @@ func LoadContext(confName string, confPaths []string) (*Context, error) { if _, isPathErr := err.(*os.PathError); !isPathErr { return nil, fmt.Errorf("%v: %v", path, err) } + continue } ctx.config.Merge(conf) }