sabotage-linux / netbsd-curses

libcurses and dependencies taken from netbsd and brought into a portable shape (at least to musl or glibc)
Other
147 stars 14 forks source link

nnn with netbsd-curses #46

Closed jarun closed 3 years ago

jarun commented 3 years ago

Hi,

We are trying out nnn with netbsd-curses and musl libc.

The performance results are just awesome!

Seeing the issue https://github.com/jarun/nnn/issues/998 because of the following difference between ncursesw and netbsd-curses:

In a case where ncursesw sends KEY_RESIZE (key value 410) twice, netbsd-curses sends a KEY_RESIZE (key value 512) followed by a NULL (key value 0) character.

nnn uses the NULL key (^Space) for range selection leading into issues.

Can you please take a look why the NULL key is sent after the KEY_RESIZE?

Attached the debug logs from ncursesw and netbsd-curses. A diff would show the problem.

Please let us know if you need some more details.

nnndbg_ncurses.txt nnndbg_netbsd.txt

jarun commented 3 years ago

While we have made changes in nnn to discard the keybind ^Space which also sends a 0 value, it would be great if this can be fixed in netbsd curses to make the bahaviour uniform with ncursesw.

jarun commented 3 years ago

Also, is there any macro available to identify the netbsd curses library?

rofl0r commented 3 years ago

hi @jarun , i just finished backporting 2 years worth of commits from upstream. could you test whether that fixes your issue ? if not we need probably to report this issue to netbsd mailing list and adopt their fix (or debug the issue ourselves, if you feel up to it).

is there any macro available to identify the netbsd curses library?

fortunately not, altough in the commits i backported are 2 that introduced a curses_version function and another one that introduced a version macro for internal usage, but it was then reverted in 2bd12392015125db1bd0e6889a7e2bde892e782a (with a link to discussion on mailing list). i used to have a backchannel on IRC to one of their developers, but he is sadly currently not online due to health issues.

in regard to curses macro, i say fortunately because having a macro would just lead to people hardcoding wrong assumptions with ifdefs into their sources, instead of having proper configure-style checks for functionality. this is also the reason why there's no MUSL macro. i've written extensively about this in https://sabotage-linux.neocities.org/blog/5/.

jarun commented 3 years ago

Unfortunately, it's still not fixed. However, the patch from nnn hides it.

The latest issue I see is https://github.com/jarun/nnn/issues/998#issuecomment-839707452.

And I couldn't figure it out yet. In case of some KEY_RESIZE events, the statusbar gets broken. We use a move(LINES - 1, 0) to go to the last line. However, it doesn't work in some cases as you will see in the asciicast.

If I call move() in a loop for each line the statusbar goes down. But when the issue happens it's still 2 lines above the last line.

Please see if you can do something about it.

rofl0r commented 3 years ago

since it happens with a specific terminal, it might be something stupid related to terminfo. if you dont mind you could update terminfo/terminfo with latest from ncurses, rebuild all of ncurses; this could fix at least the statusbar issue. i didn't update terminfo yet as there's an ever-growing mismatch between ncurses version, the one used by netbsd, and ours. i'll probably resort to import vanilla ncurses into the repo, and apply a number of custom patches during build process to create the binary version that's baked into libterminfo from.

jarun commented 3 years ago

terminfo/terminfo with latest from ncurses

Can you please point me to the repo?

jarun commented 3 years ago

If you are indicating this: https://ftp.gnu.org/pub/gnu/ncurses/ I am not sure if it compiles with musl libc. The reason I wanted to try netbsd-curses is it plays well with musl libc.

jarun commented 3 years ago

With the terminfo from ncurses-6.2.tar.gz that issue seems to be fixed. However, I see one small problem - the cursor at the filter prompt (bottom line) does not move:

jarun commented 3 years ago

Sorry, even the issue persists if I open nnn from tmux.

rofl0r commented 3 years ago

yeah i meant terminfo file from ncurses or directly from https://invisible-island.net/datafiles/current/terminfo.src.gz (latest). so you're saying using latest terminfo, the issue with statusbar is fixed in xfce4-terminal, but not in tmux ?

jarun commented 3 years ago

No, the original issue persists. And I see the new issue of the block cursor not moving.

jarun commented 3 years ago

The cursor not moving issue is probably introduced by one of the latest changes you pulled in yesterday.

Update: Confirmed. I tested with netbsd-curses v0.3.1 and the stuck cursor issue is not there.

So probably we should just concentrate on the issue I mentioned in https://github.com/sabotage-linux/netbsd-curses/issues/46#issuecomment-840065258. It's present in v0.3.1 as well.

jarun commented 3 years ago

Please let me know if there is any provision to enable debug logs which I can provide.

I was planning to generate a static binary in the next release but this issue is annoying enough to turn off users who use the detached edit in a tabbed terminal. So we need to get it fixed first before releasing such a binary. Also, it appears to me a well-tested new netbsd-curses release would be a good confidence booster.

Would it be possible to merge upstream netbsd changes soon?

rofl0r commented 3 years ago

The cursor not moving issue is probably introduced by one of the latest changes you pulled in yesterday.

could you try to git bisect to find the commit that introduced it ?

Would it be possible to merge upstream netbsd changes soon?

that's precisely what i did yesterday. we're currently even with upstream.

jarun commented 3 years ago

Commit 2b217e5f9e3f2cd3aae8ff66f17c54671b11e64c.

commit 2b217e5f9e3f2cd3aae8ff66f17c54671b11e64c
Refs: [HEAD]
Author:     blymn <blymn@NetBSD.org>
AuthorDate: Mon May 20 22:17:41 2019 +0000
Commit:     rofl0r <rofl0r@users.noreply.github.com>
CommitDate: Wed May 12 15:11:49 2021 +0100

    Back out incorrect fix for PR 53617 and fix it in a different way.
