Repository: google/yamlfmt (go)

Issue:
-lint always fails
I've just started using yamlfmt since v0.4.0, so I don't know how it worked before, but I think this issue was introduced in v0.4.0 because of the new diff library.

https://github.com/RageCage64/multilinediff seems to always return non-empty string but https://github.com/google/yamlfmt/blob/f3eff05a45d691309d88ccb76b1fc00bd3819a25/engine/engine.go#L95 detects lint errors by checking if a return value is empty or not. As a result, running yamlfmt with `-lint` always fails even if there's no diff.

Similarly, `-dry` always outputs all contents.

Pull request description:
feat: global linesep character, handle diffs
Closes #48 

Using the new `multilinediff` there needed to be two changes made:
* Needed to handle diffs using the count of differences found, since the new reporter always reports something
* Needed to recongize a line separation character at the engine level

This PR fixes the diff handling, which was relatively trivial. However, an oversight since the beginning is that diffs never took into account the line ending being used, and assumed `\n`. This needed to be rectified, and lead to something of a re-architecture that adds `line_ending` as a setting that acts as a global override. This required simplifying the `Factory` interface a little bit, in a way that sort of simplifies it. While refactoring anyway, I also decided to simplify the line break logic, so I didn't have to repeat the same constant check everywhere.

Patch:
diff --git a/README.md b/README.md
index d8feed2..e309eb5 100644
--- a/README.md
+++ b/README.md
@@ -45,6 +45,14 @@ exclude:
   - excluded/**/*.yaml
 ```
 
+### Line Ending
+
+The default line ending is `lf` (Unix style, Mac/Linux). The line ending can be changed to `crlf` (Windows style) with the `line_ending` setting:
+```yaml
+line_ending: crlf
+```
+This setting will be sent to any formatter as a config field called `line_ending`. If a `line_ending` is specified in the formatter, this will overwrite it. New formatters are free to ignore this setting if they don't need it, but any formatter provided by this repo will handle it accordingly.
+
 ### Formatter
 
 In your `.yamlfmt` file you can also specify configuration for the formatter if that formatter supports it. To change the indentation level of the basic formatter for example:
diff --git a/command/command.go b/command/command.go
index 9ab6fb3..06000ad 100644
--- a/command/command.go
+++ b/command/command.go
@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"runtime"
 
 	"github.com/google/yamlfmt"
 	"github.com/google/yamlfmt/engine"
@@ -40,9 +41,10 @@ type formatterConfig struct {
 }
 
 type commandConfig struct {
-	Include         []string         `mapstructure:"include"`
-	Exclude         []string         `mapstructure:"exclude"`
-	FormatterConfig *formatterConfig `mapstructure:"formatter,omitempty"`
+	Include         []string               `mapstructure:"include"`
+	Exclude         []string               `mapstructure:"exclude"`
+	LineEnding      yamlfmt.LineBreakStyle `mapstructure:"line_ending"`
+	FormatterConfig *formatterConfig       `mapstructure:"formatter,omitempty"`
 }
 
 func RunCommand(
@@ -58,6 +60,12 @@ func RunCommand(
 	if len(config.Include) == 0 {
 		config.Include = []string{"**/*.{yaml,yml}"}
 	}
+	if config.LineEnding == "" {
+		config.LineEnding = yamlfmt.LineBreakStyleLF
+		if runtime.GOOS == "windows" {
+			config.LineEnding = yamlfmt.LineBreakStyleCRLF
+		}
+	}
 
 	var formatter yamlfmt.Formatter
 	if config.FormatterConfig == nil {
@@ -65,7 +73,10 @@ func RunCommand(
 		if err != nil {
 			return err
 		}
-		formatter = factory.NewDefault()
+		formatter, err = factory.NewFormatter(nil)
+		if err != nil {
+			return err
+		}
 	} else {
 		var (
 			factory yamlfmt.Factory
@@ -80,20 +91,22 @@ func RunCommand(
 			return err
 		}
 
-		if len(config.FormatterConfig.FormatterSettings) > 0 {
-			formatter, err = factory.NewWithConfig(config.FormatterConfig.FormatterSettings)
-			if err != nil {
-				return err
-			}
-		} else {
-			formatter = factory.NewDefault()
+		config.FormatterConfig.FormatterSettings["line_ending"] = config.LineEnding
+		formatter, err = factory.NewFormatter(config.FormatterConfig.FormatterSettings)
+		if err != nil {
+			return err
 		}
 	}
 
+	lineSepChar, err := config.LineEnding.Separator()
+	if err != nil {
+		return err
+	}
 	engine := &engine.Engine{
-		Include:   config.Include,
-		Exclude:   config.Exclude,
-		Formatter: formatter,
+		Include:          config.Include,
+		Exclude:          config.Exclude,
+		LineSepCharacter: lineSepChar,
+		Formatter:        formatter,
 	}
 
 	switch operation {
diff --git a/engine/engine.go b/engine/engine.go
index d4a0722..f98d2fc 100644
--- a/engine/engine.go
+++ b/engine/engine.go
@@ -24,9 +24,10 @@ import (
 )
 
 type Engine struct {
-	Include   []string
-	Exclude   []string
-	Formatter yamlfmt.Formatter
+	Include          []string
+	Exclude          []string
+	LineSepCharacter string
+	Formatter        yamlfmt.Formatter
 }
 
 func (e *Engine) FormatAllFiles() error {
@@ -91,9 +92,9 @@ func (e *Engine) LintFile(path string) error {
 	if err != nil {
 		return err
 	}
-	diffContent := multilinediff.MultilineDiff(string(yamlBytes), string(formatted), "\n")
-	if diffContent != "" {
-		return fmt.Errorf(diffContent)
+	diff, diffCount := multilinediff.Diff(string(yamlBytes), string(formatted), e.LineSepCharacter)
+	if diffCount > 0 {
+		return fmt.Errorf(diff)
 	}
 	return nil
 }
@@ -107,10 +108,10 @@ func (e *Engine) DryRunAllFiles() (string, error) {
 	formatErrors := NewFormatFileErrors()
 	dryRunDiffs := NewDryRunDiffs()
 	for _, path := range paths {
-		diff, err := e.DryRunFile(path)
+		diff, diffCount, err := e.DryRunFile(path)
 		if err != nil {
 			formatErrors.Add(path, err)
-		} else if diff != "" {
+		} else if diffCount > 0 {
 			dryRunDiffs.Add(path, diff)
 		}
 	}
@@ -121,15 +122,15 @@ func (e *Engine) DryRunAllFiles() (string, error) {
 	return dryRunDiffs.CombineOutput(), nil
 }
 
-func (e *Engine) DryRunFile(path string) (string, error) {
+func (e *Engine) DryRunFile(path string) (string, int, error) {
 	yamlBytes, err := os.ReadFile(path)
 	if err != nil {
-		return "", err
+		return "", 0, err
 	}
 	formatted, err := e.Formatter.Format(yamlBytes)
 	if err != nil {
-		return "", err
+		return "", 0, err
 	}
-	diffContent := multilinediff.MultilineDiff(string(yamlBytes), string(formatted), "\n")
-	return diffContent, nil
+	diff, diffCount := multilinediff.Diff(string(yamlBytes), string(formatted), e.LineSepCharacter)
+	return diff, diffCount, nil
 }
diff --git a/formatters/basic/README.md b/formatters/basic/README.md
index 647cb22..f911524 100644
--- a/formatters/basic/README.md
+++ b/formatters/basic/README.md
@@ -8,6 +8,6 @@ The basic formatter is a barebones formatter that simply takes the data provided
 |:-------------------------|:---------------|:--------|:------------|
 | `indent`                 | int            | 2       | The indentation level in spaces to use for the formatted yaml|
 | `include_document_start` | bool           | false   | Include `---` at document start |
-| `line_ending`            | `lf` or `crlf` | `crlf` on Windows, `lf` otherwise | Parse and write the file with "lf" or "crlf" line endings |
+| `line_ending`            | `lf` or `crlf` | `crlf` on Windows, `lf` otherwise | Parse and write the file with "lf" or "crlf" line endings. This setting will be overwritten by the global `line_ending`. |
 | `emoji_support`          | bool           | false   | Support encoding utf-8 emojis |
 | `retain_line_breaks`     | bool           | false   | Retain line breaks in formatted yaml |
diff --git a/formatters/basic/config.go b/formatters/basic/config.go
index 3bcc602..6e3775b 100644
--- a/formatters/basic/config.go
+++ b/formatters/basic/config.go
@@ -21,11 +21,11 @@ import (
 )
 
 type Config struct {
-	Indent               int    `mapstructure:"indent"`
-	IncludeDocumentStart bool   `mapstructure:"include_document_start"`
-	EmojiSupport         bool   `mapstructure:"emoji_support"`
-	LineEnding           string `mapstructure:"line_ending"`
-	RetainLineBreaks     bool   `mapstructure:"retain_line_breaks"`
+	Indent               int                    `mapstructure:"indent"`
+	IncludeDocumentStart bool                   `mapstructure:"include_document_start"`
+	EmojiSupport         bool                   `mapstructure:"emoji_support"`
+	LineEnding           yamlfmt.LineBreakStyle `mapstructure:"line_ending"`
+	RetainLineBreaks     bool                   `mapstructure:"retain_line_breaks"`
 }
 
 func DefaultConfig() *Config {
diff --git a/formatters/basic/errors.go b/formatters/basic/errors.go
new file mode 100644
index 0000000..35f5bb8
--- /dev/null
+++ b/formatters/basic/errors.go
@@ -0,0 +1,19 @@
+package basic
+
+import "fmt"
+
+type BasicFormatterError struct {
+	err error
+}
+
+func (e BasicFormatterError) Error() string {
+	return fmt.Sprintf("basic formatter error: %v", e.err)
+}
+
+func (e BasicFormatterError) Unwrap() error {
+	return e.err
+}
+
+func wrapBasicFormatterError(err error) error {
+	return BasicFormatterError{err: err}
+}
diff --git a/formatters/basic/factory.go b/formatters/basic/factory.go
index c43248a..6655b77 100644
--- a/formatters/basic/factory.go
+++ b/formatters/basic/factory.go
@@ -25,15 +25,13 @@ func (f *BasicFormatterFactory) Type() string {
 	return BasicFormatterType
 }
 
-func (f *BasicFormatterFactory) NewDefault() yamlfmt.Formatter {
-	return newFormatter(DefaultConfig())
-}
-
-func (f *BasicFormatterFactory) NewWithConfig(configData map[string]interface{}) (yamlfmt.Formatter, error) {
+func (f *BasicFormatterFactory) NewFormatter(configData map[string]interface{}) (yamlfmt.Formatter, error) {
 	config := DefaultConfig()
-	err := mapstructure.Decode(configData, &config)
-	if err != nil {
-		return nil, err
+	if configData != nil {
+		err := mapstructure.Decode(configData, &config)
+		if err != nil {
+			return nil, err
+		}
 	}
 	return newFormatter(config), nil
 }
diff --git a/formatters/basic/features.go b/formatters/basic/features.go
index 252591b..3edf0d8 100644
--- a/formatters/basic/features.go
+++ b/formatters/basic/features.go
@@ -50,11 +50,11 @@ func ConfigureFeaturesFromConfig(config *Config) yamlfmt.FeatureList {
 		features = append(features, featCRLFSupport)
 	}
 	if config.RetainLineBreaks {
-		linebreakStr := "\n"
-		if config.LineEnding == yamlfmt.LineBreakStyleCRLF {
-			linebreakStr = "\r\n"
+		lineSep, err := config.LineEnding.Separator()
+		if err != nil {
+			lineSep = "\n"
 		}
-		featLineBreak := hotfix.MakeFeatureRetainLineBreak(linebreakStr)
+		featLineBreak := hotfix.MakeFeatureRetainLineBreak(lineSep)
 		features = append(features, featLineBreak)
 	}
 	return features
diff --git a/go.mod b/go.mod
index c65b65c..0cfe33b 100644
--- a/go.mod
+++ b/go.mod
@@ -4,7 +4,7 @@ go 1.19
 
 require (
 	github.com/RageCage64/go-utf8-codepoint-converter v0.1.0
-	github.com/RageCage64/multilinediff v0.1.0
+	github.com/RageCage64/multilinediff v0.2.0
 	github.com/bmatcuk/doublestar/v4 v4.2.0
 	github.com/mitchellh/mapstructure v1.5.0
 	gopkg.in/yaml.v3 v3.0.1
diff --git a/go.sum b/go.sum
index a52aca2..2b59ccc 100644
--- a/go.sum
+++ b/go.sum
@@ -2,6 +2,8 @@ github.com/RageCage64/go-utf8-codepoint-converter v0.1.0 h1:6GreQRSQApXW1sgeFXMB
 github.com/RageCage64/go-utf8-codepoint-converter v0.1.0/go.mod h1:asNWDxR7n0QIQyZNYTlpNk6Dg7GkUnxtCXho987uen8=
 github.com/RageCage64/multilinediff v0.1.0 h1:P9Iht5Vj4SHSmTJhQPJnySzTlX/cpL99kN3RJxejipQ=
 github.com/RageCage64/multilinediff v0.1.0/go.mod h1:pKr+KLgP0gvRzA+yv0/IUaYQuBYN1ucWysvsL58aMP0=
+github.com/RageCage64/multilinediff v0.2.0 h1:yNSpSF5NXIrmo6bRIgO4Q0g7SXqFD4j/WEcBE+BdCFY=
+github.com/RageCage64/multilinediff v0.2.0/go.mod h1:pKr+KLgP0gvRzA+yv0/IUaYQuBYN1ucWysvsL58aMP0=
 github.com/bmatcuk/doublestar/v4 v4.2.0 h1:Qu+u9wR3Vd89LnlLMHvnZ5coJMWKQamqdz9/p5GNthA=
 github.com/bmatcuk/doublestar/v4 v4.2.0/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc=
 github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
diff --git a/internal/hotfix/unicode.go b/internal/hotfix/unicode.go
index f219dda..49acdd2 100644
--- a/internal/hotfix/unicode.go
+++ b/internal/hotfix/unicode.go
@@ -24,7 +24,7 @@ import (
 // yamlfmt.FeatureFunc
 func ParseUnicodePoints(content []byte) ([]byte, error) {
 	if len(content) == 0 {
-		return []byte{}, errors.New("no content")
+		return []byte{}, nil
 	}
 
 	p := unicodeParser{
diff --git a/yamlfmt.go b/yamlfmt.go
index 75135ff..386e731 100644
--- a/yamlfmt.go
+++ b/yamlfmt.go
@@ -25,8 +25,7 @@ type Formatter interface {
 
 type Factory interface {
 	Type() string
-	NewDefault() Formatter
-	NewWithConfig(config map[string]interface{}) (Formatter, error)
+	NewFormatter(config map[string]interface{}) (Formatter, error)
 }
 
 type Registry struct {
@@ -63,11 +62,31 @@ func (r *Registry) GetDefaultFactory() (Factory, error) {
 	return factory, nil
 }
 
+type LineBreakStyle string
+
 const (
-	LineBreakStyleLF   = "lf"
-	LineBreakStyleCRLF = "crlf"
+	LineBreakStyleLF   LineBreakStyle = "lf"
+	LineBreakStyleCRLF LineBreakStyle = "crlf"
 )
 
+type UnsupportedLineBreakError struct {
+	style LineBreakStyle
+}
+
+func (e UnsupportedLineBreakError) Error() string {
+	return fmt.Sprintf("unsupported line break style %s, see package documentation for supported styles", e.style)
+}
+
+func (s LineBreakStyle) Separator() (string, error) {
+	switch s {
+	case LineBreakStyleLF:
+		return "\n", nil
+	case LineBreakStyleCRLF:
+		return "\r\n", nil
+	}
+	return "", UnsupportedLineBreakError{style: s}
+}
+
 type FeatureFunc func([]byte) ([]byte, error)
 
 type Feature struct {


Test patch:
diff --git a/formatters/basic/factory_test.go b/formatters/basic/factory_test.go
index 79db8bc..c8123cc 100644
--- a/formatters/basic/factory_test.go
+++ b/formatters/basic/factory_test.go
@@ -78,7 +78,7 @@ func TestNewWithConfigRetainsDefaultValues(t *testing.T) {
 	factory := basic.BasicFormatterFactory{}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			formatter, err := factory.NewWithConfig(tc.configMap)
+			formatter, err := factory.NewFormatter(tc.configMap)
 			if err != nil {
 				t.Fatalf("expected factory to create config, got error: %v", err)
 			}