Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is it worthwhile to separate Diffable and Writeable? #8

Open
mvdan opened this issue Jul 9, 2018 · 3 comments
Open

Is it worthwhile to separate Diffable and Writeable? #8

mvdan opened this issue Jul 9, 2018 · 3 comments
Labels
needs thought unclear what the right path forward is

Comments

@mvdan
Copy link
Contributor

mvdan commented Jul 9, 2018

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
Copy link
Contributor Author

mvdan commented Jul 9, 2018

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
Copy link
Contributor

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

This interacts with #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 added a commit that referenced this issue Aug 30, 2019
Use WriteOpts instead of methods to control names.

Rename Writeable to WriterTo.

Updates #4
Updates #8
@josharian
Copy link
Contributor

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.

@josharian josharian added the needs thought unclear what the right path forward is label Aug 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs thought unclear what the right path forward is
Projects
None yet
Development

No branches or pull requests

2 participants