pkg / diff

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

Is it worthwhile to separate Diffable and Writeable? #8

Open mvdan opened 6 years ago

mvdan commented 6 years ago

As I was reading the TODO: better name on DiffWriteable, I thought to myself - is there a good reason to separate the interfaces?

Unifying them would fix the naming issue, and make the simple use of the API simpler.

On the other hand, there's something I don't like about a "diffable" interface concerning io.Writer.

mvdan commented 6 years ago

Here's another proposal - make Writeable half of what it is. That is:

type Writeable interface {
    WriteTo(w io.Writer, i int) (int, error)
    Filename() string
}

func (e EditScript) WriteUnified(w io.Writer, left, right Writeable) (int, error)

Then the issue is that helpers like Strings would either start returning a Diffable only, or Diffable, Writeable, Writeable.

Still, this option B would remove the need for DiffWriteable, and it would make funcs like WriteUnified a bit more modular.

josharian commented 5 years ago

Here's another proposal - make Writeable half of what it is.

This interacts with https://github.com/pkg/diff/issues/4, which would get rid of the Filename part of Writeable. At which point it is a single method, so it could be a func param. I can't say that this feels like it fixes any problems, though.

Another option would be give Diffable an AtA and AtB method to get individual elements, and then just pass whatever those return into fmt.Fprint. This has an appeal, but it doesn't play nicely with diffing and writing slices of byte slices, since by default, byte slices print as a slice of integral values. We could work around that by special-casing byte slices, but that's ugly.

Any other ideas?

josharian commented 5 years ago

I just pushed some commits that make this a bit cleaner. I still agree it'd be nice to do something cleaner here; writing is a bit of a fly in the ointment. So I'll leave this open a bit for further discussion. But I'm out of ideas at the moment.