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

clisqlshell: hide the libedit dep behind a go interface #88574

Merged
merged 1 commit into from
Nov 3, 2022

Conversation

knz
Copy link
Contributor

@knz knz commented Sep 23, 2022

In #86457 and related work we will want to offer two editors side-by-side in a transition period, so folk can compare or fall back on something known in case they are not happy with the new stuff.

To enable this transition period, this commit hides the editor behind a go interface.

This also makes the shell code overall easier to read and understand.

Epic: CRDB-22182
Release note: None

@knz knz requested review from rafiss and a team September 23, 2022 15:44
@knz knz requested a review from a team as a code owner September 23, 2022 15:44
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@knz knz force-pushed the 20220923-editor branch 3 times, most recently from aac2daa to d5449c8 Compare September 24, 2022 17:45
@knz knz mentioned this pull request Oct 31, 2022
Copy link
Collaborator

@ZhouXing19 ZhouXing19 left a comment

Choose a reason for hiding this comment

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

Very cool improvement! It makes the code much easier to read.

"github.com/cockroachdb/errors"
)

// bufioReader implements the editor interface.
Copy link
Collaborator

Choose a reason for hiding this comment

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

could we include declarations for the implementing types that assert that they implement the interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Done.

In cockroachdb#86457 and related work we will want to offer two editors
side-by-side in a transition period, so folk can compare or fall back
on something known in case they are not happy with the new stuff.

To enable this transition period, this commit hides the editor behind
a go interface.

This also makes the shell code overall easier to read and understand.

Release note: None
@knz
Copy link
Contributor Author

knz commented Nov 3, 2022

bors r=ZhouXing19,rafiss

@craig
Copy link
Contributor

craig bot commented Nov 3, 2022

Build succeeded:

@craig craig bot merged commit cb5d760 into cockroachdb:master Nov 3, 2022
craig bot pushed a commit that referenced this pull request Dec 1, 2022
86457: cli: replace libedit with bubbline r=ZhouXing19 a=knz

First commit from #88574.
Benefits from (but is not dependent on) #87606 server-side.

Fixes #21826
Fixes #71207
Fixes #71209
Fixes #57885

NB: this will benefit from upstream library releases based off the still-unmerged PRs listed in knz/bubbline#2.

Release justification: n/a will not merge before stability ends

Release note (cli change): The engine used as line editor in the
interactive shell (`cockroach sql`, `demo`) has been updated. It
includes numerous bug fixes and new features.
The previous engine can still be accessed via the env
var COCKROACH_SQL_FORCE_LIBEDIT. This support will be removed
in a later version.

92335: kvserver,logstore: introduce log StateLoader r=tbg a=pavelkalinnikov

Previously the `StateLoader` accessed both log and state machine state. This commit moves most of the log-specific accesstors to the `logstore` package.

Part of #91979
Release note: None

Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Co-authored-by: Pavel Kalinnikov <pavel@cockroachlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants