Skip to content

Commit d57cc1a

Browse files
authored
Refactor formatting adjustments (#81)
Instead of depending on a diff to restore newlines, this completely changes the algorithm to compare the input contents with the raw render YAML contents (before any mutations) to identify changes. We compute the places where newlines need to be injected, run Ratchet, and then insert those newlines back after modification. There are also now many more tests and a test harness that is more sustainable for adding tests and edge cases. This also introduces an intentionally-undocumented environment variable that dumps debugging information about whitespace computations when set. This will help with issue debugging. This results in a slight behavior change: YAML contents _will_ have consistently-formatted indentation now, but all newline whitespace changes are preserved, including block scalars. Fixes GH-80
1 parent 20b1e1a commit d57cc1a

23 files changed

+491
-598
lines changed

.github/workflows/release.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ jobs:
2121

2222
- uses: 'actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491' # ratchet:actions/setup-go@v5
2323
with:
24-
go-version: '1.21'
24+
go-version-file: 'go.mod'
2525

2626
- uses: 'docker/login-action@e92390c5fb421da1463c202d546fed0ec5c39f20' # ratchet:docker/login-action@v3
2727
with:

.github/workflows/test.yml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,13 @@ jobs:
2121

2222
- uses: 'actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491' # ratchet:actions/setup-go@v5
2323
with:
24-
go-version: '1.21'
24+
go-version-file: 'go.mod'
2525

2626
- name: 'Run tests'
2727
run: |-
28-
go test -count=1 -shuffle=on -timeout=10m -race ./...
28+
go test \
29+
-count=1 \
30+
-shuffle=on \
31+
-timeout=10m \
32+
-race \
33+
./...

command/check.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func (c *CheckCommand) Run(ctx context.Context, originalArgs []string) error {
6262

6363
fsys := os.DirFS(".")
6464

65-
files, err := loadYAMLFiles(fsys, args, false)
65+
files, err := loadYAMLFiles(fsys, args)
6666
if err != nil {
6767
return err
6868
}

command/command.go

Lines changed: 84 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,13 @@ import (
99
"fmt"
1010
"io/fs"
1111
"os"
12+
"slices"
13+
"strconv"
1214
"strings"
1315

1416
// Using banydonk/yaml instead of the default yaml pkg because the default
1517
// pkg incorrectly escapes unicode. https://github.com/go-yaml/yaml/issues/737
1618
"github.com/braydonk/yaml"
17-
"github.com/hexops/gotextdiff"
18-
"github.com/hexops/gotextdiff/myers"
19-
"github.com/hexops/gotextdiff/span"
2019
"github.com/sethvargo/ratchet/internal/version"
2120
)
2221

@@ -93,24 +92,48 @@ func extractCommandAndArgs(args []string) (string, []string) {
9392
}
9493

9594
// marshalYAML encodes the yaml node into the given writer.
96-
func marshalYAML(m *yaml.Node) ([]byte, error) {
95+
func marshalYAML(m *yaml.Node) (string, error) {
9796
var b bytes.Buffer
9897

9998
enc := yaml.NewEncoder(&b)
10099
enc.SetIndent(2)
100+
enc.SetAssumeBlockAsLiteral(true)
101101
if err := enc.Encode(m); err != nil {
102-
return nil, fmt.Errorf("failed to encode yaml: %w", err)
102+
return "", fmt.Errorf("failed to encode yaml: %w", err)
103103
}
104104
if err := enc.Close(); err != nil {
105-
return nil, fmt.Errorf("failed to finalize yaml: %w", err)
105+
return "", fmt.Errorf("failed to finalize yaml: %w", err)
106106
}
107-
return b.Bytes(), nil
107+
return b.String(), nil
108108
}
109109

110110
type loadResult struct {
111111
path string
112112
node *yaml.Node
113-
contents []byte
113+
contents string
114+
newlines []int
115+
}
116+
117+
func (r *loadResult) marshalYAML() (string, error) {
118+
contents, err := marshalYAML(r.node)
119+
if err != nil {
120+
return "", err
121+
}
122+
123+
// Restore newlines
124+
lines := strings.Split(contents, "\n")
125+
126+
for _, v := range r.newlines {
127+
lines = slices.Insert(lines, v, "")
128+
}
129+
130+
// Handle the edge case where a document starts with "---", which the Go YAML
131+
// parser discards.
132+
if strings.HasPrefix(strings.TrimSpace(r.contents), "---") && !strings.HasPrefix(contents, "---") {
133+
lines = slices.Insert(lines, 0, "---")
134+
}
135+
136+
return strings.Join(lines, "\n"), nil
114137
}
115138

116139
type loadResults []*loadResult
@@ -123,7 +146,7 @@ func (r loadResults) nodes() []*yaml.Node {
123146
return n
124147
}
125148

126-
func loadYAMLFiles(fsys fs.FS, paths []string, format bool) (loadResults, error) {
149+
func loadYAMLFiles(fsys fs.FS, paths []string) (loadResults, error) {
127150
r := make(loadResults, 0, len(paths))
128151

129152
for _, pth := range paths {
@@ -134,94 +157,75 @@ func loadYAMLFiles(fsys fs.FS, paths []string, format bool) (loadResults, error)
134157
}
135158

136159
var node yaml.Node
137-
if err := yaml.Unmarshal(contents, &node); err != nil {
160+
dec := yaml.NewDecoder(bytes.NewReader(contents))
161+
dec.SetScanBlockScalarAsLiteral(true)
162+
if err := dec.Decode(&node); err != nil {
138163
return nil, fmt.Errorf("failed to parse yaml for %s: %w", pth, err)
139164
}
140-
lr := &loadResult{
141-
path: pth,
142-
node: &node,
143-
contents: contents,
144-
}
145165

146-
if format {
147-
if err := fixIndentation(lr); err != nil {
148-
return nil, fmt.Errorf("failed to format indentation: %w", err)
149-
}
166+
// Remarshal the content before any modification so we can compute the
167+
// places where a newline should be inserted post-rendering.
168+
remarshaled, err := marshalYAML(&node)
169+
if err != nil {
170+
return nil, fmt.Errorf("failed to remarshal yaml for %s: %w", pth, err)
150171
}
151172

152-
r = append(r, lr)
173+
newlines := computeNewlineTargets(string(contents), remarshaled)
174+
175+
r = append(r, &loadResult{
176+
path: pth,
177+
node: &node,
178+
contents: string(contents),
179+
newlines: newlines,
180+
})
153181
}
154182

155183
return r, nil
156184
}
157185

158-
// fixIndentation corrects the indentation for the given loadResult and edits it in-place.
159-
func fixIndentation(f *loadResult) error {
160-
updated, err := marshalYAML(f.node)
161-
if err != nil {
162-
return fmt.Errorf("failed to marshal yaml for %s: %w", f.path, err)
163-
}
164-
lines := strings.Split(string(f.contents), "\n")
165-
afterLines := strings.Split(string(updated), "\n")
166-
167-
editedLines := []string{}
168-
afterIndex := 0
169-
// Loop through both lists line by line using a two-pointer technique.
170-
for _, l := range lines {
171-
token := strings.TrimSpace(l)
172-
if token == "" {
173-
editedLines = append(editedLines, l)
174-
continue
186+
func computeNewlineTargets(before, after string) []int {
187+
before = strings.TrimPrefix(before, "---\n")
188+
189+
debug, _ := strconv.ParseBool(os.Getenv("RATCHET_DEBUG_NEWLINE_PARSING"))
190+
if debug {
191+
fmt.Fprintf(os.Stderr, "\n")
192+
fmt.Fprintf(os.Stderr, "Original content:\n")
193+
for i, l := range strings.Split(string(before), "\n") {
194+
fmt.Fprintf(os.Stderr, "%3d: %s\n", i, l)
175195
}
176-
currentAfterLine := afterLines[afterIndex]
177-
indexInAfterLine := strings.Index(currentAfterLine, token)
178-
for indexInAfterLine == -1 {
179-
afterIndex++
180-
currentAfterLine = afterLines[afterIndex]
181-
indexInAfterLine = strings.Index(currentAfterLine, token)
196+
fmt.Fprintf(os.Stderr, "\n")
197+
fmt.Fprintf(os.Stderr, "Rendered content:\n")
198+
for i, l := range strings.Split(after, "\n") {
199+
fmt.Fprintf(os.Stderr, "%3d: %s\n", i, l)
182200
}
183-
184-
lineWithCorrectIndent := currentAfterLine[:indexInAfterLine] + token
185-
editedLines = append(editedLines, lineWithCorrectIndent)
186-
afterIndex++
201+
fmt.Fprintf(os.Stderr, "\n")
187202
}
188203

189-
f.contents = []byte(strings.Join(editedLines, "\n"))
190-
return nil
191-
}
204+
result := make([]int, 0, 8)
205+
afteri, afterLines := 0, strings.Split(after, "\n")
206+
beforeLines := strings.Split(before, "\n")
192207

193-
func removeNewLineChanges(beforeContent, afterContent string) string {
194-
lines := strings.Split(beforeContent, "\n")
195-
edits := myers.ComputeEdits(span.URIFromPath("before.txt"), beforeContent, afterContent)
196-
unified := gotextdiff.ToUnified("before.txt", "after.txt", beforeContent, edits)
197-
198-
editedLines := make(map[int]string)
199-
// Iterates through all changes and only keep changes to lines that are not empty.
200-
for _, h := range unified.Hunks {
201-
// Changes are in-order of delete line followed by insert line for lines that were modified.
202-
// We want to locate the position of all deletes of non-empty lines and replace
203-
// these in the original content with the modified line.
204-
var deletePositions []int
205-
inserts := 0
206-
for i, l := range h.Lines {
207-
if l.Kind == gotextdiff.Delete && l.Content != "\n" {
208-
deletePositions = append(deletePositions, h.FromLine+i-1-inserts)
209-
}
210-
if l.Kind == gotextdiff.Insert && l.Content != "" {
211-
pos := deletePositions[0]
212-
deletePositions = deletePositions[1:]
213-
editedLines[pos] = strings.TrimSuffix(l.Content, "\n")
214-
inserts++
215-
}
208+
for beforei := 0; beforei < len(beforeLines); beforei++ {
209+
if afteri >= len(afterLines) {
210+
result = append(result, beforei)
211+
continue
216212
}
217-
}
218-
var formattedLines []string
219-
for i, line := range lines {
220-
if editedLine, ok := editedLines[i]; ok {
221-
formattedLines = append(formattedLines, editedLine)
213+
214+
beforeLine := strings.TrimSpace(beforeLines[beforei])
215+
afterLine := strings.TrimSpace(afterLines[afteri])
216+
217+
if beforeLine != afterLine && beforeLine == "" {
218+
result = append(result, beforei)
222219
} else {
223-
formattedLines = append(formattedLines, line)
220+
afteri++
224221
}
225222
}
226-
return strings.Join(formattedLines, "\n")
223+
224+
if debug {
225+
fmt.Fprintf(os.Stderr, "\n")
226+
fmt.Fprintf(os.Stderr, "newline indicies: %v\n", result)
227+
fmt.Fprintf(os.Stderr, "\n")
228+
}
229+
230+
return result
227231
}

0 commit comments

Comments
 (0)