pkg / diff

BSD 3-Clause "New" or "Revised" License
88 stars 9 forks source link

Add a utility function with filenames #4

Open mvdan opened 6 years ago

mvdan commented 6 years ago

That is, func Files(left, right string) DiffWriteable. It would read the contents of the files up-front and diff the lines as []string or [][]byte, while including the original filenames. It would also do things that are common when one diffs plaintext files, such as treating windows line endings like unix line endings, or ignoring a trailing newline unless it appears/disappears.

In the rare case that someone needs this to support streaming, they can just easily implement the interface themselves.

mvdan commented 5 years ago

A better alternative would be an option or wrapper that allows adding context to a diff operation, such as filenames. For example, imagine if I could write:

ab := diff.Strings(a, b)
ab = diff.WithFilenames(ab, "before.txt", "after.txt")

Or using option func parameters:

ab := diff.Strings(a, b, diff.Names("before.txt", "after.txt"))
josharian commented 5 years ago

What do you think about Writeable shrinking to just two methods:

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)
}

And then have WriteUnified take context arguments to provide names, as well as dates, times, time zones, and the other goo that can show up in diff output. Something like:

func (e EditScript) WriteUnified(w io.Writer, ab Writeable, ctx ...WriteContext) (int, error)

Where WriteContext is something like:

type WriteContext interface {
  private() // private ensures that only types in this package can implement WriteContext
}

func WithNames(a, b string) WriteContext() {
  return names{a, b}
}

type names struct {
  a, b string
}

And then WriteUnified can simply peer inside all WriteContexts provided, since it knows all possible types that can occur. Use would look something like this:

ab := diff.Strings(a, b)
n, err := diff.WriteUnified(w, ab, diff.WithNames("before.txt", "after.txt"), diff.WithTimes(atime, btime))

(If no write contexts were provided, we'd use the names a and b, and omit dates/times/etc.)

mvdan commented 5 years ago

I don't like ctx WriteContext because I think everyone is far too used to ctx context.Context :)

Why not call these options or metadata, which is much more common?

Beyond that, I like making names optional and the interface smaller.

josharian commented 5 years ago

I implemented WriteOpts as discussed here, and renamed Writeable to WriterTo. I think this has helped. I'm going to leave this open for future discussion and work on the original idea, which was a utility for diffing files, including a bunch of niceties discussed in the OP.