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.

Sign up for GitHub

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

Jump to bottom

cli/sql: support \s #98035

Merged
merged 1 commit into from
Mar 8, 2023
Merged

cli/sql: support \s #98035

merged 1 commit into from
Mar 8, 2023

Conversation

lyang24
Copy link
Contributor

@lyang24 lyang24 commented Mar 5, 2023 •
edited by knz
Loading

Fixes: #97575

@lyang24 lyang24 requested a review from a team March 5, 2023 03:47
@lyang24 lyang24 requested a review from a team as a code owner March 5, 2023 03:47
@blathers-crl
Copy link

blathers-crl bot commented Mar 5, 2023

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

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.

I have added a few people who may be able to assist in reviewing:

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-triaged blathers was able to find an owner labels Mar 5, 2023
@blathers-crl blathers-crl bot requested a review from knz March 5, 2023 03:47
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@blathers-crl
Copy link

blathers-crl bot commented Mar 5, 2023

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for updating your pull request.

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 dev-inf.

@Xiang-Gu
Copy link
Contributor

Xiang-Gu commented Mar 5, 2023

Thanks for contributing to CRDB! Please add tests before someone took a closer look at it.

@blathers-crl
Copy link

blathers-crl bot commented Mar 5, 2023

Thank you for updating your pull request.

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 dev-inf.

@lyang24
Copy link
Contributor Author

lyang24 commented Mar 5, 2023

Thanks for contributing to CRDB! Please add tests before someone took a closer look at it.

Hi, thanks for the feedback. I added pkg/cli/interactive_tests/test_print_command_history.tcl to acceptance test. I spend sometime to look at the testing infrastructure since this command is for interactive shell only and most of the test execute with sql -e {statements} which uses another editor with no history. I am having trouble to find the right testing infrastructure? @knz could you please let me know if there are more places i could test this?

@knz
Copy link
Contributor

knz commented Mar 5, 2023

I will look when I am back at the computer later this week.

@knz knz force-pushed the add_history_to_sql_shell branch from cc09ed5 to 3d4d76e Compare March 8, 2023 13:49
knz
knz approved these changes Mar 8, 2023
View reviewed changes
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.

Hello, I have reviewed this and it looks good. The test was to be added to test_history.tcl; I have updated your PR accordingly.

Thanks for your change.

@knz knz changed the title add command line history in sql shell cli/sql: support \s Mar 8, 2023
@knz
cli/sql: support \s
1e6f330 
Release note (cli change): `cockroach sql` and `cockroach demo` now
support the client-side command `\s` to display the previous command
history.

Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
@knz knz force-pushed the add_history_to_sql_shell branch from 3d4d76e to 1e6f330 Compare March 8, 2023 14:08
@knz
Copy link
Contributor

knz commented Mar 8, 2023

bors r+

@craig
Copy link
Contributor

craig bot commented Mar 8, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Mar 8, 2023

Build succeeded:

@craig craig bot merged commit 0fda198 into cockroachdb:master Mar 8, 2023
@cockroach-teamcity cockroach-teamcity mentioned this pull request Mar 9, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@knz knz knz approved these changes

Assignees
No one assigned
Labels
O-community Originated from the community X-blathers-triaged blathers was able to find an owner
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cli/sql: add command to display history
4 participants
@lyang24 @cockroach-teamcity @Xiang-Gu @knz