search

troglobit / editline

A small replacement for GNU readline() for UNIX
https://troglobit.com/projects/editline/
Other
279 stars 58 forks source link

Fix issues about long commands #26

Closed abitmore closed 5 years ago

abitmore commented 5 years ago

This PR fixes a few bugs related to long commands.

Steps to reproduce the bugs:

  1. type a long command, longer than screen/terminal width, so the line is wrapped, do not press enter yet;
  2. press backspace, the first line will show twice (which is unexpected);
  3. press left arrow several times, press backspace or delete key, unexpected behavior;
  4. press left arrow several times, type anything, unexpected behavior;
  5. press enter;
  6. type a short command, press enter;
  7. press up arrow, up arrow, down arrow, down arrow, unexpected behavior.

(can also try commands which are longer than 2 lines.)

The root cause of the bugs is the tty_put('\r') call in reposition(), which doesn't work well when the command is too long. Another issue in reposition() is the use of rl_point in the loop, I don't know whether I'll break anything if change it to rl_end, so in the end I decided to not run into reposition() (as well as clear_line()) at all. The new code is maybe less efficient.

This PR is now ready for review/merge.

abitmore commented 5 years ago

I think this PR is ready for merge, please review.

abitmore commented 5 years ago

I found there is an old line-scroll branch which is related to long commands, the last commit happened 9 year ago, why it hasn't been merged?

troglobit commented 5 years ago

I'll have a look and test your PR later tonight, from a first glance it looks great!

Regarding the old line-scroll branch, I really have no idea why I never merged that. But I guess I never finalized testing of it, and then just forgot about it. I'll have a look at that too, later, unless you beat me to it :)

abitmore commented 5 years ago

Depends on terminal type (I guess, but don't know how to check), I found some strange behaviors when the command is wrapping.

If run BitShares cli_wallet inside tmux, when typing a long command, when the cursor is on the last column, typing one more character doesn't move the cursor to next line although the new character will show; typing one more character again then the cursor will show on next line at the correct position; with this PR applied, if now pressing backspace twice, the cursor behaviors are normal.

If run cli_wallet in a ssh session (from a terminal running in an Ubuntu desktop), when the cursor is on the last column, typing one more character then the cursor disappears;

If run cli_wallet directly from console (virtualbox Ubuntu server), when the cursor is on the last column, typing one more character doesn't move the cursor; typing one more character again then the cursor will show on next line; pressing backspace once, the behavior is normal; pressing backspace again (with this PR applied) the cursor doesn't move to the previous line.

Any idea?

troglobit commented 5 years ago

Ah, I'm starting to remember the issues I had with the line-scroll branch. You're definitely onto something with differences in terminal types, that's what haunted me as well. However, I have no really good idea how to solve it at the moment.

Also, without your PR the behavior for me is ugly, but if I press backspace on a too long line I can at least see the cursor return to the previous line -- ignoring the fact that I have ten duplicate lines above mine. With your PR don't get the ugliness with duplicate lines, but on the other hand the cursor doesn't return so I cannot see where I am. Pressing Ctrl-L refreshes the screen and the line though ...

I think what I wanted to accomplish with my line-scroll branch was to truncate long lines, i.e. move the entire contents to fit within the terminal. Displaying $ at beginning and end, to indicate more to the left or right, similar to how Emacs does when in that line-wrap mode.

So, as things are right now, I don't think I can merge your PR.

abitmore commented 5 years ago

So you encountered the "cursor doesn't return to the previous line" issue as well, probably because the program decided that it should not go back to previous line with the special terminal emulator, so the tty_puts(backspace) call used in the code (actually the tty_flush() call) doesn't move the cursor up. Since tty_flush() finally calls write(), does it mean it's kernel that decided not to move the cursor up? If it's the case, for BitShares project, I guess I can find a way to cheat the kernel. But for editline itself, I think it's good to have a way to set desired behavior, or detect whether write('\b') will work then use a working alternative.

By the way, although the "truncate" behavior would be useful in some cases, personally I don't like it, because it would be inconvenient to copy long commands (with mouse, to another program, which is useful for BitShares). Again, best if the behavior is configurable.

I'll see what I can do.

troglobit commented 5 years ago

Having it configurable would be awesome. Thank you for your understanding, and your effort this far, it's very appreciated!

abitmore commented 5 years ago

Because behavior of write(stdout,'\b') (which is currently used in editline to move cursor backwards) is terminal-dependent, this PR doesn't always work. Without ncurses library, the only way I've found is to always move cursor by the program itself rather than expecting write() to move cursor to our desired position, however, it perhaps require a major refactory of the code of editline. By the way, (I guess) the ncurses way to fix the \b behavior is to enable bw capacity.

troglobit commented 5 years ago

Yeah, we don't want to add fully fledged ncurses support to editline. There are other projects for that ... However, we could always use terminfo and ANSI escape sequences to track the cursor relative to the currently edited line. That was one of the things I was looking to explore with my line-scroll branch.