fix(diff): sorting output must respect integer addresses

before we were treating them like strings

.gitignore

@@ -1,4 +1,5 @@
 coverage.txt
 testdata/graph.dot
 deepdiff.test
-deepdiff
+deepdiff
+testdata/graphs

README.md

@@ -113,9 +113,8 @@ func main() {
 		panic(err)
 	}
 
-	// Diff will use default configuration to produce a slice of Deltas
-	// that describe the structured changes. by default Diff will not updates
-	// only inserts & deletes
+	// Diff will use default configuration to produce a slice of Changes
+	// by default Diff will not generate update change only inserts & deletes
 	diffs, err := deepdiff.Diff(a, b)
 	if err != nil {
 		panic(err)

deepdiff.go

@@ -11,9 +11,9 @@ import (
 
 // Config are any possible configuration parameters for calculating diffs
 type Config struct {
-	// Setting Changes to true will have diff represent in-place value shifts
+	// Setting CalcChanges to true will have diff represent in-place value shifts
 	// as changes instead of add-delete pairs
-	Changes bool
+	CalcChanges bool
 }
 
 // DiffOption is a function that adjust a config, zero or more DiffOptions
@@ -33,7 +33,7 @@ func New(opts ...DiffOption) *DeepDiff {
 	}
 
 	return &DeepDiff{
-		changes: cfg.Changes,
+		changes: cfg.CalcChanges,
 	}
 }
 
@@ -53,7 +53,7 @@ func (dd *DeepDiff) StatDiff(ctx context.Context, a, b interface{}) (Deltas, *St
 	return deepdiff.diff(ctx), deepdiff.stats, nil
 }
 
-// Stat calculates the DiffStata between two documents
+// Stat calculates the DiffStats between two documents
 func (dd *DeepDiff) Stat(ctx context.Context, a, b interface{}) (*Stats, error) {
 	deepdiff := &diff{changes: dd.changes, d1: a, d2: b, stats: &Stats{}}
 	deepdiff.diff(ctx)
@@ -372,6 +372,7 @@ func (d *diff) calcDeltas(t1, t2 node) (dts Deltas) {
 			// (eg, empty object is a leaf node)
 			if delta := compareScalar(match, n, p[len(p)-1]); delta != nil {
 				n.SetChangeType(DTUpdate)
+				// TODO (b5) - restore support for change calculation, add tests
 				// if d.changes {
 				// 	// addDelta(root, delta, p)
 				// 	dts = append(dts, delta)
@@ -454,6 +455,7 @@ func sortDeltasAndMaybeCalcStats(deltas Deltas, st *Stats) {
 	}
 }
 
+// TODO (b5) - restore this. We need this if we want to show moves.
 // // calcReorderDeltas creates deltas that describes moves within the same parent
 // // it starts by calculates the largest (order preserving) common subsequence between
 // // two matched parent compound nodes. Background on LCSS:

deepdiff_test.go

@@ -225,22 +225,37 @@ func TestBasicDiffing(t *testing.T) {
 				{Type: DTContext, Path: StringAddr("c"), Value: []interface{}{float64(3)}},
 			},
 		},
+		{
+			"two inserts",
+			`[
+				{ "fruit": "apple", "color": "red" },
+				{ "fruit": "banana", "color": "yellow" },
+				{ "fruit": "cherry", "color": "red" }
+			]`,
+			`[
+				{ "fruit": "apple", "color": "red" },
+				{ "fruit": "blueberry", "color": "blue" },
+				{ "fruit": "cherry", "color": "red" },
+				{ "fruit": "durian", "color": "green" }
+			]`,
+			Deltas{
+				{Type:  DTContext, Path:  IndexAddr(0), Value: map[string]interface{}{"color": string("red"), "fruit": string("apple")}},
+				{Type:  DTDelete, Path:  IndexAddr(1), Value: map[string]interface{}{"color": string("yellow"), "fruit": string("banana")}},
+				{Type:  DTInsert, Path:  IndexAddr(1), Value: map[string]interface{}{"color": string("blue"), "fruit": string("blueberry")}},
+				{Type:  DTContext, Path:  IndexAddr(2), Value: map[string]interface{}{"color": string("red"), "fruit": string("cherry")}},
+				{Type:  DTInsert, Path:  IndexAddr(3), Value: map[string]interface{}{"color": string("green"), "fruit": string("durian")}},
+			},
+		},
 
-		// TODO (b5) - another problematic diff case
+		// TODO (b5) - problematic test case
 		// {
 		// 	"diff object and array children",
 		// 	`{"a":[0,1,2], "b": true }`,
 		// 	`{"a":{"foo": [0,1,2] }, "b": false }`,
-		// 	Deltas{},
-		// },
-
-		// TODO (b5) - I think this should be an error. These inputs don't share a
-		// common root data type
-		// {
-		// 	"object-to-array root",
-		// 	`[ [1,2,3], [4,5,6], [7,8,9] ]`,
-		// 	`{ "foo": [1,2,3], "baz": { "bat": [false]}}`,
-		// 	Deltas{},
+		// 	Deltas{
+		// 		{Type: DTContext, Path: RootPath{}, Value: map[string]interface{}{"a": []interface{}{float64(0), float64(1), float64(2)}, "b": bool(true)}},
+		// 		{Type: DTInsert, Path: RootAddr{}, Value: }
+		// 	},
 		// },
 	}
 
@@ -276,7 +291,39 @@ func TestChangeDiffs(t *testing.T) {
 		},
 	}
 
