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

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

Closed
bew opened this issue Mar 9, 2024 · 10 comments · Fixed by #770
Closed

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

bew opened this issue Mar 9, 2024 · 10 comments · Fixed by #770

Comments

@bew
Copy link
Contributor

bew commented Mar 9, 2024

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

  1. Setup a keybinding with:
     {
       name: git_status
       mode: [emacs vi_insert vi_normal]
       modifier: Alt keycode: Char_g
       event: {
         send: ExecuteHostCommand
         cmd: "git status"
       }
     }
  2. 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

nushell.cmd.output.in.keybind.hidden.mov

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
Copy link
Member

This seems related

@bew
Copy link
Contributor Author

bew commented Mar 9, 2024

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
Copy link
Contributor

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. nushell/nushell#12045 (comment)), users change the cursor position by commandline but do not want to display a new prompt.

@matcls
Copy link

matcls commented Mar 11, 2024

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
Copy link
Contributor Author

bew commented Mar 11, 2024

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
Copy link
Member

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
Copy link
Contributor Author

bew commented Mar 11, 2024

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
Copy link
Member

sholderbach commented Mar 11, 2024

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

self.executing_host_command = false;
would be the position to introduce a slightly altered cursor check (just initialize_prompt_position was not working before #758)

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

@bew
Copy link
Contributor Author

bew commented Mar 11, 2024

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
Copy link
Contributor Author

bew commented Mar 12, 2024

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

@sholderbach sholderbach transferred this issue from nushell/nushell Mar 12, 2024
@sholderbach sholderbach changed the title 0.91.0 does not respect cursor position after host cmd exec in a keybinding 0.30.0 (Nu 0.91.0) does not respect cursor position after host cmd exec in a keybinding Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants