Terminal emulation: missing primary device attribute response (for fish shell)

[krobelus]

Is there an existing issue for this?

• I have searched the existing issues

Midnight Commander version and build configuration

4.8.33

Operating system

Linux

Is this issue reproducible using the latest version of Midnight Commander?

• I confirm the issue is still reproducible with the latest version of Midnight Commander

How to reproduce

fish shell >= 4.1 requires the terminal to implement
the Primary Device Attribute.

For now fish has added a workaround; it will not use this query when MC_TMPDIR (or MC_SID, depending on the exact fish version) is set.

To reproduce, disable this workaround; for example by:

diff --git a/lib/vfs/interface.c b/lib/vfs/interface.c
index 831a0ba946..42d045d730 100644
--- a/lib/vfs/interface.c
+++ b/lib/vfs/interface.c
@@ -783,7 +783,7 @@
 
     tmpdir = g_mkdtemp (buffer);
     if (tmpdir != NULL)
-        g_setenv ("MC_TMPDIR", tmpdir, TRUE);
+        ;
     else
     {
         fprintf (stderr,
diff --git a/src/subshell/common.c b/src/subshell/common.c
index bb7bd2701e..4525592d05 100644
--- a/src/subshell/common.c
+++ b/src/subshell/common.c
@@ -337,16 +337,6 @@
     // FIXME? What about when we re-run the subshell?
     MC_UNUSED const int ret_chdir = chdir (mc_config_get_home_dir ());
 
-    // Set MC_SID to prevent running one mc from another
-    mc_sid = getsid (0);
-    if (mc_sid != -1)
-    {
-        char sid_str[BUF_SMALL];
-
-        g_snprintf (sid_str, sizeof (sid_str), "MC_SID=%ld", (long) mc_sid);
-        putenv (g_strdup (sid_str));
-    }
-
     switch (mc_global.shell->type)
     {
     case SHELL_BASH:

Then run mc like

SHELL=$(command -v fish) FISH_DEBUG=reader FISH_DEBUG_OUTPUT=/tmp/fish.log mc

Expected behavior

mc should respond to the Primary DA query, meaning that fish will not hang for two seconds.

As suggested by @egmontkob, mc should maybe be transparent to the shell,
i.e. forward requests and responses to and from the parent terminal. This is how shpool works.

Alternatively, mc could build its own terminal emulation (like tmux) but that probably makes less sense.

Actual behavior

This will hang for two seconds and the resulting /tmp/fish.log will contain a warning:

warning: fish could not read response to Primary Device Attribute query after waiting for 2 seconds.

Additional context

No response

[egmontkob]

Last night I was thinking about the patch in my pending PR #4784 which changes it not to send Ctrl-C while the shell is starting up... and I don't really like it, I think there's still a feed_subshell() (a quite badly named method btw) running there, meaning a user keypress can still mess up things.

I'd really like to include in my current work (or rather: let that PR land before its really gets out of hand, and follow up with) a proper solution to #4253, #4789, and this one here. I think these three issues are essentially the same.

Instead of feeding commands to the shell (pretending that the user typed them from stdin), we should just pass our init file as an rc file to the shell, continuing with the shell's real rc file. This is especially easy with fish as it has a dedicated cmdline option for this. While doing this, mc would be in the exact same mode as when it's running a command issued from its cmdline underneath the panels.

Gotta refresh my memories what exactly was the problem, how emitting that Ctrl-C fixed a race in the persistent cmdline buffer audotedection. But I think I'm getting reasonably close to understanding the big picture and being able to fix every related issue at once. Fingers crossed. Will try it hopefully next week.

@mc-worker @zyv What are your plans for the next release? There hasn't been one for a long while. This one's going to be a somewhat significant architectural change, I'd like it to remain unreleased (receiving internal testing) for at least a month, rather two or three. If you wish to have a release sooner then I'd like it to land after that, for 4.8.35.

[egmontkob]

Alternatively, mc could build its own terminal emulation (like tmux) but that probably makes less sense.

Yup that's a hard "no" from me. Even just recognizing nothing more than one particular escape sequence (DA1) is a pretty strong "no".

[zyv]

@mc-worker @zyv What are your plans for the next release? There hasn't been one for a long while. This one's going to be a somewhat significant architectural change, I'd like it to remain unreleased (receiving internal testing) for at least a month, rather two or three. If you wish to have a release sooner then I'd like it to land after that, for 4.8.35.

We normally make two releases per year. This time, for the first time in decades, I was able to get some infrastructure work done full time in spring, but then got totally overwhelmed (and still am overwhelmed) in summer/fall, so I think the only option is just to skip the "summer" one and do the "winter" one this time.

My thinking was to start working on a new release by Christmas and put out an RC sometime around New Year, followed by a final release in a month or so. Hopefully, after the move to GitHub, the RC can get more testing than usual. I will be away in December, so no chance of squeezing it earlier anyways.

There are some things I wished I'd do for the release, most importantly sorting out the GID/UID thing for the VFS (do we want to always use 0/0, or UID/GID, or symbolic names?) and removing --enable-subshell, but this simply ain't gonna happen. Barring that, there is only one "hard" requirement in my eyes, that is to fix distcheck, and I think I will be able to do that.

Otherwise, there have been more changes than there have been in years, so I think it's actually good to take as much possibly breaking stuff in as possible. We could even think of doing a 4.9.0 to celebrate the demise of Trac. How about that? On top of this, I think the stuff you all are working on now counts as very major to me.

[egmontkob]

We could even think of doing a 4.9.0 to celebrate the demise of Trac. How about that? On top of this, I think the stuff you all are working on now counts as very major to me.

Ever since the Linux kernel decided to go from x.20 to (x+1).0, you don't need a reason to bump the non-smallest component :)

But, this time, I wouldn't just go for 4.9 at least without wider testing. There's just too much (especially if I get to do the refactoring of this ticket), it's too risky that we break something.

We could do a 4.9.0-pre1 and based on feedback a 4.9.0 a few months later. Or just a 4.8.34 and then a follow-up 4.8.35 (if necessary) a short time afterwards. And call it 4.9.0 when we're quite certain we didn't make a blunder.

I'd be happy with just continuing 4.8.x. After all, the user won't see these as major changes, from the user's view these are just minor bugfixes (most users won't even be affected by them). And, honestly, while the fixes and improvements we're making are important, they are not really that major (IMHO), and they are just a series of patchwork rather than significant internal cleanups.

I guess a switch to glib main loop could really justify 4.9 :)

So, I slightly tend towards staying on 4.8.x, but you two with Andrew are the bosses, I'd let you decide. Just give it at least one tarball release (4.8.34 or 4.9.0-pre1) before calling it 4.9.0, please.

[zyv]

I guess a switch to glib main loop could really justify 4.9 :)

