-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Conversation
aac2daa
to
d5449c8
Compare
There was a problem hiding this 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. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
bors r=ZhouXing19,rafiss |
Build succeeded: |
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 <[email protected]> Co-authored-by: Pavel Kalinnikov <[email protected]>
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