ctrl+\ (Directory hotlist) broken

[Mihail Zenkov]

ctrl+\ (Directory hotlist) broken on linux. Regression after ​http://www.midnight-commander.org/changeset/38d4c655d322837574e957b4a824f4a0d1bb3b86

[zaytsev]

Hi Thomas,

I can confirm that this patch causes Ctrl+\ to generate SIGQUIT with ncurses on Linux, instead of freeing this key for use in mc. I'm not sure why this is happening; we've got the patch from NetBSD curses maintainer claiming that this is the way to do it to remain compatible both with ncurses and NetBSD curses by relying on the official interface instead of private implementation details.

Could you please be so kind to have a look and maybe tell us how to do it the right way? You are definitively the authority on this issue, and hopefully that would only take you a couple of moments.

Many thanks!

[egmont]

FYI: mc-4.8.18 on Ubuntu Yakkety beta; any of xterm / gnome-terminal / Linux console: Ctrl+\ brings up the directory hotlist dialog for me as expected.

[egmont]

/me stupid. Forget my previous comment. I was building with slang. My bad.

Indeed I can confirm this bug.

[dickey]

Control-backslash is normally the quit character (

stty -a
speed 38400 baud; rows 40; columns 80; line = 0;
intr = ^C; quit = ^\; erase = ^H; kill = ^U; eof = ^D; eol = <undef>;
eol2 = <undef>; swtch = <undef>; start = ^Q; stop = ^S; susp = ^Z; rprnt = ^R;
werase = ^W; lnext = ^V; flush = ^O; min = 1; time = 0;
-parenb -parodd cs8 -hupcl -cstopb cread -clocal -crtscts
-ignbrk -brkint -ignpar -parmrk -inpck -istrip -inlcr -igncr icrnl ixon -ixoff
-iuclc -ixany -imaxbel -iutf8
opost -olcuc -ocrnl onlcr -onocr -onlret -ofill -ofdel nl0 cr0 tab0 bs0 vt0 ff0
isig icanon iexten echo echoe echok -echonl -noflsh -xcase -tostop -echoprt
echoctl echoke

).

ncurses sets up signal handlers during ​initscr/newterm

It sounds as if ncurses does its initialization for this after midnight commander.

[zaytsev]

Hmmmm, the call to initscr() is right above the code in question that tries to override the signal handlers.

Click here to see the functions in full:

• Old: ​http://www.midnight-commander.org/browser/lib/tty/tty-ncurses.c?rev=eecf1fc946eadffdba11c2c6e266d2964ef5aab2#L177
• New: ​http://www.midnight-commander.org/browser/lib/tty/tty-ncurses.c?rev=38d4c655d322837574e957b4a824f4a0d1bb3b86#L177

I would hate it to have to hardcode an ifdef to distinguish ncurses and NetBSD curses :-(

[ag]

Changeset [1] claims:

The code that manipulates the ncurses backend into changing
the key combination to generate SIGINT from CTRL-c to CTRL-g does
so by accessing undocumented internal ncurses data structures.

Is that true? ncurses has curses.priv.h, but the datastructure in question (struct term) and cur_term are in public /usr/include/term.h and I see no comment, that it is only for private use.

[1] ​http://www.midnight-commander.org/changeset/38d4c655d322837574e957b4a824f4a0d1bb3b86

[dickey]

Just reading the ncurses source, the indicated change will not work because in the calls to raw(), etc., ncurses uses the values from cur_term to make updates - not simply the current state of the terminal.

I'm looking at this chunk -

​https://github.com/ThomasDickey/ncurses-snapshots/blob/master/ncurses/tinfo/lib_raw.c#L93

which uses this assignment:

buf = termp->Nttyb;

Agreeing with the comments about private data structures, NetBSD has made private a number of data items which are publicly accessible in other versions of curses, and has added functions for accessing those (but not done a complete job of allowing access - form/menu libraries have problems for portability). I've a page giving my perspective:

​http://invisible-island.net/ncurses/ncurses-netbsd.html

So it looks as if you need an ifdef: to handle NetBSD where it differs from other versions of curses.

[andrew_b]

Replying to dickey:

So it looks as if you need an ifdef: to handle NetBSD where it differs from other versions of curses.

If I understand you correctly, we need something like

#ifdef BSDCURSES
...
#else /* other curses */
...
#endif

not

#ifdef NCURSES
...
#else /* other curses */
...
#endif

ncurses provides the NCURSES_VERSION macro which identifies the ncurses library. I can't find such unique identifier for NetBSD curses.

[zaytsev-work]

So, since there haven't been any feedback from rofl0r so far, my suggestion would be to simply revert this commit and wait until someone comes back with a proper fix.

As it appears, the only viable solution is to introduce conditionals, but I'd be unhappy to put the condition on curses. It would be much better to put a conditional on NetBSD curses, but as Andrew has pointed out, there doesn't seem to be an obvious way to detect it, and if they are uncooperative, we should probably just follow the path of least resistance.

[andrew_b]

Branch: 3697_ncurses_bsdcurses
Initial changeset:b77820b9d78862bd21e69f852ba664c876b9d943

[andrew_b]

Merged to master: [fc3d153b6715d0018e3f83b5fca22a9bf489a39a].

git log --pretty=oneline f3509db..fc3d153

[zaytsev]

Ticket #3743 has been marked as a duplicate of this ticket.

[dickey]

With the upcoming ncurses 6.1, you'll need a different fix, since the enclosing structure is opaque:

diff --git a/lib/tty/tty-ncurses.c b/lib/tty/tty-ncurses.c
index a7a11f368..6e5304ba9 100644
--- a/lib/tty/tty-ncurses.c
+++ b/lib/tty/tty-ncurses.c
@@ -194,14 +194,6 @@ tty_init (gboolean mouse_enable, gboolean is_xterm)
     ESCDELAY = 200;
 #endif /* HAVE_ESCDELAY */

-#ifdef NCURSES_VERSION
-    /* use Ctrl-g to generate SIGINT */
-    cur_term->Nttyb.c_cc[VINTR] = CTRL ('g');   /* ^g */
-    /* disable SIGQUIT to allow use Ctrl-\ key */
-    cur_term->Nttyb.c_cc[VQUIT] = NULL_VALUE;
-    tcsetattr (cur_term->Filedes, TCSANOW, &cur_term->Nttyb);
-#else
-    /* other curses implementation (bsd curses, ...) */
     {   
         struct termios mode;

@@ -212,7 +204,8 @@ tty_init (gboolean mouse_enable, gboolean is_xterm)
         mode.c_cc[VQUIT] = NULL_VALUE;
         tcsetattr (STDIN_FILENO, TCSANOW, &mode);
     }
-#endif /* NCURSES_VERSION */
+    /* curses remembers the "in-program" modes after this call */
+    def_prog_mode();

     tty_start_interrupt_key ();

[andrew_b]

Replying to dickey:

With the upcoming ncurses 6.1, you'll need a different fix, since the enclosing structure is opaque:

Is this fix correct for ncurses < 6.1?

[dickey]

yes, the change should work with any version of ncurses.

[zaytsev]

Reopening so that this doesn't get forgotten. Getting rid of ifdef sounds awesome. Thank you Thomas!

[andrew_b]

Branch: 3697_ncurses_tty_init
changeset:4d46a108629beb66a293672db7b44f863b6598ba

[andrew_b]

Merged to master: [e0d6617fd0c5b08ac44a3310802ff77b881eb034].