rofl0r commented 3 years ago

cool, thanks. meanwhile i tried to reproduce your issue, you say:

keep opening and closing a text file detached in another tab again and again, the window breaks.

since i never used nnn, i don't know 1) how to open a text file (ENTER does nothing on e.g. /etc/hosts) 2) how to detach a tab 3) how to switch a tab

jarun commented 3 years ago

Are you using xfce4-terminal? It would be easier with that.

Try the step 2 here: https://github.com/jarun/nnn#quickstart

I have export VISUAL=ewrap.

Contents from ewrap:

#!/usr/bin/env sh

xfce4-terminal --tab -e "vim \"$*\""

Then open xfce4-terminal, fire nnn and navigate to a text file, press Enter to open it in another tab in vim. Then press ZQ to close it. Try it a few times and hopefully you'll see the issue.

I generally use xfce4-terminal in drop-down mode. But now I tried this in standalone and it seems LINES is reduced each time I close the text file. Seems to be the same problem to me. Attached a video for you to visualize the problem better. Check the bottom of the terminal each time vim is closed.

https://user-images.githubusercontent.com/5959286/118147682-75558900-b42d-11eb-9cb3-56ef04b1784e.mp4

rofl0r commented 3 years ago

i hoped i could reproduce this with lxterminal which is based on the same terminal emulator library, but unfortunately it lacks --tab option. using tmux inside lxterminal with ewrap as tmux split-window -h "nano \"$*\"" (i don't have vim installed) everythings works just fine. no issues with cursor, or statusbar, or LINES whatsoever, after i close the tmux splitted window. i suspect the root cause of your problem could be the interaction between terminal compiled against ncurses, tmux compiled against ncurses, but nnn compiled against netbsd-curses. the only visual issue i can identify is that while tmux has the editor window selected, the contents of the nnn pane disappear until i switch to that pane.

here's my nnn binary https://0x0.st/-McS.xz (netbsd-curses master, libedit, musl etc static) and my tmux binary https://0x0.st/-Mcj.xz (netbsd-curses 0.3.1, musl, static)

// edit: using nnn 4.0 // edit: oh btw, next time you post screenies, asciinema, videos, pleas resize height of your terminal, my screen is to small to display it entirely.

jarun commented 3 years ago

In the example I am not using tmux... just running nnn within xfce4-terminal.

jarun commented 3 years ago

And I can see the issue with your binary as well. Again, no tmux.

rofl0r commented 3 years ago

so this should be reproducible if i just run VISUAL=nano bin/nnn -e and open/close a couple files in lxterminal ?

jarun commented 3 years ago

I don't know. You mentioned lxterminal doesn't have tabs. So I guess not. Please do not introduce irrelevant parameters which will make the issue more complicated to reproduce.

Any decent player should resize videos according to the screen size. Anyway, please watch the video when you come across a larger screen.

Trying to explain this in brief: when I press Enter on the text file, xfce4-terminal opens it in a new tab (in vim). When I close vim, the new tab is also closed and I see the issue in the original tab where nnn is still running. I am sorry but I don't know any other way to explain it more clearly.

rofl0r commented 3 years ago

did you try to use getmaxyx(stdscr, row, col) instead of using LINES and COLS ?

rofl0r commented 3 years ago

Any decent player should resize videos according to the screen size. Anyway, please watch the video when you come across a larger screen.

i watched the video inside browser, same as your casts over at nnn issue tracker, but i have to constantly scroll up and down as you seem to use terminal with lines set to 60% more than what fits into browser window. i did notice that your terminal became smaller whenever you closed a file in vim.

jarun commented 3 years ago

With the following change I see very erratic behaviour:

diff --git a/src/nnn.c b/src/nnn.c
index 91b97eb..989606c 100644
--- a/src/nnn.c
+++ b/src/nnn.c
@@ -5783,8 +5783,9 @@ static void draw_line(char *path, int ncols)

 static void redraw(char *path)
 {
-       xlines = LINES;
-       xcols = COLS;
+       //xlines = LINES;
+       //xcols = COLS;
+       getmaxyx(stdscr, xlines, xcols);

        int ncols = (xcols <= PATH_MAX) ? xcols : PATH_MAX;
        int onscreen = xlines - 4;

Each time I close vim the window size increases/decreases (instead of steadily decreasing as earlier).

rofl0r commented 3 years ago

Each time I close vim the window size increases/decreases

does that mean it first increases by 1, then jumps back to where it was ? at this point this sounds pretty much like an xfce4-terminal bug to me, why would a terminal emulator even start to resize itself unless the user dragged it around manually?

jarun commented 3 years ago

Yes, that appears to a be an xfce4-terminal issue. I could repro it with ncursesw as well.

And yes, leaving that aside, it seems the issue is now fixed with getmaxyx(stdscr, xlines, xcols). :+1:

jarun commented 3 years ago

Thank you for your time! Closing the defect.

jarun commented 3 years ago

Please notify when the issue introduced in https://github.com/sabotage-linux/netbsd-curses/issues/46#issuecomment-840606629 is fixed.

rofl0r commented 3 years ago

Please notify when the issue introduced in #46 (comment) is fixed.

well we need someone who sends a mail to the netbsd mailing list and reports the issue. i'm not an email user myself and my IRC backchannel to said developer is currently onhold until he's cured.

jarun commented 3 years ago

@rofl0r good news!

The broken cursor issue appears to be fixed with https://github.com/jarun/nnn/commit/f4f6919c02d46889800e74f70a82ef5ef80b9e46.

rofl0r commented 3 years ago

cool, thanks for info :+1: