Merge pull request #7268 from knz/format-cli

cli: use pretty-formatted tables only with terminals or --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,6 +61,11 @@ var cockroachCmd = &cobra.Command{
 	Long: `CockroachDB command-line interface and server.`,
 }
 
+// isInteractive indicates whether both stdin and stdout refer to the
+// terminal.
+var isInteractive = isatty.IsTerminal(os.Stdout.Fd()) &&
+	isatty.IsTerminal(os.Stdin.Fd())
+
 func init() {
 	cockroachCmd.AddCommand(
 		startCmd,

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
 
@@ -728,23 +729,31 @@ func Example_user() {
 	defer c.stop()
 
 	c.Run("user ls")
+	c.Run("user ls --pretty")
+	c.Run("user ls --pretty=false")
 	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 |
 	// +----------+
 	// +----------+
 	// (0 rows)
+	// user ls --pretty=false
+	// 0 rows
+	// username
 	// user set foo --password=bar
 	// INSERT 1
-	// user ls
+	// user ls --pretty
 	// +----------+
 	// | username |
 	// +----------+
@@ -753,7 +762,7 @@ func Example_user() {
 	// (1 row)
 	// user rm foo
 	// DELETE 1
-	// user ls
+	// user ls --pretty
 	// +----------+
 	// | username |
 	// +----------+
@@ -850,10 +859,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 |
 	// +----+
@@ -904,13 +918,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,16 @@ 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.
+		f.BoolVar(&cliCtx.prettyFmt, cliflags.PrettyName, isInteractive, 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()

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