Ticket #3697 (closed defect: fixed)

Opened 4 months ago

Last modified 6 weeks ago

ctrl+\ (Directory hotlist) broken

Reported by: Mihail Zenkov Owned by: andrew_b
Priority: major Milestone: 4.8.19
Component: mc-tty Version: 4.8.18
Keywords: Cc: retnyg@…, dickey@…, egmont, andrey.gursky@…
Blocked By: Blocking:
Branch state: merged Votes for changeset: committed-master

Description

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

Change History

comment:1 Changed 4 months ago by zaytsev

  • Cc dickey@… added

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!

comment:2 Changed 4 months ago by egmont

  • Cc egmont added

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.

comment:3 Changed 4 months ago by egmont

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

Indeed I can confirm this bug.

comment:4 Changed 4 months ago by 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.

comment:5 Changed 4 months ago by 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:

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

comment:6 Changed 4 months ago by ag

  • Cc andrey.gursky@… added

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

comment:7 follow-up: ↓ 8 Changed 4 months ago by 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.

Last edited 4 months ago by dickey (previous) (diff)

comment:8 in reply to: ↑ 7 Changed 4 months ago by 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.

comment:9 Changed 3 months ago by 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.

comment:10 Changed 2 months ago by andrew_b

  • Owner set to andrew_b
  • Status changed from new to accepted
  • Version changed from master to 4.8.18
  • Branch state changed from no branch to on review
  • Milestone changed from Future Releases to 4.8.19

Branch: 3697_ncurses_bsdcurses
Initial changeset:b77820b9d78862bd21e69f852ba664c876b9d943

comment:11 Changed 2 months ago by zaytsev

  • Votes for changeset set to zaytsev
  • Branch state changed from on review to approved

comment:12 Changed 2 months ago by andrew_b

  • Status changed from accepted to testing
  • Votes for changeset changed from zaytsev to committed-master
  • Resolution set to fixed
  • Branch state changed from approved to merged

Merged to master: [fc3d153b6715d0018e3f83b5fca22a9bf489a39a].

git log --pretty=oneline f3509db..fc3d153

comment:13 Changed 2 months ago by andrew_b

  • Status changed from testing to closed

comment:14 Changed 6 weeks ago by zaytsev

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

Note: See TracTickets for help on using tickets.