That could justify 4.10, and ECMAScript engine for 5.0 ;-) Rust re-write for 6.0. Let's keep dreaming.

So, I slightly tend towards staying on 4.8.x, but you two with Andrew are the bosses, I'd let you decide. Just give it at least one tarball release (4.8.34 or 4.9.0-pre1) before calling it 4.9.0, please.

Sure, 4.9.0-pre1 and then 4.9.0, this is what I meant by RC.

[mc-worker]

I'd be happy with just continuing 4.8.x. After all, the user won't see these as major changes, from the user's view these are just minor bugfixes (most users won't even be affected by them). And, honestly, while the fixes and improvements we're making are important, they are not really that major (IMHO), and they are just a series of patchwork rather than significant internal cleanups.

I agree. We don't have significant changes for 4.9.

BTW, do you remember that last release in 4.5.x was 4.5.55?

[zyv]

I'd be happy with just continuing 4.8.x. After all, the user won't see these as major changes, from the user's view these are just minor bugfixes (most users won't even be affected by them). And, honestly, while the fixes and improvements we're making are important, they are not really that major (IMHO), and they are just a series of patchwork rather than significant internal cleanups.

I agree. We don't have significant changes for 4.9.

I'm arguing that infrastructure work and its consequences are significant, and on top of that, all of the terminal and subshell fixing. But actually, we don't even need to justify it these days, just as stated above... and I want to do it.

BTW, do you remember that last release in 4.5.x was 4.5.55?

During the 4.5.x cycle they made these 55 releases in 3 years, so more than one release per month! Do you remember that at that time 1 USD was worth about 30 RUB? ;-) We need to adjust for the inflation.

[egmontkob]

There's ongoing work (and a proof of concept branch) in #4789.

fish shell >= 4.1 requires the terminal to implement the Primary Device Attribute.

With those changes, during the subshell's startup mc will forward fish's output to the host terminal, and the host terminal's response back to fish (i.e. effectively support any terminal querying escape sequence).

Later, during interactive usage of the terminal, mc will sometimes forward them and sometimes not, as the pretty complex logic of subshell interaction requires it.

Will this be good enough for fish? The device attributes aren't really expected to change during a terminal's lifetime, so I really hope fish only queries them once during startup.