From 84646f6bec98ff723876dbe64f597516f8df2731 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Wed, 2 Oct 2024 21:03:31 +0200 Subject: [PATCH] internal/diff: drop last uses of cue.Value.Struct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was still used as part of the public EditScript API; it always referenced struct fields or list elements by index, which is OK for lists, but not a great mechanism for structs. Lean into cue.Selector instead, which works well for structs (regular fields, optional fields, definitions, etc) as well as for lists via index selectors. This also means we can massively simplify the API surface, given that the selector for each side of an edit already describes which field or element is being edited, and it can be used to obtain the edited value on either side via cue.Value.LookupPath. Note that the simplified API surface exposes fields directly in EditScript and Edit rather than using getter methods. This seems perfectly fine given that this is an internal API which is only used by the diff printer in the same package. In fact, the printer did not bother to use the getter methods anyway. Signed-off-by: Daniel Martí Change-Id: I964bbbb97b55025d0e7c911207034f0fc39976a7 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202111 Unity-Result: CUE porcuepine TryBot-Result: CUEcueckoo Reviewed-by: Roger Peppe --- internal/diff/diff.go | 103 +++++++---------------------------------- internal/diff/print.go | 70 ++++++++++++++-------------- 2 files changed, 51 insertions(+), 122 deletions(-) diff --git a/internal/diff/diff.go b/internal/diff/diff.go index f39ef7dfe85..d3e5110217d 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -58,7 +58,7 @@ func (p *Profile) Diff(x, y cue.Value) (Kind, *EditScript) { d := differ{cfg: *p} k, es := d.diffValue(x, y) if k == Modified && es == nil { - es = &EditScript{x: x, y: y} + es = &EditScript{X: x, Y: y} } return k, es } @@ -80,89 +80,18 @@ const ( // EditScript represents the series of differences between two CUE values. // x and y must be either both list or struct. type EditScript struct { - x, y cue.Value - edits []Edit -} - -// Len returns the number of edits. -func (es *EditScript) Len() int { - return len(es.edits) -} - -// Label returns a string representation of the label. -func (es *EditScript) LabelX(i int) string { - e := es.edits[i] - p := e.XPos() - if p < 0 { - return "" - } - return label(es.x, p) -} - -func (es *EditScript) LabelY(i int) string { - e := es.edits[i] - p := e.YPos() - if p < 0 { - return "" - } - return label(es.y, p) -} - -// TODO: support label expressions. -func label(v cue.Value, i int) string { - st, err := v.Struct() - if err != nil { - return "" - } - - // TODO: return formatted expression for optionals. - f := st.Field(i) - str := f.Selector - if f.IsOptional { - str += "?" - } - str += ":" - return str -} - -// ValueX returns the value of X involved at step i. -func (es *EditScript) ValueX(i int) (v cue.Value) { - p := es.edits[i].XPos() - if p < 0 { - return v - } - st, err := es.x.Struct() - if err != nil { - return v - } - return st.Field(p).Value -} - -// ValueY returns the value of Y involved at step i. -func (es *EditScript) ValueY(i int) (v cue.Value) { - p := es.edits[i].YPos() - if p < 0 { - return v - } - st, err := es.y.Struct() - if err != nil { - return v - } - return st.Field(p).Value + X, Y cue.Value + Edits []Edit } // Edit represents a single operation within an edit-script. type Edit struct { - kind Kind - xPos int32 // 0 if UniqueY - yPos int32 // 0 if UniqueX - sub *EditScript // non-nil if Modified + Kind Kind + XSel cue.Selector // valid if UniqueY + YSel cue.Selector // valid if UniqueX + Sub *EditScript // non-nil if Modified } -func (e Edit) Kind() Kind { return e.kind } -func (e Edit) XPos() int { return int(e.xPos - 1) } -func (e Edit) YPos() int { return int(e.yPos - 1) } - type differ struct { cfg Profile } @@ -249,7 +178,7 @@ func (d *differ) diffStruct(x, y cue.Value) (Kind, *EditScript) { if yp > 0 { break } - edits = append(edits, Edit{UniqueX, int32(xi + 1), 0, nil}) + edits = append(edits, Edit{UniqueX, xf.sel, cue.Selector{}, nil}) differs = true } for ; yi < len(yFields); yi++ { @@ -262,7 +191,7 @@ func (d *differ) diffStruct(x, y cue.Value) (Kind, *EditScript) { break } yMap[yf.sel] = 0 - edits = append(edits, Edit{UniqueY, 0, int32(yi + 1), nil}) + edits = append(edits, Edit{UniqueY, cue.Selector{}, yf.sel, nil}) differs = true } @@ -288,14 +217,14 @@ func (d *differ) diffStruct(x, y cue.Value) (Kind, *EditScript) { kind, script = d.diffValue(xf.val, yf.val) } - edits = append(edits, Edit{kind, int32(xi + 1), int32(yp), script}) + edits = append(edits, Edit{kind, xf.sel, yf.sel, script}) differs = differs || kind != Identity } } if !differs { return Identity, nil } - return Modified, &EditScript{x: x, y: y, edits: edits} + return Modified, &EditScript{X: x, Y: y, Edits: edits} } // TODO: right now we do a simple element-by-element comparison. Instead, @@ -307,7 +236,7 @@ func (d *differ) diffList(x, y cue.Value) (Kind, *EditScript) { edits := []Edit{} differs := false - i := int32(1) + i := 0 for { // TODO: This would be much easier with a Next/Done API. @@ -316,7 +245,7 @@ func (d *differ) diffList(x, y cue.Value) (Kind, *EditScript) { if !hasX { for hasY { differs = true - edits = append(edits, Edit{UniqueY, 0, i, nil}) + edits = append(edits, Edit{UniqueY, cue.Selector{}, cue.Index(i), nil}) hasY = iy.Next() i++ } @@ -325,7 +254,7 @@ func (d *differ) diffList(x, y cue.Value) (Kind, *EditScript) { if !hasY { for hasX { differs = true - edits = append(edits, Edit{UniqueX, i, 0, nil}) + edits = append(edits, Edit{UniqueX, cue.Index(i), cue.Selector{}, nil}) hasX = ix.Next() i++ } @@ -337,11 +266,11 @@ func (d *differ) diffList(x, y cue.Value) (Kind, *EditScript) { if kind != Identity { differs = true } - edits = append(edits, Edit{kind, i, i, script}) + edits = append(edits, Edit{kind, cue.Index(i), cue.Index(i), script}) i++ } if !differs { return Identity, nil } - return Modified, &EditScript{x: x, y: y, edits: edits} + return Modified, &EditScript{X: x, Y: y, Edits: edits} } diff --git a/internal/diff/print.go b/internal/diff/print.go index 5023839f2f9..4f6cc646137 100644 --- a/internal/diff/print.go +++ b/internal/diff/print.go @@ -84,29 +84,29 @@ func (p *printer) printf(format string, args ...interface{}) { } func (p *printer) script(e *EditScript) { - switch e.x.Kind() { + switch e.X.Kind() { case cue.StructKind: p.printStruct(e) case cue.ListKind: p.printList(e) default: - p.printElem("-", e.x) - p.printElem("+", e.y) + p.printElem("-", e.X) + p.printElem("+", e.Y) } } func (p *printer) findRun(es *EditScript, i int) (start, end int) { lastEnd := i - for ; i < es.Len() && es.edits[i].kind == Identity; i++ { + for ; i < len(es.Edits) && es.Edits[i].Kind == Identity; i++ { } start = i // Find end of run include := p.context - for ; i < es.Len(); i++ { - e := es.edits[i] - if e.kind != Identity { + for ; i < len(es.Edits); i++ { + e := es.Edits[i] + if e.Kind != Identity { include = p.context + 1 continue } @@ -138,7 +138,7 @@ func (p *printer) printStruct(es *EditScript) { }() var start, i int - for i < es.Len() { + for i < len(es.Edits) { lastEnd := i // Find provisional start of run. start, i = p.findRun(es, i) @@ -146,7 +146,7 @@ func (p *printer) printStruct(es *EditScript) { p.printSkipped(start - lastEnd) p.printFieldRun(es, start, i) } - p.printSkipped(es.Len() - i) + p.printSkipped(len(es.Edits) - i) } func (p *printer) printList(es *EditScript) { @@ -157,11 +157,11 @@ func (p *printer) printList(es *EditScript) { p.println("]") }() - x := getElems(es.x) - y := getElems(es.y) + x := getElems(es.X) + y := getElems(es.Y) var start, i int - for i < es.Len() { + for i < len(es.Edits) { lastEnd := i // Find provisional start of run. start, i = p.findRun(es, i) @@ -169,7 +169,7 @@ func (p *printer) printList(es *EditScript) { p.printSkipped(start - lastEnd) p.printElemRun(es, x, y, start, i) } - p.printSkipped(es.Len() - i) + p.printSkipped(len(es.Edits) - i) } func getElems(x cue.Value) (a []cue.Value) { @@ -194,63 +194,63 @@ func (p *printer) printValue(v cue.Value) { func (p *printer) printFieldRun(es *EditScript, start, end int) { // Determine max field len. for i := start; i < end; i++ { - e := es.edits[i] + e := es.Edits[i] - switch e.kind { + switch e.Kind { case UniqueX: - p.printField("-", es, es.LabelX(i), es.ValueX(i)) + p.printField("-", e.XSel.String(), es.X.LookupPath(cue.MakePath(e.XSel))) case UniqueY: - p.printField("+", es, es.LabelY(i), es.ValueY(i)) + p.printField("+", e.YSel.String(), es.Y.LookupPath(cue.MakePath(e.YSel))) case Modified: - if e.sub != nil { - io.WriteString(p, es.LabelX(i)) - io.WriteString(p, " ") - p.script(e.sub) + if e.Sub != nil { + io.WriteString(p, e.XSel.String()) + io.WriteString(p, ": ") + p.script(e.Sub) break } // TODO: show per-line differences for multiline strings. - p.printField("-", es, es.LabelX(i), es.ValueX(i)) - p.printField("+", es, es.LabelY(i), es.ValueY(i)) + p.printField("-", e.XSel.String(), es.X.LookupPath(cue.MakePath(e.XSel))) + p.printField("+", e.YSel.String(), es.Y.LookupPath(cue.MakePath(e.YSel))) case Identity: // TODO: write on one line - p.printField("", es, es.LabelX(i), es.ValueX(i)) + p.printField("", e.XSel.String(), es.X.LookupPath(cue.MakePath(e.XSel))) } } } -func (p *printer) printField(prefix string, es *EditScript, label string, v cue.Value) { +func (p *printer) printField(prefix string, label string, v cue.Value) { p.prefix = prefix io.WriteString(p, label) - io.WriteString(p, " ") + io.WriteString(p, ": ") p.printValue(v) io.WriteString(p, "\n") p.prefix = "" } func (p *printer) printElemRun(es *EditScript, x, y []cue.Value, start, end int) { - for _, e := range es.edits[start:end] { - switch e.kind { + for _, e := range es.Edits[start:end] { + switch e.Kind { case UniqueX: - p.printElem("-", x[e.XPos()]) + p.printElem("-", x[e.XSel.Index()]) case UniqueY: - p.printElem("+", y[e.YPos()]) + p.printElem("+", y[e.YSel.Index()]) case Modified: - if e.sub != nil { - p.script(e.sub) + if e.Sub != nil { + p.script(e.Sub) break } // TODO: show per-line differences for multiline strings. - p.printElem("-", x[e.XPos()]) - p.printElem("+", y[e.YPos()]) + p.printElem("-", x[e.XSel.Index()]) + p.printElem("+", y[e.YSel.Index()]) case Identity: // TODO: write on one line - p.printElem("", x[e.XPos()]) + p.printElem("", x[e.XSel.Index()]) } } }