Skip to content

cli: add \x command to toggle records display format#56829

Merged
craig[bot] merged 2 commits intocockroachdb:masterfrom
erikgrinaker:cli-slash-x
Nov 20, 2020
Merged

cli: add \x command to toggle records display format#56829
craig[bot] merged 2 commits intocockroachdb:masterfrom
erikgrinaker:cli-slash-x

Conversation

@erikgrinaker
Copy link
Copy Markdown
Contributor

@erikgrinaker erikgrinaker commented Nov 17, 2020 •
edited
Loading

psql has a \x command to toggle extended output for results. CRDB
already supports this output format via \set display_format=records.
This commit adds \x [on|off] to toggle between records and table
display formats.

The auto option from psql to automatically enable extended output
depending on the result width is not supported, only on and off.

Resolves #56706.

Release note (cli change): A \x [on|off] command has been added to
toggle the records display format, following psql behavior.

Also adds support for yes and no as boolean options for slash commands as a separate commit, following psql.

@erikgrinaker erikgrinaker requested a review from a team as a code owner November 17, 2020 22:37
@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Nov 17, 2020

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl blathers-crl bot added the O-community Originated from the community label Nov 17, 2020
@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented Nov 17, 2020

This change is Reviewable

knz
knz reviewed Nov 18, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Eric, glad to see your contribution!

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @erikgrinaker)

pkg/cli/sql.go, line 318 at r1 (raw file):

			val = strings.ToLower(strings.TrimSpace(val))
			switch val {
			case "false", "0", "off":

then reuse it here

pkg/cli/sql.go, line 489 at r1 (raw file):

		err = opt.set(val)
	} else {
		switch val {

Can you extract this boolean parsing code into a reusable function

pkg/cli/sql.go, line 1181 at r1 (raw file):

		case 2:
			switch strings.TrimSpace(strings.ToLower(cmd[1])) {
			case "on", "1", "true":

and also here

thanks

@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Nov 18, 2020

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@erikgrinaker
Copy link
Copy Markdown
Contributor Author

erikgrinaker commented Nov 18, 2020

Sure @knz, done! Also added support for yes and no for boolean options, since psql already does this. Split it out as a separate commit.

@erikgrinaker erikgrinaker force-pushed the cli-slash-x branch 2 times, most recently from d4b2008 to 1cd5b84 Compare November 18, 2020 19:07
@erikgrinaker
cli: add yes/no as valid boolean strings for slash commands
5fb712f 
This commit adds a utility function `parseBool` for parsing boolean
strings in SQL shell slash commands, and uses it instead of ad-hoc parsing
in individual commands.

It also adds support for `yes` and `no` as valid boolean values,
following `psql` behavior.

Release note (cli change): The SQL shell now accepts `yes`/`no` as
boolean options for slash commands, following `psql` behavior.
@erikgrinaker
cli: add \x command to toggle records display format
07ecb7f 
`psql` has a `\x` command to toggle extended output for results. CRDB
already supports this output format via `\set display_format=records`.
This commit adds `\x [on|off]` to toggle between `records` and `table`
display formats.

The `auto` option from `psql` to automatically enable extended output
depending on the result width is not supported, only `on` and `off`.

Resolves cockroachdb#56706.

Release note (cli change): A `\x [on|off]` command has been added to
toggle the `records` display format, following `psql` behavior.
knz
knz approved these changes Nov 19, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: thanks

Reviewed 3 of 4 files at r2, 2 of 2 files at r3.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@erikgrinaker
Copy link
Copy Markdown
Contributor Author

erikgrinaker commented Nov 20, 2020

Could you ask bors to merge this please?

@knz
Copy link
Copy Markdown
Contributor

knz commented Nov 20, 2020

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Nov 20, 2020

Build succeeded:

@craig craig bot merged commit 39015a0 into cockroachdb:master Nov 20, 2020
@erikgrinaker erikgrinaker deleted the cli-slash-x branch November 20, 2020 02:03
This was referenced Dec 9, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@knz knz knz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

O-community Originated from the community

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

sql: add \x slash command as alias for \set display_format=records

3 participants

@erikgrinaker @cockroach-teamcity @knz