reedline icon indicating copy to clipboard operation
reedline copied to clipboard

Fix (properly) the logic around prompt re-use & Host Command handling

Open bew opened this issue 1 year ago • 1 comments

First PR for nushell/reedline \o/

Revert/Rewrites nushell/nushell#758

Fixes nushell/nushell#755 (correctly this time 😬)

Where did the newline actually come from in that issue?

The phantom newline that OP reported was due to this block in painter initialization: (👉 It would detect some text before the cursor and always make a new prompt on the next line..) https://github.com/nushell/reedline/blob/0698712701418a7212ebf75d5724d72eccc4b43f/src/painting/painter.rs#L107-L120

Fixes https://github.com/nushell/reedline/issues/771 (my issue) Now I have the same experience than on my zsh config :smiley:

Supports everything I mentioned in https://github.com/nushell/reedline/issues/771 Supports replacing the prompt (even from multi-line old prompt)

Demo

https://github.com/nushell/reedline/assets/9730330/ab52e593-cc1d-4762-8df8-9e5774538531

MISSING:

  • [ ] Unit tests for the painter initialization logic
  • [ ] Refactor painter initialization logic (to remove that unwrap 👀

What do you think of this implementation?

bew avatar Mar 12 '24 06:03 bew

This example isn't perfect DX/UX, there's a bad case with a multi-line prompt + cursor on one of the first lines + executing command echo; foo-that-outputs.. Like with a 3 lines prompt, cursor on first line, that command would go down 1 line but would overwrite the 3rd line.

A solution would be to use the command commandline set-cursor --end; echo; foo-that-outputs. But I'm thinking we could remove the echo and expose a new flag like commandline set-cursor --below-prompt to place the cursor below all the lines of the prompt (and it's nice & easy to read 🙃)

bew avatar Mar 12 '24 07:03 bew

Hello! I finally went back to finish this PR, it should be ready for review now 🙏

bew avatar Apr 14 '24 10:04 bew

I've played a little bit with this without nushell and it appears to be outputting less ansi escape codes, which is good.

Separately, on my own and unrelated to this PR, I'm trying to figure out how/why reedline prints so many CRLFs in nushell and also how/why the prompt is redrawn with every character typed.

fdncred avatar Apr 14 '24 13:04 fdncred

Hello o/ Is there anything else I can do to get this PR merged? 🤔

bew avatar Apr 22 '24 11:04 bew

there's a bad case with a multi-line prompt + cursor on one of the first lines + executing command echo; foo-that-outputs..

@bew does this 'bad case' still exist?

fdncred avatar Apr 23 '24 12:04 fdncred

there's a bad case with a multi-line prompt + cursor on one of the first lines + executing command echo; foo-that-outputs..

@bew does this 'bad case' still exist?

The 'bad case' I mentioned is only a limitation with the example, due to using 'echo' in the keybind. There is nothing I can do in this PR afaik.

To property solve this limitation with using echo to move cursor below the prompt I suggested the creation of a commandline set-cursor option that would do that, but I think this should be done in a separate PR.

bew avatar Apr 23 '24 14:04 bew

ok, thanks. let's try it because we're a week away from a release. if we don't try it now, it'll have to wait until after the release.

fdncred avatar Apr 23 '24 15:04 fdncred