From abc481260532f212bd869b992415f2f66fc5ddb0 Mon Sep 17 00:00:00 2001 From: Braydon Kains <93549768+braydonk@users.noreply.github.com> Date: Sat, 27 Aug 2022 14:17:22 -0400 Subject: [PATCH] fix: hotfix shim for CRLF behaviour (#36) * fix: hotfix shim for CRLF behaviour Added a configuration option for line endings, which is a feature that needs to exist anyway. Also included temporary hotfix behaviour for CRLF normalization while waiting for upstream fixes to get merged. * LineBreakStyle was useless, LineEndings rename was renamed to LineEnding to match the config key value. * remove accidental fmt println --- formatters/basic/README.md | 9 ++++---- formatters/basic/config.go | 18 ++++++++++++--- formatters/basic/factory_test.go | 27 ++++++++++++++++++++++ formatters/basic/formatter.go | 20 ++++++++++++++--- formatters/basic/formatter_test.go | 15 +++++++++++++ internal/hotfix/crlf.go | 22 ++++++++++++++++++ internal/hotfix/crlf_test.go | 36 ++++++++++++++++++++++++++++++ yamlfmt.go | 5 +++++ 8 files changed, 142 insertions(+), 10 deletions(-) create mode 100644 internal/hotfix/crlf.go create mode 100644 internal/hotfix/crlf_test.go diff --git a/formatters/basic/README.md b/formatters/basic/README.md index d61e450..62db5a4 100644 --- a/formatters/basic/README.md +++ b/formatters/basic/README.md @@ -4,7 +4,8 @@ The basic formatter is a barebones formatter that simply takes the data provided ## Configuration -| Key | Default | Description | -|:-------------------------|:--------|:------------| -| `indent` | 2 | The indentation level in spaces to use for the formatted yaml| -| `include_document_start` | false | Include `---` at document start | +| Key | Type | Default | Description | +|:-------------------------|:---------------|:--------|:------------| +| `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 | diff --git a/formatters/basic/config.go b/formatters/basic/config.go index 5f0aac7..5e267fd 100644 --- a/formatters/basic/config.go +++ b/formatters/basic/config.go @@ -14,13 +14,25 @@ package basic +import ( + "runtime" + + "github.com/google/yamlfmt" +) + type Config struct { - Indent int `mapstructure:"indent"` - IncludeDocumentStart bool `mapstructure:"include_document_start"` + Indent int `mapstructure:"indent"` + IncludeDocumentStart bool `mapstructure:"include_document_start"` + LineEnding string `mapstructure:"line_ending"` } func DefaultConfig() *Config { + lineBreakStyle := yamlfmt.LineBreakStyleLF + if runtime.GOOS == "windows" { + lineBreakStyle = yamlfmt.LineBreakStyleCRLF + } return &Config{ - Indent: 2, + Indent: 2, + LineEnding: lineBreakStyle, } } diff --git a/formatters/basic/factory_test.go b/formatters/basic/factory_test.go index a52fd7b..79db8bc 100644 --- a/formatters/basic/factory_test.go +++ b/formatters/basic/factory_test.go @@ -17,6 +17,7 @@ package basic_test import ( "testing" + "github.com/google/yamlfmt" "github.com/google/yamlfmt/formatters/basic" ) @@ -34,6 +35,7 @@ func TestNewWithConfigRetainsDefaultValues(t *testing.T) { expectedConfig: basic.Config{ Indent: 4, IncludeDocumentStart: false, + LineEnding: yamlfmt.LineBreakStyleLF, }, }, { @@ -44,6 +46,31 @@ func TestNewWithConfigRetainsDefaultValues(t *testing.T) { expectedConfig: basic.Config{ Indent: 2, IncludeDocumentStart: true, + LineEnding: yamlfmt.LineBreakStyleLF, + }, + }, + { + name: "only line_ending style specified", + configMap: map[string]interface{}{ + "line_ending": "crlf", + }, + expectedConfig: basic.Config{ + Indent: 2, + IncludeDocumentStart: false, + LineEnding: yamlfmt.LineBreakStyleCRLF, + }, + }, + { + name: "all specified", + configMap: map[string]interface{}{ + "indent": 4, + "line_ending": "crlf", + "include_document_start": true, + }, + expectedConfig: basic.Config{ + Indent: 4, + IncludeDocumentStart: true, + LineEnding: yamlfmt.LineBreakStyleCRLF, }, }, } diff --git a/formatters/basic/formatter.go b/formatters/basic/formatter.go index 632d72d..01944b8 100644 --- a/formatters/basic/formatter.go +++ b/formatters/basic/formatter.go @@ -19,6 +19,8 @@ import ( "errors" "io" + "github.com/google/yamlfmt" + "github.com/google/yamlfmt/internal/hotfix" "gopkg.in/yaml.v3" ) @@ -33,7 +35,15 @@ func (f *BasicFormatter) Type() string { } func (f *BasicFormatter) Format(yamlContent []byte) ([]byte, error) { - decoder := yaml.NewDecoder(bytes.NewReader(yamlContent)) + var reader *bytes.Reader + if f.Config.LineEnding == yamlfmt.LineBreakStyleCRLF { + crStrippedContent := hotfix.StripCRBytes(yamlContent) + reader = bytes.NewReader(crStrippedContent) + } else { + reader = bytes.NewReader(yamlContent) + } + + decoder := yaml.NewDecoder(reader) documents := []yaml.Node{} for { var docNode yaml.Node @@ -57,10 +67,14 @@ func (f *BasicFormatter) Format(yamlContent []byte) ([]byte, error) { } } + encodedContent := b.Bytes() if f.Config.IncludeDocumentStart { - return withDocumentStart(b.Bytes()), nil + encodedContent = withDocumentStart(b.Bytes()) + } + if f.Config.LineEnding == yamlfmt.LineBreakStyleCRLF { + return hotfix.WriteCRLFBytes(encodedContent), nil } - return b.Bytes(), nil + return encodedContent, nil } func withDocumentStart(document []byte) []byte { diff --git a/formatters/basic/formatter_test.go b/formatters/basic/formatter_test.go index e19b6ae..a2c1084 100644 --- a/formatters/basic/formatter_test.go +++ b/formatters/basic/formatter_test.go @@ -18,6 +18,7 @@ import ( "strings" "testing" + "github.com/google/yamlfmt" "github.com/google/yamlfmt/formatters/basic" ) @@ -83,3 +84,17 @@ func TestWithDocumentStart(t *testing.T) { t.Fatalf("expected document start to be included, result was: %s", string(s)) } } + +func TestCRLFLineEnding(t *testing.T) { + f := &basic.BasicFormatter{Config: basic.DefaultConfig()} + f.Config.LineEnding = yamlfmt.LineBreakStyleCRLF + + yaml := "# comment\r\na:\r\n" + result, err := f.Format([]byte(yaml)) + if err != nil { + t.Fatalf("expected formatting to pass, returned error: %v", err) + } + if string(yaml) != string(result) { + t.Fatalf("didn't write CRLF properly in result: %v", result) + } +} diff --git a/internal/hotfix/crlf.go b/internal/hotfix/crlf.go new file mode 100644 index 0000000..f8cba6e --- /dev/null +++ b/internal/hotfix/crlf.go @@ -0,0 +1,22 @@ +package hotfix + +func StripCRBytes(crlfContent []byte) []byte { + onlyLf := []byte{} + for _, b := range crlfContent { + if b != '\r' { + onlyLf = append(onlyLf, b) + } + } + return onlyLf +} + +func WriteCRLFBytes(lfContent []byte) []byte { + crlfContent := []byte{} + for _, b := range lfContent { + if b == '\n' { + crlfContent = append(crlfContent, '\r') + } + crlfContent = append(crlfContent, b) + } + return crlfContent +} diff --git a/internal/hotfix/crlf_test.go b/internal/hotfix/crlf_test.go new file mode 100644 index 0000000..ecb94f8 --- /dev/null +++ b/internal/hotfix/crlf_test.go @@ -0,0 +1,36 @@ +package hotfix_test + +import ( + "testing" + + "github.com/google/yamlfmt/internal/hotfix" +) + +func TestStripCRBytes(t *testing.T) { + crlfContent := []byte("two\r\nlines\r\n") + lfContent := hotfix.StripCRBytes(crlfContent) + count := countByte(lfContent, '\r') + if count != 0 { + t.Fatalf("Found %d CR (decimal 13) bytes in %v", count, lfContent) + } +} + +func TestWriteCRLF(t *testing.T) { + lfContent := []byte("two\nlines\n") + crlfContent := hotfix.WriteCRLFBytes(lfContent) + countCR := countByte(crlfContent, '\r') + countLF := countByte(crlfContent, '\n') + if countCR != countLF { + t.Fatalf("Found %d CR (decimal 13) and %d LF (decimal 10) bytes in %v", countCR, countLF, crlfContent) + } +} + +func countByte(content []byte, searchByte byte) int { + count := 0 + for _, b := range content { + if b == searchByte { + count++ + } + } + return count +} diff --git a/yamlfmt.go b/yamlfmt.go index e3e6260..14c3835 100644 --- a/yamlfmt.go +++ b/yamlfmt.go @@ -60,3 +60,8 @@ func (r *Registry) GetDefaultFactory() (Factory, error) { } return factory, nil } + +const ( + LineBreakStyleLF = "lf" + LineBreakStyleCRLF = "crlf" +)