Merge pull request #76 from zimmski/cleanup-ini

Cleanup ini

ini.go

@@ -3,11 +3,9 @@ package flags
 import (
 	"fmt"
 	"io"
-	"os"
 )
 
-// IniError contains location information on where in the ini file an error
-// occured.
+// IniError contains location information on where an error occured.
 type IniError struct {
 	// The error message.
 	Message string
@@ -21,37 +19,38 @@ type IniError struct {
 
 // Error provides a "file:line: message" formatted message of the ini error.
 func (x *IniError) Error() string {
-	return fmt.Sprintf("%s:%d: %s",
+	return fmt.Sprintf(
+		"%s:%d: %s",
 		x.File,
 		x.LineNumber,
-		x.Message)
+		x.Message,
+	)
 }
 
-// IniOptions for writing ini files
+// IniOptions for writing
 type IniOptions uint
 
 const (
 	// IniNone indicates no options.
 	IniNone IniOptions = 0
 
-	// IniIncludeDefaults indicates that default values should be written
-	// when writing options to an ini file.
+	// IniIncludeDefaults indicates that default values should be written.
 	IniIncludeDefaults = 1 << iota
 
 	// IniCommentDefaults indicates that if IniIncludeDefaults is used
 	// options with default values are written but commented out.
 	IniCommentDefaults
 
 	// IniIncludeComments indicates that comments containing the description
-	// of an option should be written when writing options to an ini file.
+	// of an option should be written.
 	IniIncludeComments
 
 	// IniDefault provides a default set of options.
 	IniDefault = IniIncludeComments
 )
 
 // IniParser is a utility to read and write flags options from and to ini
-// files.
+// formatted strings.
 type IniParser struct {
 	parser *Parser
 }
@@ -64,11 +63,12 @@ func NewIniParser(p *Parser) *IniParser {
 }
 
 // IniParse is a convenience function to parse command line options with default
-// settings from an ini file. The provided data is a pointer to a struct
+// settings from an ini formatted file. The provided data is a pointer to a struct
 // representing the default option group (named "Application Options"). For
 // more control, use flags.NewParser.
 func IniParse(filename string, data interface{}) error {
 	p := NewParser(data, Default)
+
 	return NewIniParser(p).ParseFile(filename)
 }
 
@@ -110,8 +110,7 @@ func (i *IniParser) ParseFile(filename string) error {
 // namespacing notation (i.e [subcommand.Options]). Group section names are
 // matched case insensitive.
 //
-// The returned errors can be of the type flags.Error or
-// flags.IniError.
+// The returned errors can be of the type flags.Error or flags.IniError.
 func (i *IniParser) Parse(reader io.Reader) error {
 	i.parser.storeDefaults()
 
@@ -128,16 +127,7 @@ func (i *IniParser) Parse(reader io.Reader) error {
 // for more information. The returned error occurs when the specified file
 // could not be opened for writing.
 func (i *IniParser) WriteFile(filename string, options IniOptions) error {
-	file, err := os.Create(filename)
-
-	if err != nil {
-		return err
-	}
-
-	defer file.Close()
-	i.Write(file, options)
-
-	return nil
+	return writeIniToFile(i, filename, options)
 }
 
 // Write writes the current values of all the flags to an ini format.

ini_private.go

@@ -163,6 +163,20 @@ func writeIni(parser *IniParser, writer io.Writer, options IniOptions) {
 	writeCommandIni(parser.parser.Command, "", writer, options)
 }
 
+func writeIniToFile(parser *IniParser, filename string, options IniOptions) error {
+	file, err := os.Create(filename)
+
+	if err != nil {
+		return err
+	}
+
+	defer file.Close()
+
+	writeIni(parser, file, options)
+
+	return nil
+}
+
 func readIniFromFile(filename string) (ini, error) {
 	file, err := os.Open(filename)
 
@@ -193,9 +207,7 @@ func readIni(contents io.Reader, filename string) (ini, error) {
 
 		if err == io.EOF {
 			break
-		}
-
-		if err != nil {
+		} else if err != nil {
 			return nil, err
 		}
 
@@ -289,8 +301,10 @@ func (i *IniParser) parse(ini ini) error {
 		groups := i.matchingGroups(name)
 
 		if len(groups) == 0 {
-			return newError(ErrUnknownGroup,
-				fmt.Sprintf("could not find option group `%s'", name))
+			return newError(
+				ErrUnknownGroup,
+				fmt.Sprintf("could not find option group `%s'", name),
+			)
 		}
 
 		for _, inival := range section {
@@ -312,8 +326,10 @@ func (i *IniParser) parse(ini ini) error {
 
 			if opt == nil {
 				if (p.Options & IgnoreUnknown) == None {
-					return newError(ErrUnknownFlag,
-						fmt.Sprintf("unknown option: %s", inival.Name))
+					return newError(
+						ErrUnknownFlag,
+						fmt.Sprintf("unknown option: %s", inival.Name),
+					)
 				}
 
 				continue

ini_test.go

@@ -2,6 +2,8 @@ package flags
 
 import (
 	"bytes"
+	"io/ioutil"
+	"os"
 	"strings"
 	"testing"
 )
@@ -300,3 +302,64 @@ value = some value
 
 	assertError(t, err, ErrUnknownFlag, "unknown option: value")
 }
+
+func TestIniParse(t *testing.T) {
+	file, err := ioutil.TempFile("", "")
+	if err != nil {
+		t.Fatalf("Cannot create temporary file: %s", err)
+	}
+	defer os.Remove(file.Name())
+
+	_, err = file.WriteString("value = 123")
+	if err != nil {
+		t.Fatalf("Cannot write to temporary file: %s", err)
+	}
+
+	file.Close()
+
+	var opts struct {
+		Value int `long:"value"`
+	}
+
+	err = IniParse(file.Name(), &opts)
+	if err != nil {
+		t.Fatalf("Could not parse ini: %s", err)
+	}
+
+	if opts.Value != 123 {
+		t.Fatalf("Expected Value to be \"123\" but was \"%d\"", opts.Value)
+	}
+}
+
+func TestWriteFile(t *testing.T) {
+	file, err := ioutil.TempFile("", "")
+	if err != nil {
+		t.Fatalf("Cannot create temporary file: %s", err)
+	}
+	defer os.Remove(file.Name())
+
+	var opts struct {
+		Value int `long:"value"`
+	}
+
+	opts.Value = 123
+
+	p := NewParser(&opts, Default)
+	ini := NewIniParser(p)
+
+	err = ini.WriteFile(file.Name(), IniIncludeDefaults)
+	if err != nil {
+		t.Fatalf("Could not write ini file: %s", err)
+	}
+
+	found, err := ioutil.ReadFile(file.Name())
+	if err != nil {
+		t.Fatalf("Could not read written ini file: %s", err)
+	}
+
+	expected := "[Application Options]\nValue = 123\n\n"
+
+	if string(found) != expected {
+		t.Fatalf("Expected file content to be \"%s\" but was \"%s\"", expected, found)
+	}
+}