feat(diff): overhauling diff to use difff

api/datasets.go

@@ -337,37 +337,38 @@ func (h *DatasetHandlers) diffHandler(w http.ResponseWriter, r *http.Request) {
  d.Format = r.FormValue("format")
  }
 
- left, err := DatasetRefFromPath(d.Left)
- if err != nil {
- util.WriteErrResponse(w, http.StatusBadRequest, fmt.Errorf("error getting datasetRef from left path: %s", err.Error()))
- return
- }
-
- right, err := DatasetRefFromPath(d.Right)
- if err != nil {
- util.WriteErrResponse(w, http.StatusBadRequest, fmt.Errorf("error getting datasetRef from right path: %s", err.Error()))
- return
- }
+ // TODO (b5): finish
+ // left, err := DatasetRefFromPath(d.Left)
+ // if err != nil {
+ // util.WriteErrResponse(w, http.StatusBadRequest, fmt.Errorf("error getting datasetRef from left path: %s", err.Error()))
+ // return
+ // }
+
+ // right, err := DatasetRefFromPath(d.Right)
+ // if err != nil {
+ // util.WriteErrResponse(w, http.StatusBadRequest, fmt.Errorf("error getting datasetRef from right path: %s", err.Error()))
+ // return
+ // }
 
  diffs := make(map[string]*dsdiff.SubDiff)
- p := &lib.DiffParams{
- Left: left,
- Right: right,
- DiffAll: true,
- }
-
- if err = h.Diff(p, &diffs); err != nil {
- util.WriteErrResponse(w, http.StatusInternalServerError, fmt.Errorf("error diffing datasets: %s", err))
- }
-
- if d.Format != "" {
- formattedDiffs, err := dsdiff.MapDiffsToString(diffs, d.Format)
- if err != nil {
- util.WriteErrResponse(w, http.StatusInternalServerError, fmt.Errorf("error formating diffs: %s", err))
- }
- util.WriteResponse(w, formattedDiffs)
- return
- }
+ // p := &lib.DiffParams{
+ //  Left: left,
+ //  Right: right,
+ //  DiffAll: true,
+ // }
+
+ // if err = h.Diff(p, &diffs); err != nil {
+ //  util.WriteErrResponse(w, http.StatusInternalServerError, fmt.Errorf("error diffing datasets: %s", err))
+ // }
+
+ // if d.Format != "" {
+ //  formattedDiffs, err := dsdiff.MapDiffsToString(diffs, d.Format)
+ //  if err != nil {
+ //  util.WriteErrResponse(w, http.StatusInternalServerError, fmt.Errorf("error formating diffs: %s", err))
+ //  }
+ //  util.WriteResponse(w, formattedDiffs)
+ //  return
+ // }
 
  util.WriteResponse(w, diffs)
 }

cmd/diff.go

@@ -1,10 +1,11 @@
 package cmd
 
 import (
- "github.com/qri-io/dsdiff"
+ "fmt"
+
+ "github.com/qri-io/difff"
  "github.com/qri-io/ioes"
  "github.com/qri-io/qri/lib"
- "github.com/qri-io/qri/repo"
  "github.com/spf13/cobra"
 )
 
@@ -36,7 +37,7 @@ the same dataset.`,
  },
  }
 
- cmd.Flags().StringVarP(&o.Display, "display", "d", "", "set display format [reg|short|delta|detail]")
+ // cmd.Flags().StringVarP(&o.Display, "display", "d", "", "set display format [reg|short|delta|detail]")
  // datasetDiffCmd.Flags().BoolP("color", "c", false, "set ")
 
  return cmd
@@ -46,16 +47,24 @@ the same dataset.`,
 type DiffOptions struct {
  ioes.IOStreams
 
- Display string
- Left string
- Right string
+ // Display string
+ Selector string
+ Left string
+ Right string
 
  UsingRPC bool
  DatasetRequests *lib.DatasetRequests
 }
 
 // Complete adds any missing configuration that can only be added just before calling Run
 func (o *DiffOptions) Complete(f Factory, args []string) (err error) {
+ if len(args) > 0 {
+ if isDatasetField.MatchString(args[0]) {
+ o.Selector = args[0]
+ args = args[1:]
+ }
+ }
+
  if len(args) > 1 {
  o.Left = args[0]
  o.Right = args[1]
@@ -69,48 +78,51 @@ func (o *DiffOptions) Complete(f Factory, args []string) (err error) {
 }
 
 // Run executes the diff command
-func (o *DiffOptions) Run() error {
+func (o *DiffOptions) Run() (err error) {
  if o.UsingRPC {
  return usingRPCError("diff")
  }
 
- left, err := repo.ParseDatasetRef(o.Left)
- if err != nil && err != repo.ErrEmptyRef {
- return err
- }
- right, err := repo.ParseDatasetRef(o.Right)
- if err != nil && err != repo.ErrEmptyRef {
- return err
- }
-
- diffs := make(map[string]*dsdiff.SubDiff)
+ // diffs := make(map[string]*dsdiff.SubDiff)
  p := &lib.DiffParams{
- Left: left,
- Right: right,
- DiffAll: true,
+ LeftPath: o.Left,
+ RightPath: o.Right,
+ Selector: o.Selector,
+ // DiffAll: true,
  }
 
- if err = o.DatasetRequests.Diff(p, &diffs); err != nil {
+ var changes []*lib.Delta
+ if err = o.DatasetRequests.Diff(p, &changes); err != nil {
  return err
  }
 
- displayFormat := "listKeys"
- switch o.Display {
- case "reg", "regular":
- displayFormat = "listKeys"
- case "short", "s":
- displayFormat = "simple"
- case "delta":
- displayFormat = "delta"
- case "detail":
- displayFormat = "plusMinus"
- }
+ // data, err := json.MarshalIndent(res, "", " ")
+ // if err != nil {
+ // return err
+ // }
 
- result, err := dsdiff.MapDiffsToString(diffs, displayFormat)
+ text, err := difff.FormatPrettyJSON(changes)
  if err != nil {
  return err
  }
-
- printDiffs(o.Out, result)
+ fmt.Fprint(o.Out, text)
+
+ // displayFormat := "listKeys"
+ // switch o.Display {
+ // case "reg", "regular":
+ // displayFormat = "listKeys"
+ // case "short", "s":
+ // displayFormat = "simple"
+ // case "delta":
+ // displayFormat = "delta"
+ // case "detail":
+ // displayFormat = "plusMinus"
+ // }
+
+ // result, err := dsdiff.MapDiffsToString(diffs, displayFormat)
+ // if err != nil {
+ // return err
+ // }
+ // printDiffs(o.Out, result)
  return nil
 }

lib/datasets.go

@@ -11,7 +11,6 @@ import (
  "github.com/qri-io/dag"
  "github.com/qri-io/dataset"
  "github.com/qri-io/dataset/dsfs"
- "github.com/qri-io/dsdiff"
  "github.com/qri-io/jsonschema"
  "github.com/qri-io/qfs"
  "github.com/qri-io/qri/actions"
@@ -572,40 +571,6 @@ func (r *DatasetRequests) Validate(p *ValidateDatasetParams, errors *[]jsonschem
  return
 }
 
-// DiffParams defines parameters for diffing two datasets with Diff
-type DiffParams struct {
- // The pointers to the datasets to diff
- Left, Right repo.DatasetRef
- // override flag to diff full dataset without having to specify each component
- DiffAll bool
- // if DiffAll is false, DiffComponents specifies which components of a dataset to diff
- // currently supported components include "structure", "data", "meta", "transform", and "viz"
- DiffComponents map[string]bool
-}
-
-// Diff computes the diff of two datasets
-func (r *DatasetRequests) Diff(p *DiffParams, diffs *map[string]*dsdiff.SubDiff) (err error) {
- refs := []repo.DatasetRef{}
-
- // Handle `qri use` to get the current default dataset.
- if err := DefaultSelectedRefs(r.node.Repo, &refs); err != nil {
- return err
- }
- // fill in the left side if Left is empty, and there are enough
- // refs in the `use` list
- if p.Left.IsEmpty() && len(refs) > 0 {
- p.Left = refs[0]
- }
- // fill in the right side if Right is empty, and there are enough
- // refs in the `use` list
- if p.Right.IsEmpty() && len(refs) > 1 {
- p.Right = refs[1]
- }
-
- *diffs, err = actions.DiffDatasets(r.node, p.Left, p.Right, p.DiffAll, p.DiffComponents)
- return
-}
-
 // Manifest generates a manifest for a dataset path
 func (r *DatasetRequests) Manifest(refstr *string, m *dag.Manifest) (err error) {
  if r.cli != nil {

lib/datasets_test.go

@@ -17,7 +17,6 @@ import (
  "github.com/qri-io/dataset/dsfs"
  "github.com/qri-io/dataset/dsio"
  "github.com/qri-io/dataset/dstest"
- "github.com/qri-io/dsdiff"
  "github.com/qri-io/jsonschema"
  "github.com/qri-io/qfs"
  "github.com/qri-io/qfs/cafs"
@@ -799,98 +798,6 @@ Pirates of the Caribbean: At World's End ,foo
  }
 }
 
-func TestDatasetRequestsDiff(t *testing.T) {
- rc, _ := regmock.NewMockServer()
- mr, err := testrepo.NewTestRepo(rc)
- if err != nil {
- t.Fatalf("error allocating test repo: %s", err.Error())
- }
- node, err := p2p.NewQriNode(mr, config.DefaultP2PForTesting())
- if err != nil {
- t.Fatal(err.Error())
- }
- req := NewDatasetRequests(node, nil)
-
- // File 1
- fp1, err := dstest.BodyFilepath("testdata/jobs_by_automation")
- if err != nil {
- t.Errorf("getting data filepath: %s", err.Error())
- return
- }
-
- dsRef1 := repo.DatasetRef{}
- initParams := &SaveParams{
- Dataset: &dataset.Dataset{
- Name: "jobs_ranked_by_automation_prob",
- BodyPath: fp1,
- },
- }
-
- err = req.Save(initParams, &dsRef1)
- if err != nil {
- t.Errorf("couldn't init file 1: %s", err.Error())
- return
- }
-
- // File 2
- fp2, err := dstest.BodyFilepath("testdata/jobs_by_automation_2")
- if err != nil {
- t.Errorf("getting data filepath: %s", err.Error())
- return
- }
- dsRef2 := repo.DatasetRef{}
- initParams = &SaveParams{
- Dataset: &dataset.Dataset{
- Name: "jobs_ranked_by_automation_prob",
- BodyPath: fp2,
- },
- }
- err = req.Save(initParams, &dsRef2)
- if err != nil {
- t.Errorf("couldn't load second file: %s", err.Error())
- return
- }
-
- //test cases
- cases := []struct {
- Left, Right repo.DatasetRef
- All bool
- Components map[string]bool
- displayFormat string
- expected string
- err string
- }{
- {dsRef1, dsRef2, false, map[string]bool{"structure": true}, "listKeys", "Structure: 3 changes\n\t- modified checksum\n\t- modified length\n\t- modified schema", ""},
- {dsRef1, dsRef2, true, nil, "listKeys", "Structure: 3 changes\n\t- modified checksum\n\t- modified length\n\t- modified schema", ""},
- }
- // execute
- for i, c := range cases {
- p := &DiffParams{
- Left: c.Left,
- Right: c.Right,
- DiffAll: c.All,
- DiffComponents: c.Components,
- }
- res := map[string]*dsdiff.SubDiff{}
- err := req.Diff(p, &res)
- if !(err == nil && c.err == "" || err != nil && err.Error() == c.err) {
- t.Errorf("case %d error mismatch: expected '%s', got '%s'", i, c.err, err.Error())
- }
-
- if c.err != "" {
- continue
- }
-
- stringDiffs, err := dsdiff.MapDiffsToString(res, c.displayFormat)
- if err != nil {
- t.Errorf("case %d error mapping to string: %s", i, err.Error())
- }
- if stringDiffs != c.expected {
- t.Errorf("case %d response mistmatch: expected '%s', got '%s'", i, c.expected, stringDiffs)
- }
- }
-}
-
 // Convert the interface value into an array, or panic if not possible
 func mustBeArray(i interface{}, err error) []interface{} {
  if err != nil {