cmd: add FileVar flag and use it

cmd: add FileVar flag and use it

PyJuJu had a nice construct that allowed a flag that was a file, or more
specifically the contents of a file to be handled nicely. As deploy ended
up implementing this, and I need it now for juju set, it made sense to 
pretty it up.

https://code.launchpad.net/~dave-cheney/juju-core/096-cmd-config-var/+merge/124698

(do not edit description out of merge proposal)

[dave_cheney.net, 2012-09-17 14:07:53]

Please take a look.

[niemeyer, 2012-09-17 15:09:58]

https://codereview.appspot.com/6490121/diff/1/cmd/filevar.go
File cmd/filevar.go (right):

https://codereview.appspot.com/6490121/diff/1/cmd/filevar.go#newcode12
cmd/filevar.go:12: io.ReadCloser
It might be better to have a Open method on the FileVar that returns (file, err)
instead, otherwise it looks pretty easy to forget to close it (as seen in
tests).

https://codereview.appspot.com/6490121/diff/1/cmd/filevar_test.go
File cmd/filevar_test.go (right):

https://codereview.appspot.com/6490121/diff/1/cmd/filevar_test.go#newcode36
cmd/filevar_test.go:36: c.Assert(err, IsNil)
defer config.ReadCloser.Close()?

https://codereview.appspot.com/6490121/diff/1/cmd/juju/cmd_test.go
File cmd/juju/cmd_test.go (right):

https://codereview.appspot.com/6490121/diff/1/cmd/juju/cmd_test.go#newcode239
cmd/juju/cmd_test.go:239: c.Assert(err, IsNil)
defer com.Config.Close()?

[dave_cheney.net, 2012-09-17 15:20:00]

Please take a look.

https://codereview.appspot.com/6490121/diff/1/cmd/filevar.go
File cmd/filevar.go (right):

https://codereview.appspot.com/6490121/diff/1/cmd/filevar.go#newcode12
cmd/filevar.go:12: io.ReadCloser
On 2012/09/17 15:09:58, niemeyer wrote:
> It might be better to have a Open method on the FileVar that returns (file,
err)
> instead, otherwise it looks pretty easy to forget to close it (as seen in
> tests).

Yes ... but, you loose the nice property that the file is opened at parse time,
so you get the error handled nicely in place.

If I make the agreement to not forget to close the file, can I keep it?

https://codereview.appspot.com/6490121/diff/1/cmd/filevar_test.go
File cmd/filevar_test.go (right):

https://codereview.appspot.com/6490121/diff/1/cmd/filevar_test.go#newcode36
cmd/filevar_test.go:36: c.Assert(err, IsNil)
On 2012/09/17 15:09:58, niemeyer wrote:
> defer config.ReadCloser.Close()?

Done.

https://codereview.appspot.com/6490121/diff/1/cmd/juju/cmd_test.go
File cmd/juju/cmd_test.go (right):

https://codereview.appspot.com/6490121/diff/1/cmd/juju/cmd_test.go#newcode239
cmd/juju/cmd_test.go:239: c.Assert(err, IsNil)
On 2012/09/17 15:09:58, niemeyer wrote:
> defer com.Config.Close()?

Done.

[niemeyer, 2012-09-17 15:21:19]

https://codereview.appspot.com/6490121/diff/1/cmd/filevar.go
File cmd/filevar.go (right):

https://codereview.appspot.com/6490121/diff/1/cmd/filevar.go#newcode12
cmd/filevar.go:12: io.ReadCloser
On 2012/09/17 15:20:01, dfc wrote:
> On 2012/09/17 15:09:58, niemeyer wrote:
> > It might be better to have a Open method on the FileVar that returns (file,
> err)
> > instead, otherwise it looks pretty easy to forget to close it (as seen in
> > tests).
> 
> Yes ... but, you loose the nice property that the file is opened at parse
time,
> so you get the error handled nicely in place.
> 
> If I make the agreement to not forget to close the file, can I keep it?

Sure, let's try that out.

LGTM

[dave_cheney.net, 2012-09-17 15:23:20]

*** Submitted:

cmd: add FileVar flag and use it

PyJuJu had a nice construct that allowed a flag that was a file, or more
specifically the contents of a file to be handled nicely. As deploy ended
up implementing this, and I need it now for juju set, it made sense to 
pretty it up.

R=niemeyer
CC=
https://codereview.appspot.com/6490121