Repository: google/yamlfmt (go)

Issue:
Make diff output more readable
Right now the `-dry` and `-lint` diff outputs are the direct result from `cmp.Diff`. This gets the message across but it's very hard to read. The most important first step would be removing all the string array display elements, and the double quotes in each line. It's not going to be trivial, since the yaml could contain double quotes, and I'm not entirely sure what happens in that scenario.

Pull request:
diff: improve diff readability
closes #7 

By implementing a custom diff reporter, this will improve diff
readability. It's still not perfect, but it's not extremely ugly like
before.

Patch:
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index 6d26aaf..9e9232b 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -20,7 +20,7 @@ jobs:
           cache: true
         
       - name: Install goimports
-        run: go install golang.org/x/tools/cmd/goimports
+        run: go install golang.org/x/tools/cmd/goimports@latest
 
       - name: Test
         run: go mod tidy
diff --git a/.goreleaser.yaml b/.goreleaser.yaml
index 034a410..4cd0cc5 100644
--- a/.goreleaser.yaml
+++ b/.goreleaser.yaml
@@ -1,3 +1,17 @@
+# Copyright 2022 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
 before:
   hooks:
     - go mod tidy
diff --git a/Makefile b/Makefile
index e063e39..5766603 100644
--- a/Makefile
+++ b/Makefile
@@ -8,4 +8,7 @@ install_tools:
 	go install github.com/google/addlicense@latest
 
 addlicense:
-	addlicense -c "Google LLC" -l apache .
\ No newline at end of file
+	addlicense -c "Google LLC" -l apache .
+
+test_diff:
+	go test -v -mod=mod github.com/google/yamlfmt/internal/diff
\ No newline at end of file
diff --git a/engine/engine.go b/engine/engine.go
index e6ba348..62d50c5 100644
--- a/engine/engine.go
+++ b/engine/engine.go
@@ -19,6 +19,7 @@ import (
 	"os"
 
 	"github.com/google/yamlfmt"
+	"github.com/google/yamlfmt/internal/diff"
 )
 
 type Engine struct {
@@ -89,9 +90,9 @@ func (e *Engine) LintFile(path string) error {
 	if err != nil {
 		return err
 	}
-	diff := MultilineStringDiff(string(yamlBytes), string(formatted))
-	if diff != "" {
-		return fmt.Errorf(diff)
+	diffContent := diff.MultilineStringDiff(string(yamlBytes), string(formatted))
+	if diffContent != "" {
+		return fmt.Errorf(diffContent)
 	}
 	return nil
 }
@@ -128,6 +129,6 @@ func (e *Engine) DryRunFile(path string) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	diff := MultilineStringDiff(string(yamlBytes), string(formatted))
-	return diff, nil
+	diffContent := diff.MultilineStringDiff(string(yamlBytes), string(formatted))
+	return diffContent, nil
 }
diff --git a/engine/errors.go b/engine/errors.go
index abc15c0..77123fa 100644
--- a/engine/errors.go
+++ b/engine/errors.go
@@ -55,7 +55,7 @@ func NewLintFileErrors() *LintFileErrors {
 func (e *LintFileErrors) Error() string {
 	errStr := "encountered the following linting errors:\n"
 	for file, err := range e.lintErrors {
-		errStr += fmt.Sprintf("%s:\n%v\n,", file, err)
+		errStr += fmt.Sprintf("%s:\n%v\n", file, err)
 	}
 	return errStr
 }
diff --git a/internal/diff/diff.go b/internal/diff/diff.go
new file mode 100644
index 0000000..a6ac577
--- /dev/null
+++ b/internal/diff/diff.go
@@ -0,0 +1,34 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package diff
+
+import (
+	"strings"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+)
+
+func MultilineStringDiff(a, b string) string {
+	var reporter prettyReporter
+	cmp.Diff(
+		a, b,
+		cmpopts.AcyclicTransformer("multiline", func(s string) []string {
+			return strings.Split(s, "\n")
+		}),
+		cmp.Reporter(&reporter),
+	)
+	return reporter.String()
+}
diff --git a/internal/diff/reporter.go b/internal/diff/reporter.go
new file mode 100644
index 0000000..95f1e73
--- /dev/null
+++ b/internal/diff/reporter.go
@@ -0,0 +1,58 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package diff
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+type prettyReporter struct {
+	path      cmp.Path
+	lines     []string
+	diffCount int
+}
+
+func (r *prettyReporter) PushStep(ps cmp.PathStep) {
+	r.path = append(r.path, ps)
+}
+
+func (r *prettyReporter) Report(rs cmp.Result) {
+	vx, vy := r.path.Last().Values()
+	if !rs.Equal() {
+		r.diffCount++
+		if vx.IsValid() {
+			r.lines = append(r.lines, fmt.Sprintf("- %+v", vx))
+		}
+		if vy.IsValid() {
+			r.lines = append(r.lines, fmt.Sprintf("+ %+v", vy))
+		}
+	} else {
+		r.lines = append(r.lines, fmt.Sprintf("  %+v", vx))
+	}
+}
+
+func (r *prettyReporter) PopStep() {
+	r.path = r.path[:len(r.path)-1]
+}
+
+func (r *prettyReporter) String() string {
+	if r.diffCount == 0 {
+		return ""
+	}
+	return strings.Join(r.lines, "\n")
+}


Test patch:
diff --git a/internal/diff/diff_test.go b/internal/diff/diff_test.go
new file mode 100644
index 0000000..5bf3516
--- /dev/null
+++ b/internal/diff/diff_test.go
@@ -0,0 +1,78 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package diff_test
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/google/yamlfmt/internal/diff"
+)
+
+func TestMultilineDiff(t *testing.T) {
+	testCases := []struct {
+		name   string
+		before string
+		after  string
+	}{
+		{
+			name: "multiple line diff",
+			before: `this
+is
+a
+first input`,
+			after: `this
+is
+the
+second input`,
+		},
+		{
+			name: "add blank line",
+			before: `foo
+bar`,
+			after: `foo
+
+bar`,
+		},
+		{
+			name: "remove blank line",
+			before: `foo
+			
+bar`,
+			after: `foo
+bar`,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			diffStr := diff.MultilineStringDiff(tc.before, tc.after)
+			if diffStr == "" {
+				t.Log("there should have been a diff")
+				t.Fail()
+			}
+			fmt.Println(diffStr)
+		})
+	}
+}
+
+func TestMultilineDiffNoDiff(t *testing.T) {
+	before := "content"
+
+	diffStr := diff.MultilineStringDiff(before, before)
+	if diffStr != "" {
+		t.Fatalf("diff output should be empty with no diff, output contained:\n%s", diffStr)
+	}
+}