Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Nov 20, 2020

Conversation

erikgrinaker
Copy link
Contributor

@erikgrinaker erikgrinaker commented Nov 17, 2020

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

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
Member

This change is Reviewable

Copy link
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

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
Contributor Author

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
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.
`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.
Copy link
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
Contributor Author

Could you ask bors to merge this please?

@knz
Copy link
Contributor

knz commented Nov 20, 2020

bors r+

@craig
Copy link
Contributor

craig bot commented Nov 20, 2020

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-community Originated from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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