Skip to content

Commit

Permalink
internal/diff: drop last uses of cue.Value.Struct
Browse files Browse the repository at this point in the history 
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í <[email protected]>
Change-Id: I964bbbb97b55025d0e7c911207034f0fc39976a7
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202111
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Roger Peppe <[email protected]>
  • Loading branch information
@mvdan
mvdan committed Oct 7, 2024
1 parent 74c79f6 commit 84646f6
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 122 deletions.
103 changes: 16 additions & 87 deletions internal/diff/diff.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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++ {
Expand All @@ -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
}

Expand All @@ -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,
Expand All @@ -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.
Expand All @@ -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++
}
Expand All @@ -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++
}
Expand All @@ -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}
}
70 changes: 35 additions & 35 deletions internal/diff/print.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -138,15 +138,15 @@ 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)

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) {
Expand All @@ -157,19 +157,19 @@ 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)

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) {
Expand All @@ -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()])
}
}
}
Expand Down

0 comments on commit 84646f6

Please sign in to comment.