cli: use pretty-formatted tables only with terminals or --pretty.

Prior to this patch all commands reporting table results would
print them using ASCII art tables (pretty-formatted). This makes
the output difficult to parse in automated scripts etc.

This patch corrects the behavior by using TSV output when the command
is not used from an interactive terminal. The pretty-formatted output
can be forced for non-terminal use using the command-line argument
`--pretty`.

cli/cli.go

@@ -23,6 +23,7 @@ import (
 	"text/tabwriter"
 
 	"github.com/cockroachdb/cockroach/build"
+	"github.com/mattn/go-isatty"
 	"github.com/spf13/cobra"
 )
 
@@ -60,7 +61,15 @@ var cockroachCmd = &cobra.Command{
 	Long: `CockroachDB command-line interface and server.`,
 }
 
+// isInteractive indicates whether both stdin and stdout refer to the
+// terminal.
+var isInteractive bool
+
 func init() {
+
+	isInteractive = isatty.IsTerminal(os.Stdout.Fd()) &&
+		isatty.IsTerminal(os.Stdin.Fd())
+
 	cockroachCmd.AddCommand(
 		startCmd,
 		certCmd,

cli/cli_test.go

@@ -58,6 +58,7 @@ func newCLITest() cliTest {
 	// pointer (because they are tied into the flags), but instead
 	// overwrite the existing struct's values.
 	baseCtx.InitDefaults()
+	cliCtx.InitCLIDefaults()
 
 	osStderr = os.Stdout
 
@@ -725,30 +726,34 @@ func Example_user() {
 	defer c.stop()
 
 	c.Run("user ls")
+	c.Run("user ls --pretty")
 	c.Run("user set foo --password=bar")
 	// Don't use get, since the output of hashedPassword is random.
 	// c.Run("user get foo")
-	c.Run("user ls")
+	c.Run("user ls --pretty")
 	c.Run("user rm foo")
-	c.Run("user ls")
+	c.Run("user ls --pretty")
 
 	// Output:
 	// user ls
+	// 0 rows
+	// username
+	// user ls --pretty
 	// +----------+
 	// | username |
 	// +----------+
 	// +----------+
 	// user set foo --password=bar
 	// INSERT 1
-	// user ls
+	// user ls --pretty
 	// +----------+
 	// | username |
 	// +----------+
 	// | foo      |
 	// +----------+
 	// user rm foo
 	// DELETE 1
-	// user ls
+	// user ls --pretty
 	// +----------+
 	// | username |
 	// +----------+
@@ -844,10 +849,15 @@ func Example_node() {
 	}
 
 	c.Run("node ls")
+	c.Run("node ls --pretty")
 	c.Run("node status 10000")
 
 	// Output:
 	// node ls
+	// 1 row
+	// id
+	// 1
+	// node ls --pretty
 	// +----+
 	// | id |
 	// +----+
@@ -897,13 +907,13 @@ func TestNodeStatus(t *testing.T) {
 		t.Fatalf("couldn't write stats summaries: %s", err)
 	}
 
-	out, err := c.RunWithCapture("node status 1")
+	out, err := c.RunWithCapture("node status 1 --pretty")
 	if err != nil {
 		t.Fatal(err)
 	}
 	checkNodeStatus(t, c, out, start)
 
-	out, err = c.RunWithCapture("node status")
+	out, err = c.RunWithCapture("node status --pretty")
 	if err != nil {
 		t.Fatal(err)
 	}

cli/context.go

@@ -39,18 +39,27 @@ func (s *statementsValue) Set(value string) error {
 	return nil
 }
 
-type sqlContext struct {
+type cliContext struct {
 	// Embed the base context.
 	*base.Context
 
-	// execStmts is a list of statements to execute.
-	execStmts statementsValue
-
 	// prettyFmt indicates whether tables should be pretty-formatted in
 	// the output during non-interactive execution.
 	prettyFmt bool
 }
 
+func (ctx *cliContext) InitCLIDefaults() {
+	ctx.prettyFmt = false
+}
+
+type sqlContext struct {
+	// Embed the cli context.
+	*cliContext
+
+	// execStmts is a list of statements to execute.
+	execStmts statementsValue
+}
+
 type debugContext struct {
 	startKey, endKey string
 	raw              bool

cli/flags.go

@@ -45,7 +45,8 @@ var undoFreezeCluster bool
 
 var serverCtx = server.MakeContext()
 var baseCtx = serverCtx.Context
-var sqlCtx = sqlContext{Context: baseCtx}
+var cliCtx = cliContext{Context: baseCtx}
+var sqlCtx = sqlContext{cliContext: &cliCtx}
 var debugCtx debugContext
 
 var cacheSize *bytesValue
@@ -440,12 +441,18 @@ func init() {
 		f.StringVar(&baseCtx.SSLCA, cliflags.CACertName, envutil.EnvOrDefaultString(cliflags.CACertName, baseCtx.SSLCA), usageEnv(cliflags.CACertName))
 		f.StringVar(&baseCtx.SSLCert, cliflags.CertName, envutil.EnvOrDefaultString(cliflags.CertName, baseCtx.SSLCert), usageEnv(cliflags.CertName))
 		f.StringVar(&baseCtx.SSLCertKey, cliflags.KeyName, envutil.EnvOrDefaultString(cliflags.KeyName, baseCtx.SSLCertKey), usageEnv(cliflags.KeyName))
+
+		// By default, client commands print their output as
+		// pretty-formatted tables on terminals, and TSV when redirected
+		// to a file. The user can override with --pretty.  We set the
+		// default to TSV here; the case for interactive output is
+		// finalized in OnInitialize() below.
+		f.BoolVar(&cliCtx.prettyFmt, cliflags.PrettyName, false, usageNoEnv(cliflags.PrettyName))
 	}
 
 	{
 		f := sqlShellCmd.Flags()
 		f.VarP(&sqlCtx.execStmts, cliflags.ExecuteName, "e", usageNoEnv(cliflags.ExecuteName))
-		f.BoolVar(&sqlCtx.prettyFmt, cliflags.PrettyName, false, usageNoEnv(cliflags.PrettyName))
 	}
 	{
 		f := freezeClusterCmd.PersistentFlags()
@@ -506,5 +513,9 @@ func init() {
 
 		serverCtx.Addr = net.JoinHostPort(connHost, connPort)
 		serverCtx.HTTPAddr = net.JoinHostPort(connHost, httpPort)
+
+		// Automatically print pretty-printed tables on interactive
+		// terminals.
+		cliCtx.prettyFmt = cliCtx.prettyFmt || isInteractive
 	})
 }

cli/node.go

@@ -72,7 +72,7 @@ func runLsNodes(cmd *cobra.Command, args []string) error {
 		})
 	}
 
-	printQueryOutput(os.Stdout, lsNodesColumnHeaders, rows, "", true)
+	printQueryOutput(os.Stdout, lsNodesColumnHeaders, rows, "", cliCtx.prettyFmt)
 	return nil
 }
 
@@ -141,7 +141,7 @@ func runStatusNode(cmd *cobra.Command, args []string) error {
 		return util.Errorf("expected no arguments or a single node ID")
 	}
 
-	printQueryOutput(os.Stdout, nodesColumnHeaders, nodeStatusesToRows(nodeStatuses), "", true)
+	printQueryOutput(os.Stdout, nodesColumnHeaders, nodeStatusesToRows(nodeStatuses), "", cliCtx.prettyFmt)
 	return nil
 }
 

cli/sql.go

@@ -30,7 +30,6 @@ import (
 	"github.com/chzyer/readline"
 	"github.com/cockroachdb/cockroach/util/envutil"
 	"github.com/cockroachdb/cockroach/util/log"
-	"github.com/mattn/go-isatty"
 	"github.com/spf13/cobra"
 )
 
@@ -222,9 +221,6 @@ func preparePrompts(dbURL string) (fullPrompt string, continuePrompt string) {
 func runInteractive(conn *sqlConn) (exitErr error) {
 	fullPrompt, continuePrompt := preparePrompts(conn.url)
 
-	isInteractive := isatty.IsTerminal(os.Stdout.Fd()) &&
-		isatty.IsTerminal(os.Stdin.Fd())
-
 	if isInteractive {
 		// We only enable history management when the terminal is actually
 		// interactive. This saves on memory when e.g. piping a large SQL
@@ -297,7 +293,7 @@ func runInteractive(conn *sqlConn) (exitErr error) {
 			addHistory(strings.Join(stmt, " "))
 		}
 
-		if exitErr = runQueryAndFormatResults(conn, os.Stdout, makeQuery(fullStmt), true); exitErr != nil {
+		if exitErr = runQueryAndFormatResults(conn, os.Stdout, makeQuery(fullStmt), cliCtx.prettyFmt); exitErr != nil {
 			fmt.Fprintln(osStderr, exitErr)
 		}
 
@@ -333,7 +329,7 @@ func runTerm(cmd *cobra.Command, args []string) error {
 
 	if len(sqlCtx.execStmts) > 0 {
 		// Single-line sql; run as simple as possible, without noise on stdout.
-		return runStatements(conn, sqlCtx.execStmts, sqlCtx.prettyFmt)
+		return runStatements(conn, sqlCtx.execStmts, cliCtx.prettyFmt)
 	}
 	return runInteractive(conn)
 }

cli/user.go

@@ -49,7 +49,7 @@ func runGetUser(cmd *cobra.Command, args []string) {
 	}
 	defer conn.Close()
 	err = runQueryAndFormatResults(conn, os.Stdout,
-		makeQuery(`SELECT * FROM system.users WHERE username=$1`, args[0]), true)
+		makeQuery(`SELECT * FROM system.users WHERE username=$1`, args[0]), cliCtx.prettyFmt)
 	if err != nil {
 		panic(err)
 	}
@@ -77,7 +77,7 @@ func runLsUsers(cmd *cobra.Command, args []string) {
 	}
 	defer conn.Close()
 	err = runQueryAndFormatResults(conn, os.Stdout,
-		makeQuery(`SELECT username FROM system.users`), true)
+		makeQuery(`SELECT username FROM system.users`), cliCtx.prettyFmt)
 	if err != nil {
 		panic(err)
 	}
@@ -105,7 +105,7 @@ func runRmUser(cmd *cobra.Command, args []string) {
 	}
 	defer conn.Close()
 	err = runQueryAndFormatResults(conn, os.Stdout,
-		makeQuery(`DELETE FROM system.users WHERE username=$1`, args[0]), true)
+		makeQuery(`DELETE FROM system.users WHERE username=$1`, args[0]), cliCtx.prettyFmt)
 	if err != nil {
 		panic(err)
 	}
@@ -177,7 +177,7 @@ func runSetUser(cmd *cobra.Command, args []string) {
 	defer conn.Close()
 	// TODO(marc): switch to UPSERT.
 	err = runQueryAndFormatResults(conn, os.Stdout,
-		makeQuery(`INSERT INTO system.users VALUES ($1, $2)`, args[0], hashed), true)
+		makeQuery(`INSERT INTO system.users VALUES ($1, $2)`, args[0], hashed), cliCtx.prettyFmt)
 	if err != nil {
 		panic(err)
 	}

cli/zone.go

@@ -388,7 +388,7 @@ func runRmZone(cmd *cobra.Command, args []string) error {
 	}
 
 	if err := runQueryAndFormatResults(conn, os.Stdout,
-		makeQuery(`DELETE FROM system.zones WHERE id=$1`, id), true); err != nil {
+		makeQuery(`DELETE FROM system.zones WHERE id=$1`, id), cliCtx.prettyFmt); err != nil {
 		return err
 	}
 	return conn.Exec(`COMMIT`, nil)
@@ -486,10 +486,10 @@ func runSetZone(cmd *cobra.Command, args []string) error {
 	id := path[len(path)-1]
 	if id == zoneID {
 		err = runQueryAndFormatResults(conn, os.Stdout,
-			makeQuery(`UPDATE system.zones SET config = $2 WHERE id = $1`, id, buf), true)
+			makeQuery(`UPDATE system.zones SET config = $2 WHERE id = $1`, id, buf), cliCtx.prettyFmt)
 	} else {
 		err = runQueryAndFormatResults(conn, os.Stdout,
-			makeQuery(`INSERT INTO system.zones VALUES ($1, $2)`, id, buf), true)
+			makeQuery(`INSERT INTO system.zones VALUES ($1, $2)`, id, buf), cliCtx.prettyFmt)
 	}
 	if err != nil {
 		return err