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

Ui.Error, Info, Output should take variadic arguments #12

Closed
keyneston opened this issue Dec 11, 2014 · 5 comments
Closed

Ui.Error, Info, Output should take variadic arguments #12

keyneston opened this issue Dec 11, 2014 · 5 comments

Comments

@keyneston
Copy link

Instead of Ui.Error(string) it should be Ui.Error(string, interface{}...). This should then call fmt.Fprintf(w, string, ...v).

Almost all of my calls to Ui.(Error|Info|Output) end up including a call to fmt.Sprintf.

@mitchellh
Copy link
Owner

The reason we don't do this is because we use the interface across RPC in a couple of our projects and it didn't deal well with variadics... At this point it would actually break backwards compatibility too (as a lib).

We also follow the pattern of using fmt.Sprintf everywhere. Is it worth breaking compat over? I'm not sure.

@keyneston
Copy link
Author

Possibly add Errorf, Outputf, Infof functions?

It would technically change the Ui interface if people have written their own implementations but otherwise would be a noop change for consumers of the library.

@keyneston
Copy link
Author

There could also be a UiFormatter interface that implemented Ui and the formatter functions? That way even if people wrote their own implementations of the interface it would be a noop change?

I'm happy to do the work once you give sign off.

@mitchellh
Copy link
Owner

That's a good idea. I like the idea of introducing a second interface. If you want to do a PR for that, that would be okay to me.

@mitchellh
Copy link
Owner

Hey @tarrant, given how long its been I think we've made it long enough that this is okay. We don't want to break BC and the actual other reason we don't do variadics is for a weird reason that this works better with our plugin system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants