0.30.0 (Nu 0.91.0) does not respect cursor position after host cmd exec in a keybinding

bew commented 8 months ago

Describe the bug

In zsh I'm used to have a number of keybindings that run commands so I don't need to type them as they are so common, especially some git commands.

In nushell I previously (in 0.89.0) was able to do something like this by simply executing a command and the prompt would continue where it was, but with 0.91.0 the output of my command is cleared and prompt is reset to where it was before. ☹

How to reproduce

Setup a keybinding with:

{
  name: git_status
  mode: [emacs vi_insert vi_normal]
  modifier: Alt keycode: Char_g
  event: {
    send: ExecuteHostCommand
    cmd: "git status"
  }
}

Open a nushell, go anywhere, use the Alt-g keybind to run git status

Expected behavior

I expected nu to continue working as it was before, allowing me to run commands by keybindings and see its output

Screenshots

https://github.com/nushell/nushell/assets/9730330/ce3b9ffe-7fe4-446f-a309-8ef1f588fdcd

Configuration

key	value
version	0.91.0
branch
commit_hash
build_os	linux-x86_64
build_target	x86_64-unknown-linux-gnu
rust_version	rustc 1.76.0 (07dca489a 2024-02-04) (built from a source tarball)
cargo_version	cargo 1.76.0
build_time	1980-01-01 00:00:00 +00:00
build_rust_channel	release
allocator	mimalloc
features	default, sqlite, trash, which, zip
installed_plugins

Additional context

No response

sholderbach commented 8 months ago

This seems related

to this change https://github.com/nushell/reedline/pull/758
to resolve https://github.com/nushell/reedline/issues/755 which is another reasonable expectation for how reedline should behave under ExecuteHostCommand

bew commented 8 months ago

Hmm.. To me this other issue is not conflicting with mine, if the command doesn't output anything it should definitely avoid printing a new prompt, but in my case the command did output stuff so there should be a new prompt below that output 🤔

I think the correct logic, that would work for both cases would be:

Position the cursor on the line after current prompt (so if there is some output it is written below current prompt)
Save this position as the initial cursor position
Run the host command
Compare the final cursor position to the initial cursor position:
if positions are the same:
    Assume there was no output and re-use the previous prompt
else: (positions are different)
    Don't re-use the previous prompt and make a new one
    (might need to add a newline to ensure it starts at the beginning of the line)

NOTE: this is the kind of behavior I have in my zsh setup, except zsh doesn't even bother with saving/comparing positions and I simply have to manually print a newline before running my cmd so the output is below current prompt

kit494way commented 8 months ago

I simply have to manually print a newline before running my cmd so the output is below current prompt

I think it is better to print a newline manually. In some cases (e.g. https://github.com/nushell/nushell/issues/12045#issuecomment-1976711746), users change the cursor position by commandline but do not want to display a new prompt.

matcls commented 8 months ago

the issue is that the output of the command is cleared, when it should be shown and stay then a new prompt should be printed Something I noted is that after scrolling a full page in the terminal it doesn't act this way

bew commented 8 months ago

Something I noted is that after scrolling a full page in the terminal it doesn't act this way

This is because when the prompt is at the very bottom of the terminal, all output will scroll the window, and the final prompt will be at the same position as before on the terminal grid (but not on the scrollback but that's not taken into account).

sholderbach commented 8 months ago

I think there are two valid usecases to cater to:

automate a regular command execution for which you expect to see the output and then return to reedline (whatever you typed is still there)
- preserve the output
execute some code/tool and return to reedline to jury rig extra features onto reedline (e.g. launching your own fzf magic, change some config)
- when returning to reedline, we don't need the output, don't keep the output.

As output from other tools is outside our control in reedline, detecting what we should do is either a somewhat heuristic endeavour. Alternatively we could split those two usecases to different bindings.

if the command doesn't output anything it should definitely avoid printing a new prompt, but in my case the command did output stuff so there should be a new prompt below that output 🤔

I think the correct logic, that would work for both cases would be:
Position the cursor on the line after current prompt (so if there is some output it is written below current prompt)
Save this position as the initial cursor position
Run the host command
Compare the final cursor position to the initial cursor position:
if positions are the same:
    Assume there was no output and re-use the previous prompt
else: (positions are different)
    Don't re-use the previous prompt and make a new one
    (might need to add a newline to ensure it starts at the beginning of the line)

bew commented 8 months ago

Note that for me in some cases I don't know in advance if the command is going to to output something or not. For example with the less pager (used by some automated git commands) I'd usually just quit it and go back to my previous prompt, but other times I want to refer to some things in a command and I'd use a less keybind to quit while outputing the current page, and have my prompt be below this output. (on my phone at the moment, but I can show you later if you want)

In this case the heuristics like I mentioned would be the only way afaik 🤔

sholderbach commented 8 months ago

Mhh if we don't want to introduce the complexity of having two different bindings

https://github.com/nushell/reedline/blob/0698712701418a7212ebf75d5724d72eccc4b43f/src/engine.rs#L675 would be the position to introduce a slightly altered cursor check (just initialize_prompt_position was not working before https://github.com/nushell/reedline/pull/758)

But maybe your case with the pager either going into alternate screen or writing directly may be an argument for separating that?

bew commented 8 months ago

But maybe your case with the pager either going into alternate screen or writing directly may be an argument for separating that?

Just for clarification, my pager always goes into alternate mode It's just that when exiting I usually just exit normally (back to previous prompt) and sometimes I use a special exit that prints the last page to the normal output (after exiting alternate mode), and I don't really know in advance if I'm gonna do one or the other. (depends on the situation, what I'm doing, what I'm looking at, what I'm gonna do after...)

bew commented 8 months ago

Can someone transfer this issue to reedline's repo? I have a PR coming to fix this properly

nushell / reedline