diff icon indicating copy to clipboard operation
diff copied to clipboard

Add a utility function with filenames

Open mvdan opened this issue 7 years ago • 4 comments

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 avatar Jul 09 '18 22:07 mvdan

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

mvdan avatar Jun 12 '19 17:06 mvdan

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

josharian avatar Aug 27 '19 18:08 josharian

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.

mvdan avatar Aug 28 '19 21:08 mvdan

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.

josharian avatar Aug 30 '19 16:08 josharian