cmd: move jujuc/server/output.go to cmd/

cmd: move jujuc/server/output.go to cmd/

This proposal moves most* of output.go up two levels 
so it can be reused by juju status.

* testMode remains in jujuc/server in server.go

https://code.launchpad.net/~dave-cheney/juju-core/go-cmd-output/+merge/115500

(do not edit description out of merge proposal)

[fwereade, 2012-07-18 09:57:21]

Move LGTM, but please write a simple test or two now that it's exposed.

[dave_cheney.net, 2012-07-18 10:50:43]

Please take a look.

[rog, 2012-07-18 11:06:07]

LGTM, with a few minor suggestions below.

https://codereview.appspot.com/6426043/diff/3006/cmd/output.go
File cmd/output.go (right):

https://codereview.appspot.com/6426043/diff/3006/cmd/output.go#newcode25
cmd/output.go:25: // DefaultFormatters are used by many juju Commands.
// DefaultFormatters holds the formatters that can
// be specified with the --format flag.
?

https://codereview.appspot.com/6426043/diff/3006/cmd/output.go#newcode79
cmd/output.go:79: // and writing a value to a file or to stdout as directed.
// The zero value is ready to use.

https://codereview.appspot.com/6426043/diff/3006/cmd/output.go#newcode85
cmd/output.go:85: // AddFlags injects appropriate command line flags into f.
trivial thought: perhaps include some text saying what the flags are, so someone
using the cmd package can use godoc to find that it interprets -o, --output and
--format.

https://codereview.appspot.com/6426043/diff/3006/cmd/output.go#newcode93
cmd/output.go:93: // Write formats and outputs value as directed by the --format
and --output
s/value/the value/

https://codereview.appspot.com/6426043/diff/3006/cmd/output.go#newcode109
cmd/output.go:109: if bytes != nil {
if len(bytes) > 0 {

https://codereview.appspot.com/6426043/diff/3006/cmd/output.go#newcode112
cmd/output.go:112: _, err = target.Write([]byte{'\n'})
maybe move this just above target.Write, as
bytes = append(bytes, '\n') so there's only one
Write.

[dave_cheney.net, 2012-07-18 11:18:02]

Please take a look.

https://codereview.appspot.com/6426043/diff/3006/cmd/output.go
File cmd/output.go (right):

https://codereview.appspot.com/6426043/diff/3006/cmd/output.go#newcode25
cmd/output.go:25: // DefaultFormatters are used by many juju Commands.
On 2012/07/18 11:06:07, rog wrote:
> // DefaultFormatters holds the formatters that can
> // be specified with the --format flag.
> ?

Done.

https://codereview.appspot.com/6426043/diff/3006/cmd/output.go#newcode79
cmd/output.go:79: // and writing a value to a file or to stdout as directed.
On 2012/07/18 11:06:07, rog wrote:
> // The zero value is ready to use.

Done.

https://codereview.appspot.com/6426043/diff/3006/cmd/output.go#newcode109
cmd/output.go:109: if bytes != nil {
On 2012/07/18 11:06:07, rog wrote:
> if len(bytes) > 0 {

Done.

https://codereview.appspot.com/6426043/diff/3006/cmd/output.go#newcode112
cmd/output.go:112: _, err = target.Write([]byte{'\n'})
On 2012/07/18 11:06:07, rog wrote:
> maybe move this just above target.Write, as
> bytes = append(bytes, '\n') so there's only one
> Write.

Done.

[niemeyer, 2012-07-18 18:56:50]

Very nice. LGTM, with three trivials:

https://codereview.appspot.com/6426043/diff/8001/cmd/output.go
File cmd/output.go (right):

https://codereview.appspot.com/6426043/diff/8001/cmd/output.go#newcode81
cmd/output.go:81: // The zero value is ready to use.

What does "ready to use" mean?

// The zero Output value <explanation of what it does>

https://codereview.appspot.com/6426043/diff/8001/cmd/output.go#newcode88
cmd/output.go:88: func (c *Output) AddFlags(f *gnuflag.FlagSet, name string,
formatters map[string]Formatter) {

s/name/defaultFormatter/

https://codereview.appspot.com/6426043/diff/8001/cmd/output.go#newcode112
cmd/output.go:112: _, err = target.Write(append(bytes, '\n'))

The original logic was more sensible. Reallocating len(bytes)+1 and copying
len(bytes) just to add a '\n' is quite a bit of waste just to avoid calling the
function again.

[dave_cheney.net, 2012-07-18 21:47:04]

*** Submitted:

cmd: move jujuc/server/output.go to cmd/

This proposal moves most* of output.go up two levels 
so it can be reused by juju status.

* testMode remains in jujuc/server in server.go

R=fwereade, rog, niemeyer
CC=
https://codereview.appspot.com/6426043

https://codereview.appspot.com/6426043/diff/8001/cmd/output.go
File cmd/output.go (right):

https://codereview.appspot.com/6426043/diff/8001/cmd/output.go#newcode81
cmd/output.go:81: // The zero value is ready to use.
On 2012/07/18 18:56:50, niemeyer wrote:
> 
> What does "ready to use" mean?
> 
> // The zero Output value <explanation of what it does>

This was a suggestion from Roger. The zero value does what it is supposed to do,
so probably doesn't need further explanation.

https://codereview.appspot.com/6426043/diff/8001/cmd/output.go#newcode88
cmd/output.go:88: func (c *Output) AddFlags(f *gnuflag.FlagSet, name string,
formatters map[string]Formatter) {
On 2012/07/18 18:56:50, niemeyer wrote:
> 
> s/name/defaultFormatter/

Done.

https://codereview.appspot.com/6426043/diff/8001/cmd/output.go#newcode112
cmd/output.go:112: _, err = target.Write(append(bytes, '\n'))
On 2012/07/18 18:56:50, niemeyer wrote:
> 
> The original logic was more sensible. Reallocating len(bytes)+1 and copying
> len(bytes) just to add a '\n' is quite a bit of waste just to avoid calling
the
> function again.

Reverted.