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

sql shell: confused cursor positioning when editing a long input that spans multiple terminal lines #71209

Closed
knz opened this issue Oct 6, 2021 · 2 comments · Fixed by #86457
Assignees
Labels
A-cli-client CLI commands that pertain to using SQL features B-os-macos Issues specific to macOS. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@knz
Copy link
Contributor

knz commented Oct 6, 2021

reported by @petermattis

Describe the problem

When interactively editing a sql query that spans multiple terminal lines, the cursor positioning gets confused.

To Reproduce

TERM=xterm or TERM=xterm-256color using macOS terminal or iterm:

Kapture 2021-10-06 at 09 31 42

Expected behavior

positioning is correct.

Additional data / screenshots

  • the problem does not occur when using a linux terminal e.g. st.
  • the problem does not occur on macOS when setting TERM to vt100
  • the problem still occurs when logging in from the macOS terminal into a remote linux system
  • the problem still occurs when running through tmux
  • the problem still occurs when running through mosh

Environment:

crdb 21.2

Jira issue: CRDB-10442

Epic CRDB-22182

@knz knz added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. B-os-macos Issues specific to macOS. A-cli-client CLI commands that pertain to using SQL features labels Oct 6, 2021
@petermattis
Copy link
Collaborator

I did some debugging of this today because it still frustrates me. Seems like libedit is just doing something strange. Just before the corruption occurs the line looks like:

root@localhost:26257/management_console_test> select organ from invites where accepted_at is NULL and revoked_at is NULL
 group by

Notice that the first line is exactly 120 characters wide which is exactly the terminal width. I then type an i. In response libedit emits "\r\x1b[59G\x1b[1@i\x1b[120GL \x1b[K\r\x1b[1@L \x1b[A\x1b[60G". Let's break down what these escape codes are doing:

  • \r: this moves the cursor back to the start of the line.
  • \x1b[59G: this is the column_address terminfo command. It moves the cursor to column 59 on the current line which is where the i will be inserted.
  • \x1b[1@i: this is the parm_ich terminfo command which inserts 1 i at the current cursor position, shifting the remainder of the line over by 1. At this point, the screen looks like:
root@localhost:26257/management_console_test> select organi from invites where accepted_at is NULL and revoked_at is NUL
 group by
  • \x1b[120G: another column_address command to move the cursor to the last column on the line.
  • L : insert L and a space at the current cursor position. This has the effect of overwriting the last L on the line, moving the cursor to the next line, and overwriting the space at the beginning of the line. At this point, the screen looks identical to its previous contents:
root@localhost:26257/management_console_test> select organi from invites where accepted_at is NULL and revoked_at is NUL
 group by
  • \x1b[K: This is the clr_eol terminfo command, which clears from the current cursor position to the end of the line. But because we're on the next line at this point, this is what wipes out the line.
root@localhost:26257/management_console_test> select organi from invites where accepted_at is NULL and revoked_at is NUL
 
  • \r\x1b[1@L : move the cursor to the beginning of the line (line 2) and insert an L, followed by a space.
  • \x1b[A: this is the cursor_up terminfo command, which moves the cursor back up to line 1.
  • \x1b[60G: move the cursor to column 60 (of line 2).

I'm not sure what is going on with my terminal or terminfo which is causing this badness. Libedit seems to think that inserting characters at the end of a line won't cause the cursor to move to the next line. It also isn't clear to me why it is trying to insert characters at the end of the line.

I've poked around in the libedit sources to try and understand what is going on. If you want to be scared of some code, take a look at the code in refresh.c:re_update_line.

@petermattis
Copy link
Collaborator

petermattis commented Nov 18, 2021

Disabling parm_ich, parm_dch, enter_insert_mode, and exit_insert_mode in my terminfo "fixes" the problem by disabling all of the insert-mode intelligence in libedit. This likely makes various term display somewhat slower as entire lines have to be redrawn, yet at least the display is accurate.

Recording this for posterity:

~ infocmp -1L -A /Applications/iTerm\ 2.app/Contents/Resources/terminfo | grep -Ev 'parm_dch|parm_ich|enter_insert_mode|exit_insert_mode' > xterm-256color
~ tic -s xterm-256color
1 entries written to /Users/pmattis/.terminfo

craig bot pushed a commit that referenced this issue 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 <[email protected]>
Co-authored-by: Pavel Kalinnikov <[email protected]>
@craig craig bot closed this as completed in #86457 Dec 1, 2022
@exalate-issue-sync exalate-issue-sync bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli-client CLI commands that pertain to using SQL features B-os-macos Issues specific to macOS. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants