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

ncspot UI flickers, no progress bar for song remaining, and backspace in ':search' prompt doesn't work #934

Closed
ThomasAdam opened this issue Sep 10, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@ThomasAdam
Copy link

Describe the bug

A recent upgrade to ncspot 0.11.0 rendered the main UI unusable. Although ncspot plays songs just fine, the interface flickers continually. Additionally, there is no progress bar indicator at the bottom of the screen, and when trying to use the backspace key in the :search prompt, that does not work, hence it is not possible to delete anything.

This is all within xterm.

In investigating this, a git bisect between v0.10.1 (the known last good/working version), and v0.11.0 indicates that the following commit is where the regression has occurred:

commit 726ff760918e244ee8d165c10c1af89c7bceb5d6
Author: Nathan White <13190658+eulerfan271@users.noreply.github.com>
Date:   Fri Aug 5 11:39:06 2022 -0700

    switch to termion as defualt backend

diff --git a/Cargo.toml b/Cargo.toml
index faba492..ab41f7b 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -67,7 +67,7 @@ optional = true
 [features]
 alsa_backend = ["librespot-playback/alsa-backend"]
 cover = ["ioctl-rs"] # Support displaying the album cover
-default = ["share_clipboard", "pulseaudio_backend", "mpris", "notify", "pancurses_backend"]
+default = ["share_clipboard", "pulseaudio_backend", "mpris", "notify", "termion_backend"]
 mpris = ["dbus", "dbus-tree"] # Allow ncspot to be controlled via MPRIS API
 notify = ["notify-rust"] # Show what's playing via a notification
 pancurses_backend = ["cursive/pancurses-backend", "pancurses/win32"]

I know the problem will probably be within the termion repository -- and I will be investigating that next, but I'm also going to raise this issue here, in case it's not.

If I use a different terminal (such as alacritty), then everything works as expected (the bug does not appear). However, UI libraries should be intelligent enough to handle this, so the suggestion of "use a different terminal", would be too glib.

I'm also curious to understand why the backend has been changed?

System (please complete the following information):

  • OS: Linux
  • Terminal: xterm
  • Version: 0.11.0
  • Installed from: AUR, but Git also broken
@ThomasAdam ThomasAdam added the bug Something isn't working label Sep 10, 2022
@Bettehem
Copy link
Contributor

Hi, I am able to reproduce the issue. I tested with Xfce Terminal and Alacritty, and the issue doesn't appear. However, when running ncspot inside tmux, the issue appears with both terminals. I was able to record this happening, it looks a bit worse irl but very similar to the recordings:
Alacritty:

2022-09-11.20-16-43.mp4

Xfce Terminal:

2022-09-11.20-18-36.mp4

This didn't happen with the pancurses backend.

@hrkfdn
Copy link
Owner

hrkfdn commented Sep 11, 2022

Looks like these could be related:

@hrkfdn hrkfdn closed this as completed in 63722c5 Sep 11, 2022
@hrkfdn
Copy link
Owner

hrkfdn commented Sep 11, 2022

Hi there, I have pushed a fix that should hopefully help with this. As I can't reproduce this myself it would be cool if you could report if it helps. Thanks! :)

@ThomasAdam
Copy link
Author

Hi @hrkfdn

Yup -- that's fixed this issue, thank you. I still cannot delete any characters when pressing backspace, when in a : prompt, but that's separate to this.

@hrkfdn
Copy link
Owner

hrkfdn commented Sep 13, 2022

@ThomasAdam Hmm, can't really reproduce that backspace issue. Does using a terminal other than xterm also fix this for you?

@ThomasAdam
Copy link
Author

Hey @hrkfdn,

Yes, alacritty works fine. I suspect the reason it fails in xterm is down to cursive or termion not understanding escape sequences correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants