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

Location of prompt gets weird after window change #9319

Open
oluceps opened this issue May 30, 2023 · 15 comments
Open

Location of prompt gets weird after window change #9319

oluceps opened this issue May 30, 2023 · 15 comments
Assignees
Labels
duplicate This issue is a duplicate of another issue and will be consolidated for easier handling line editor Issues related to reedline needs-triage An issue that hasn't had any proper look

Comments

@oluceps
Copy link

oluceps commented May 30, 2023

Describe the bug

This occurs while starting a terminal with nushell in swaywm, then changing the windows, e.g. moving the terminal window from the bottom to the right, Location of the prompt jumps to the y-axis position of the terminal's cursor relative to the screen before the window movement after one keystroke.

Maybe sophisticated to describe, later shown in a video.

Tested with and without enabling starship.

wm: sway 1.8.1
os: NixOS 23.11.20230527.e108023

tested terminal:

  • alacritty 0.12.1
  • foot 1.14.0

bash zsh fish all've no this issue in my environment.

How to reproduce

shows in the video below.

v

The prompt jumps into the bottom after one keystroke.

Expected behavior

Works as other shells.

Screenshots

No response

Configuration

key value
version 0.80.0
branch
commit_hash
build_os linux-x86_64
build_target x86_64-unknown-linux-gnu
rust_version rustc 1.69.0 (84c898d65 2023-04-16) (built from a source tarball)
cargo_version cargo 1.69.0
build_time 1980-01-01 00:00:00 +00:00
build_rust_channel release
features default, zip
installed_plugins

Additional context

nushell configuration:
https://github.com/oluceps/nixos-config/blob/main/home/programs/nu/config.nu
https://github.com/oluceps/nixos-config/blob/main/home/programs/nu/env.nu

@oluceps oluceps added the needs-triage An issue that hasn't had any proper look label May 30, 2023
@fdncred
Copy link
Collaborator

fdncred commented May 30, 2023

I think all these issues are related, and probably more.
#8213
#8935
#9213
#9166
#9164

@amtoine
Copy link
Member

amtoine commented May 30, 2023

this is related to the terminal printing lines not at the same speed as Nushell, as we've already seen during startup? 😮

@sholderbach sholderbach added line editor Issues related to reedline duplicate This issue is a duplicate of another issue and will be consolidated for easier handling labels Jun 6, 2023
@fdncred fdncred mentioned this issue Aug 29, 2023
@albheim
Copy link

albheim commented Sep 21, 2023

Also annoyed by this, and to add to @fdncred's list I found this issue #4906 which also seems related.

@fdncred
Copy link
Collaborator

fdncred commented Sep 21, 2023

We're hoping that someone gets so annoyed that they fix it. That may sound bad, but there's a lot going on and I don't think it's something trivial. Please someone work on this 🙏.

@oluceps
Copy link
Author

oluceps commented Nov 17, 2023

May related nushell/reedline#665

@danielsomerfield
Copy link
Contributor

I'm annoyed enough by it that I'm going to give it a go, but this is a stretch for me. If I can't fix it, I'll at least collect what I can. Here is what I have so far.

  • I believe the issue is ostensibly in reedline
  • when the terminal resizes, nothing happens, but after a key is clicked, handle_resize in impl Painter is called.
  • it tries to recalculate the line height via saturating_sub() and seems to do so incorrectly. For example, if I resize the screen from 39 lines to 118 lines, it does the following:
image

I have yet to figure out why it is set to 817 in the first place or why that causes a reset. I'll look at that next.

It's worth noting, I'm able to test this on any reedline example app. It doesn't require nushell, specifically.

@fdncred
Copy link
Collaborator

fdncred commented Nov 29, 2023

Good luck @danielsomerfield. We'd welcome a fix. It's asked about frequently.

@danielsomerfield
Copy link
Contributor

Ok -- I think I've homed in on it and will re-work the logic a little bit. One problem is that it's setting it to the last row when it should be leaving it exactly where it already is. Another is that it isn't reacting to terminal clear, although it does work if you call clear. I expect there's another event we need to handle, although I'm not 100%.

Unless there is any great objection, I might flip the logic: right now the default is to change the location and it breaks out if it doesn't need to. I think it's a bit more intuitive to do nothing, unless it thinks it DOES need to. I haven't rebuilt all the cases yet but I think I should be able to. It might not be as terse as the current code, but it would be clearer what all the cases are. Ok with that?

Once I have this fixed, I might be able to dig into some of those notes where edge cases are not handled.

@fdncred
Copy link
Collaborator

fdncred commented Nov 30, 2023

sounds like a fine plan

@danielsomerfield
Copy link
Contributor

Hopefully the noise isn't going to drive people to crazy on this issue. Let me know if it does and I'll keep it to myself.

Latest finding: it appears the beahviour between term, iterm2, and kitty are all very different, iterm2, as least with my current configuration is the most dramatic. In particular, what happens when you start up with a clear screen but something in the buffer before the screen has been cleared. They all behave the same with completely empty buffers, it seems, but once their is some state built up, they deviate radically.

I'll try and get as good of a compromise solution as I can to start and work with it for a while across multiple terminal configuration to see if I can figure out some clean common logic.

@fdncred
Copy link
Collaborator

fdncred commented Nov 30, 2023

I like your updates so please keep providing them.

I personally test with probably a dozen different terminals but what may be most appropriate is to optimize for what the community uses based on our last survey, which is listed here https://www.nushell.sh/blog/2023-11-16-nushell-2023-survey-results.html#which-terminal-emulator-do-you-use-with-nushell.

The most popular terminals for working with Nushell are:

Windows Terminal (38.8%)
Alacritty (32.9%)
Wezterm (20%)
gnome-terminal (17.9%)
iterm2 (12.5%)

I wouldn't want to have code that says, if you're on Alacritty, do x and if you're on Wezterm do y, but these should be a good testbed since it's the ones people reported using.

@danielsomerfield
Copy link
Contributor

I can definitely test on the ones with mac ports, but I might need someone to give a try to the windows. I might be able to swing the gnome-terminal if I can dig up some hardware for a linux desktop. Thanks for the reference to popular terms. Very helpful.

@danielsomerfield
Copy link
Contributor

Oh, and feel free to assign this to me if it would help tracking. It might take a week or two, but if that's ok, I'm committed to getting it fixed, if not perfectly, at least to be better than it is now. Up to you though--depends on how you like to manage the issues.

@fdncred
Copy link
Collaborator

fdncred commented Dec 1, 2023

I assigned it to you. Take your time. We're perfectly fine with incremental improvements. It doesn't have to go from broke to perfect in 1 PR.

On my Mac I us UTM and run Ubuntu and Windows in VM.

@danielsomerfield
Copy link
Contributor

The brute force PR seems to be working ok, but here is an edge case. Also looking at performance.

Nu.Shell.Bug.2023-DEC-16.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue is a duplicate of another issue and will be consolidated for easier handling line editor Issues related to reedline needs-triage An issue that hasn't had any proper look
Projects
None yet
Development

No branches or pull requests

6 participants