Skip to content

Commit

Permalink
rework writing API
Browse files Browse the repository at this point in the history
Use WriteOpts instead of methods to control names.

Rename Writeable to WriterTo.

Updates #4
Updates #8
  • Loading branch information
josharian committed Aug 30, 2019
1 parent 686b669 commit 060157d
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 39 deletions.
21 changes: 6 additions & 15 deletions adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,12 @@ import (
"reflect"
)

// Strings returns a PairWriteable that diffs a and b.
func Strings(a, b []string) PairWriteable {
// Strings returns a PairWriterTo that can diff and write a and b.
func Strings(a, b []string) PairWriterTo {
return &diffStrings{a: a, b: b}
}

type diffStrings struct {
defaultNames
a, b []string
}

Expand All @@ -23,13 +22,12 @@ func (ab *diffStrings) Equal(ai, bi int) bool { return ab.a[a
func (ab *diffStrings) WriteATo(w io.Writer, i int) (int, error) { return io.WriteString(w, ab.a[i]) }
func (ab *diffStrings) WriteBTo(w io.Writer, i int) (int, error) { return io.WriteString(w, ab.b[i]) }

// Bytes returns a PairWriteable that diffs a and b.
func Bytes(a, b [][]byte) PairWriteable {
// Bytes returns a PairWriterTo that can diff and write a and b.
func Bytes(a, b [][]byte) PairWriterTo {
return &diffBytes{a: a, b: b}
}

type diffBytes struct {
defaultNames
a, b [][]byte
}

Expand All @@ -39,11 +37,11 @@ func (ab *diffBytes) Equal(ai, bi int) bool { return bytes.Eq
func (ab *diffBytes) WriteATo(w io.Writer, i int) (int, error) { return w.Write(ab.a[i]) }
func (ab *diffBytes) WriteBTo(w io.Writer, i int) (int, error) { return w.Write(ab.b[i]) }

// Slices returns a PairWriteable that diffs a and b.
// Slices returns a PairWriterTo that diffs a and b.
// It uses fmt.Print to print the elements of a and b.
// It uses equal to compare elements of a and b;
// if equal is nil, Slices uses reflect.DeepEqual.
func Slices(a, b interface{}, equal func(x, y interface{}) bool) PairWriteable {
func Slices(a, b interface{}, equal func(x, y interface{}) bool) PairWriterTo {
if equal == nil {
equal = reflect.DeepEqual
}
Expand All @@ -55,7 +53,6 @@ func Slices(a, b interface{}, equal func(x, y interface{}) bool) PairWriteable {
}

type diffSlices struct {
defaultNames
a, b reflect.Value
eq func(x, y interface{}) bool
}
Expand All @@ -68,12 +65,6 @@ func (ab *diffSlices) Equal(ai, bi int) bool { return ab.eq(a
func (ab *diffSlices) WriteATo(w io.Writer, i int) (int, error) { return fmt.Fprint(w, ab.atA(i)) }
func (ab *diffSlices) WriteBTo(w io.Writer, i int) (int, error) { return fmt.Fprint(w, ab.atB(i)) }

// defaultNames provides NameA == "a" and NameB == "b".
type defaultNames struct{}

func (ab defaultNames) NameA() string { return "a" }
func (ab defaultNames) NameB() string { return "b" }

// TODO: consider adding a LargeFile wrapper.
// It should read each file once, storing the location of all newlines in each file,
// probably using a compact, delta-based encoding.
Expand Down
23 changes: 9 additions & 14 deletions diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,19 @@ type Pair interface {
Equal(ai, bi int) bool
}

// A Writeable type can be written using WriteUnified.
// A WriterTo type supports writing a diff, element by element.
// A is the initial state; B is the final state.
type Writeable interface {
// WriteATo writes the element a[idx] to w.
WriteATo(w io.Writer, idx int) (int, error)
// WriteBTo writes the element b[idx] to w.
WriteBTo(w io.Writer, idx int) (int, error)
// NameA returns the name to use for a, commonly a filename.
NameA() string
// NameB returns the name to use for b, commonly a filename.
NameB() string
type WriterTo interface {
// WriteATo writes the element a[ai] to w.
WriteATo(w io.Writer, ai int) (int, error)
// WriteBTo writes the element b[bi] to w.
WriteBTo(w io.Writer, bi int) (int, error)
}

// PairWriteable is the union of Pair and Writeable.
// TODO: better name
type PairWriteable interface {
// PairWriterTo is the union of Pair and WriterTo.
type PairWriterTo interface {
Pair
Writeable
WriterTo
}

// TODO: consider adding a StringIntern type, something like:
Expand Down
16 changes: 16 additions & 0 deletions example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,19 @@ func Example_strings() {
// c
// +d
}

func Example_Names() {
a := []string{"a", "b", "c"}
b := []string{"a", "c", "d"}
ab := diff.Strings(a, b)
e := diff.Myers(context.Background(), ab)
e.WriteUnified(os.Stdout, ab, diff.Names("before", "after"))
// Output:
// --- before
// +++ after
// @@ -1,3 +1,3 @@
// a
// -b
// c
// +d
}
45 changes: 38 additions & 7 deletions print.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,48 @@ import (
// TODO: add html diffs (?)
// TODO: add intraline highlighting?

// WriteUnified writes e to w using unified diff format, using ab to write the individual elements.
// It returns the number of bytes written successfully and the first error (if any) encountered.
func (e EditScript) WriteUnified(w io.Writer, ab Writeable) (int, error) {
// A WriteOpt is used to provide options when writing a diff.
type WriteOpt interface {
isWriteOpt()
}

// Names provides the before/after names for writing a diff.
// They are traditionally filenames.
func Names(a, b string) WriteOpt {
return names{a, b}
}

type names struct {
a, b string
}

func (names) isWriteOpt() {}

// WriteUnified writes e to w using unified diff format.
// ab writes the individual elements. Opts are optional write arguments.
// WriteUnified returns the number of bytes written and the first error (if any) encountered.
func (e EditScript) WriteUnified(w io.Writer, ab WriterTo, opts ...WriteOpt) (int, error) {
// read opts
nameA := "a"
nameB := "b"
for _, opt := range opts {
switch opt := opt.(type) {
case names:
nameA = opt.a
nameB = opt.b
// TODO: add date/time/timezone WriteOpts
default:
panic(fmt.Sprintf("unrecognized WriteOpt type %T", opt))
}
}

w = newErrWriter(w)
// TODO: Wrap w in a bufio.Writer? And then use w.WriteByte below instead of w.Write.
// Maybe bufio.Writer is enough and we should entirely ditch newErrWriter.

// per-file header
// TODO: add date/time/timezone methods to ab and use them here
fmt.Fprintf(w, "--- %s\n", ab.NameA())
fmt.Fprintf(w, "+++ %s\n", ab.NameB())
fmt.Fprintf(w, "--- %s\n", nameA)
fmt.Fprintf(w, "+++ %s\n", nameB)

for i := 0; i < len(e.segs); {
// Peek into the future to learn the line ranges for this chunk of output.
Expand Down Expand Up @@ -59,7 +90,7 @@ func (e EditScript) WriteUnified(w io.Writer, ab Writeable) (int, error) {

// Print chunk header.
// TODO: add per-chunk context, like what function we're in
// But how do we get this? need to add PairWriteable methods?
// But how do we get this? need to add PairWriter methods?
// Maybe it should be stored in the EditScript,
// and we can have EditScript methods to populate it somehow?
fmt.Fprintf(w, "@@ -%s +%s @@\n", ar, br)
Expand Down
4 changes: 1 addition & 3 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ Useful background reading about diffs:
TODO before declaring this package stable:

* API review. Some open questions:
- Pair won't suffice for other diff algorithms, like patience diff or using indentation-based heuristics. That's probably ok, but input welcome.
- Writeable won't suffice if we want to add dates/times/timezones to unified diff output. Do we?
- Do we want a single union Writeable interface for all possible kinds of EditScript printing? If not, what should this one be called?
- Pair won't suffice for other diff algorithms, like patience diff or using indentation-based heuristics. Writer/WriteOpts might not suffice for other writing formats. We'll probably just need to cross that bridge when we get to it, but our current names might be too general.
- Should there be some way to step through an EditScript manually? E.g. for use in a system that uses the diff to perform actions to get to a desired end state.
- I've long wanted a way to vary the number of lines of context depending on content. For example, if I deleted the first line of a function, I don't need three full lines of "before" context; it should truncate at the function declaration. Do we have enough state to store such information, if needed? (Would it be better to make EditScript an interface?)
- What is the compatibility guarantee? Do we guarantee the exact same diff given the same input? If not, it is less useful for other peoples' golden tests, but it really ties the hands of the package authors; it'd be nice to be able to generate better diffs over time.
Expand Down

0 comments on commit 060157d

Please sign in to comment.