-	RunTestCases(t, cases, func(c *Config) { c.Changes = true })
+	RunTestCases(t, cases, func(c *Config) { c.CalcChanges = true })
+}
+
+func TestDeltaSorting(t *testing.T) {
+	cases := []TestCase{
+		{
+		"more than 10 index-addressed deltas",
+		`[[0],[1],[2],[3],[4],[5],[6],[7],[8],[9],[10],[11],[12],[13],[14]]`,
+		`[[0],[1],[2],[3],[4],[5],[6],[7],[8],["apple splat"],[10],[11],[12],[13],[14]]`,
+		Deltas{
+			{Type: DTContext, Path: IndexAddr(0), Value: []interface{}{float64(0)} },
+			{Type: DTContext, Path: IndexAddr(1), Value: []interface{}{float64(1)} },
+			{Type: DTContext, Path: IndexAddr(2), Value: []interface{}{float64(2)} },
+			{Type: DTContext, Path: IndexAddr(3), Value: []interface{}{float64(3)} },
+			{Type: DTContext, Path: IndexAddr(4), Value: []interface{}{float64(4)} },
+			{Type: DTContext, Path: IndexAddr(5), Value: []interface{}{float64(5)} },
+			{Type: DTContext, Path: IndexAddr(6), Value: []interface{}{float64(6)} },
+			{Type: DTContext, Path: IndexAddr(7), Value: []interface{}{float64(7)} },
+			{Type: DTContext, Path: IndexAddr(8), Value: []interface{}{float64(8)} },
+			{Type: DTContext, Path: IndexAddr(9), Deltas: Deltas{
+				{Type: DTDelete, Path: IndexAddr(0), Value: float64(9) },
+				{Type: DTInsert, Path: IndexAddr(0), Value: "apple splat" },
+			}},
+			{Type: DTContext, Path: IndexAddr(10), Value: []interface{}{float64(10)} },
+			{Type: DTContext, Path: IndexAddr(11), Value: []interface{}{float64(11)} },
+			{Type: DTContext, Path: IndexAddr(12), Value: []interface{}{float64(12)} },
+			{Type: DTContext, Path: IndexAddr(13), Value: []interface{}{float64(13)} },
+			{Type: DTContext, Path: IndexAddr(14), Value: []interface{}{float64(14)} },
+		},
+	},
+	}
+
+	RunTestCases(t, cases)
 }
 
 func TestInsertGeneralizing(t *testing.T) {
@@ -337,33 +384,27 @@ func TestInsertGeneralizing(t *testing.T) {
 
 func TestNestedScalar(t *testing.T) {
 	cases := []TestCase{
-		// {
-		// 	"single nested scalar change no scalar matches",
-		// 	`{ "structure": { "formatConfig": { "headerRow": false }}}`,
-		// 	`{ "structure": { "formatConfig": { "headerRow": true }}}`,
-		// 	Deltas{
-		// 		{Type: DTDelete, Path: RootAddr{}, Value: map[string]interface{}{
-		// 			"structure" : map[string]interface{}{
-		// 				"formatConfig": map[string]interface{}{
-		// 					"headerRow": false,
-		// 				},
-		// 			},
-		// 		}},
-		// 		{Type: DTInsert, Path: RootAddr{}, Value: map[string]interface{}{
-		// 			"structure" : map[string]interface{}{
-		// 				"formatConfig": map[string]interface{}{
-		// 					"headerRow": true,
-		// 				},
-		// 			},
-		// 		}},
-		// 		// {Type: DTContext, Path: StringAddr("structure"), Deltas: Deltas{
-		// 		// 	{ Type: DTContext, Path: StringAddr("formatConfig"), Deltas: Deltas{
-		// 		// 		{Type: DTDelete, Path: StringAddr("headerRow"), Value: false },
-		// 		// 		{Type: DTInsert, Path: StringAddr("headerRow"), Value: true },
-		// 		// 	}},
-		// 		// }},
-		// 	},
-		// },
+		{
+			"single nested scalar change no scalar matches",
+			`{ "structure": { "formatConfig": { "headerRow": false }}}`,
+			`{ "structure": { "formatConfig": { "headerRow": true }}}`,
+			Deltas{
+				{Type: DTDelete, Path: RootAddr{}, Value: map[string]interface{}{
+					"structure" : map[string]interface{}{
+						"formatConfig": map[string]interface{}{
+							"headerRow": false,
+						},
+					},
+				}},
+				{Type: DTInsert, Path: RootAddr{}, Value: map[string]interface{}{
+					"structure" : map[string]interface{}{
+						"formatConfig": map[string]interface{}{
+							"headerRow": true,
+						},
+					},
+				}},
+			},
+		},
 		{
 			"single nested scalar change one scalar match",
 			`{ "qri": "ds:0",  "structure": { "formatConfig": { "headerRow": false }}}`,
@@ -392,11 +433,17 @@ func TestRootChanges(t *testing.T) {
 
 	cases := []TestCase{
 		{
-			"large structure overflow",
+			"object-to-array",
 			`{ "qri": "ds:0" }`,
 			`[ "ds:0", ["rank","probability_of_automation","soc_code","job_title"] ]`,
 			Deltas{},
 		},
+		{
+			"array-to-object root",
+			`[ [1,2,3], [4,5,6], [7,8,9] ]`,
+			`{ "foo": [1,2,3], "baz": { "bat": [false]}}`,
+			Deltas{},
+		},
 	}
 
 	RunTestCases(t, cases)

delta.go

@@ -119,7 +119,15 @@ func (RootAddr) MarshalJSON() ([]byte, error) {
 type sortableAddrs []Addr
 
 func (a sortableAddrs) Len() int { return len(a) }
-func (a sortableAddrs) Less(i,j int) bool { return a[i].String() < a[j].String() }
+func (a sortableAddrs) Less(i,j int) bool {
+	if ii, ok := a[i].Value().(int); ok {
+		if jj, ok := a[j].Value().(int); ok {
+			return ii < jj
+		}
+	}
+
+	return a[i].String() < a[j].String()
+}
 func (a sortableAddrs) Swap(i,j int) {
 	a[i], a[j] = a[j], a[i]
 }
@@ -165,20 +173,30 @@ type Deltas []*Delta
 func (ds Deltas) Len() int { return len(ds) }
 
 // opOrder determines a total order for operations. It's crucial that deletes
-// comve *before* inserts in diff script presentation
+// come *before* inserts in diff script presentation
 var opOrder = map[Operation]uint{
 	DTDelete:  0,
 	DTContext: 1,
 	DTInsert:  2,
 	DTUpdate:  3,
 }
 
-// Less returns trus if the value at index i is a smaller sort quantity than
+// Less returns true if the value at index i is a smaller sort quantity than
 // the value at index j
 func (ds Deltas) Less(i, j int) bool {
-	iAddr := ds[i].Path.String()
-	jAddr := ds[j].Path.String()
-	return iAddr < jAddr || (iAddr == jAddr && opOrder[ds[i].Type] < opOrder[ds[j].Type])
+	iAddr := ds[i].Path
+	jAddr := ds[j].Path
+	if iAddr.Eq(jAddr) {
+		return opOrder[ds[i].Type] < opOrder[ds[j].Type]
+	}
+
+	if a, ok := ds[i].Path.Value().(int); ok {
+		if b, ok := ds[j].Path.Value().(int); ok {
+			return a < b 
+		}
+	}
+
+	return iAddr.String() < jAddr.String()
 }
 
 // Swap reverses the values at indicies i & J

format.go

@@ -8,8 +8,8 @@ import (
 	"strings"
 )
 
-// FormatPrettyString is a convenice wrapper that outputs to a string instead of
-// an io.Writer
+// FormatPrettyString is a convenience wrapper that outputs to a string instead
+// of an io.Writer
 func FormatPrettyString(changes Deltas, colorTTY bool) (string, error) {
 	buf := &bytes.Buffer{}
 	if err := FormatPretty(buf, changes, colorTTY); err != nil {
@@ -27,19 +27,23 @@ func FormatPretty(w io.Writer, changes Deltas, colorTTY bool) error {
 	var colorMap map[Operation]string
 
 	if colorTTY {
-		colorMap = map[Operation]string{
-			Operation("close"): "\x1b[0m", // end color tag
-
-			DTContext: "\x1b[37m", // netural
-			DTInsert:  "\x1b[32m", // green
-			DTDelete:  "\x1b[31m", // red
-			DTUpdate:  "\x1b[34m", // blue
-		}
+		colorMap = ttyColorMap()
 	}
 
 	return formatPretty(w, changes, 0, colorMap)
 }
 
+func ttyColorMap() map[Operation]string {
+	return map[Operation]string{
+		Operation("close"): "\x1b[0m", // end color tag
+
+		DTContext: "\x1b[37m", // netural
+		DTInsert:  "\x1b[32m", // green
+		DTDelete:  "\x1b[31m", // red
+		DTUpdate:  "\x1b[34m", // blue
+	}
+}
+
 func formatPretty(w io.Writer, changes Deltas, indent int, colorMap map[Operation]string) error {
 	for _, d := range changes {
 		dataStr := ""
@@ -74,14 +78,7 @@ func FormatPrettyStats(w io.Writer, diffStat *Stats, colorTTY bool) {
 	var colorMap map[Operation]string
 
 	if colorTTY {
-		colorMap = map[Operation]string{
-			Operation("close"): "\x1b[0m", // end color tag
-
-			DTContext: "\x1b[37m", // netural
-			DTInsert:  "\x1b[32m", // green
-			DTDelete:  "\x1b[31m", // red
-			DTUpdate:  "\x1b[34m", // blue
-		}
+		colorMap = ttyColorMap()
 	}
 
 	formatStats(w, diffStat, colorMap)

patch.go

@@ -34,33 +34,24 @@ func patch(target reflect.Value, delta *Delta) (reflect.Value, error) {
 	// value to the updated child
 	if len(delta.Deltas) > 0 {
 		for _, dlt := range delta.Deltas {
-			// fmt.Printf("patching %s on target %#v\n", dlt.Path, target)
 			patchedChild, err := patch(child(target, delta.Path), dlt)
 			if err != nil {
 				return target, err
 			}
 
-			// fmt.Printf("patch output: %#v\n", patchedChild)
-
 			target, err = set(target, patchedChild, delta.Path)
 			if err != nil {
 				return target, err
 			}
-
-			// fmt.Printf("post-patch-set target: %#v\n\n", target)
 		}
 	}
 
 	switch delta.Type {
 	case DTInsert:
-		// fmt.Printf("applying insert to %s on target %#v\n", delta.Path, target)
 		target, err = insert(target, reflect.ValueOf(delta.Value), delta.Path)
 	case DTDelete:
-		// fmt.Printf("applying delete to %s on target %#v\n", delta.Path, target)
 		target, err = remove(target, delta.Path)
-		// fmt.Printf("post-delete target %#v\n", target)
 	case DTUpdate:
-		// fmt.Printf("applying update to %s on target %#v\n", delta.Path, target)
 		target, err = set(target, reflect.ValueOf(delta.Value), delta.Path)
 	}