command: correctly propogate line_ending setting

The line_ending setting from the formatter was accidentally being
ignored in favour of only checking the top level. This PR refactors the
code while taking this edge case into account.

command/command.go

@@ -60,35 +60,9 @@ type Command struct {
 }
 
 func (c *Command) Run() error {
-	var formatter yamlfmt.Formatter
-	if c.Config.FormatterConfig == nil {
-		factory, err := c.Registry.GetDefaultFactory()
-		if err != nil {
-			return err
-		}
-		formatter, err = factory.NewFormatter(nil)
-		if err != nil {
-			return err
-		}
-	} else {
-		var (
-			factory yamlfmt.Factory
-			err     error
-		)
-		if c.Config.FormatterConfig.Type == "" {
-			factory, err = c.Registry.GetDefaultFactory()
-		} else {
-			factory, err = c.Registry.GetFactory(c.Config.FormatterConfig.Type)
-		}
-		if err != nil {
-			return err
-		}
-
-		c.Config.FormatterConfig.FormatterSettings["line_ending"] = c.Config.LineEnding
-		formatter, err = factory.NewFormatter(c.Config.FormatterConfig.FormatterSettings)
-		if err != nil {
-			return err
-		}
+	formatter, err := c.getFormatter()
+	if err != nil {
+		return err
 	}
 
 	lineSepChar, err := c.Config.LineEnding.Separator()
@@ -191,6 +165,30 @@ func (c *Command) Run() error {
 	return nil
 }
 
+func (c *Command) getFormatter() (yamlfmt.Formatter, error) {
+	var factoryType string
+
+	// In the existing codepaths, this value is always set. But
+	// it's a habit of mine to check anything that can possibly be nil
+	// if I remember that to be the case. :)
+	if c.Config.FormatterConfig != nil {
+		factoryType = c.Config.FormatterConfig.Type
+
+		// The line ending set within the formatter settings takes precedence over setting
+		// it from the top level config. If it's not set in formatter settings, then
+		// we use the value from the top level.
+		if _, ok := c.Config.FormatterConfig.FormatterSettings["line_ending"]; !ok {
+			c.Config.FormatterConfig.FormatterSettings["line_ending"] = c.Config.LineEnding
+		}
+	}
+
+	factory, err := c.Registry.GetFactory(factoryType)
+	if err != nil {
+		return nil, err
+	}
+	return factory.NewFormatter(c.Config.FormatterConfig.FormatterSettings)
+}
+
 func (c *Command) collectPaths() ([]string, error) {
 	collector := c.makePathCollector()
 	return collector.CollectPaths()

command/command_test.go

@@ -0,0 +1,32 @@
+package command
+
+import (
+	"testing"
+
+	"github.com/google/yamlfmt"
+	"github.com/google/yamlfmt/formatters/basic"
+	"github.com/google/yamlfmt/internal/assert"
+)
+
+// This test asserts the proper behaviour for `line_ending` settings specified
+// in formatter settings overriding the global configuration.
+func TestLineEndingFormatterVsGloabl(t *testing.T) {
+	c := &Command{
+		Config: &Config{
+			LineEnding: "lf",
+			FormatterConfig: &FormatterConfig{
+				FormatterSettings: map[string]any{
+					"line_ending": yamlfmt.LineBreakStyleLF,
+				},
+			},
+		},
+		Registry: yamlfmt.NewFormatterRegistry(&basic.BasicFormatterFactory{}),
+	}
+
+	f, err := c.getFormatter()
+	assert.NilErr(t, err)
+	configMap, err := f.ConfigMap()
+	assert.NilErr(t, err)
+	formatterLineEnding := configMap["line_ending"].(yamlfmt.LineBreakStyle)
+	assert.Assert(t, formatterLineEnding == yamlfmt.LineBreakStyleLF, "expected formatter's line ending to be lf")
+}

formatter.go

@@ -46,6 +46,9 @@ func (r *Registry) Add(f Factory) {
 }
 
 func (r *Registry) GetFactory(fType string) (Factory, error) {
+	if fType == "" {
+		return r.GetDefaultFactory()
+	}
 	factory, ok := r.registry[fType]
 	if !ok {
 		return nil, fmt.Errorf("no formatter registered with type \"%s\"", fType)

formatters/basic/formatter.go

@@ -17,6 +17,7 @@ package basic
 import (
 	"bytes"
 	"errors"
+	"fmt"
 	"io"
 
 	"github.com/braydonk/yaml"
@@ -102,6 +103,7 @@ func (f *BasicFormatter) getNewEncoder(buf *bytes.Buffer) *yaml.Encoder {
 		e.SetWidth(f.Config.LineLength)
 	}
 
+	fmt.Println(f.Config.LineEnding)
 	if f.Config.LineEnding == yamlfmt.LineBreakStyleCRLF {
 		e.SetLineBreakStyle(yaml.LineBreakStyleCRLF)
 	}

formatters/basic/formatter_test.go

@@ -15,6 +15,7 @@
 package basic_test
 
 import (
+	"runtime"
 	"strings"
 	"testing"
 
@@ -351,3 +352,27 @@ b: 2
 		t.Fatalf("expected: '%s', got: '%s'", expectedYml, resultStr)
 	}
 }
+
+func TestWindowsEOFNewlineCRLFBug(t *testing.T) {
+	if runtime.GOOS != "windows" {
+		t.Skip("this test is Windows specific")
+	}
+
+	config := basic.DefaultConfig()
+	config.RetainLineBreaks = false
+	config.EOFNewline = true
+	config.LineEnding = yamlfmt.LineBreakStyleLF
+	f := newFormatter(config)
+
+	yml := `a: 1
+b: 2`
+
+	result, err := f.Format([]byte(yml))
+	if err != nil {
+		t.Fatalf("expected formatting to pass, returned error: %v", err)
+	}
+	resultStr := string(result)
+	if strings.Contains(resultStr, "\r") {
+		t.Fatalf("expected: no CRLF, found it")
+	}
+}