mc-wrapper: fish shenanigans #4726

krobelus · 2025-06-28T11:05:08Z

so the only behavior this wrapper adds is that the parent shell will cd to mc's last working directory?
That sounds reasonable I guess, I can try it later.

Yes, some people prefer that the parent shell switch to mc's last working directory after it exits. It's not possible to implement it in any way other than wrapping the mc call with a shell-specific wrapper. We ship such wrappers for sh and csh, which distributions then use. However, there has been no such wrapper for fish until now. This PR adds one.

this works for me, feel free to apply/squash. If it helps I can also push a branch

From f7628b64f0903d2c28ee019ad39b2c90aea06245 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger <aclopte@gmail.com> Date: Sat, 28 Jun 2025 15:33:02 +0200 Subject: [PATCH 1/2] doc/man/mc.1.in: fix typo Signed-off-by: Johannes Altmanninger <aclopte@gmail.com> --- doc/man/mc.1.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/man/mc.1.in b/doc/man/mc.1.in index dc90272ad..c67dbcbcc 100644 --- a/doc/man/mc.1.in +++ b/doc/man/mc.1.in @@ -79,7 +79,7 @@ script that automatically changes the current directory of the shell to the last directory Midnight Commander was in. Source the file .B %pkglibexecdir%/mc.sh (bash and zsh users) or -.B %libexecdir%/mc.csh +.B %pkglibexecdir%/mc.csh (tcsh users) respectively to define .B mc as an alias to the appropriate shell script. -- 2.50.0.rc0.61.gb07857f7dc From 847219b6dc9fbb6bd334f33db7ae98971d03be4b Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger <aclopte@gmail.com> Date: Sat, 28 Jun 2025 14:26:38 +0200 Subject: [PATCH 2/2] updates for fish wrapper - make gitignore and makefile work for all shell wrappers - use $@ instead of repeating target name - add missing fish targets - remove unnecessary quoting (fish does not do word splitting, only line splitting) - fix wrong quoting ("$argv" would join args by spaces into a single arg) - add missing contrib/mc.fish.in; use "function" instead of "alias" since that's more idiomatic for fish. Though "alias mc='source @pkglibexecdir@/mc-wrapper.fish '" would result in pretty much the same function being defined internally. Confirmed it works (but only when I exit mc with F10, not when I type "exit" in its subshell): $ make install $ source /home/johannes/.local/libexec/mc/mc.fish $ type mc mc is a function with definition # Defined in /home/johannes/.local/libexec/mc/mc.fish @ line 1 function mc --description 'Visual shell for Unix-like systems - fish wrapper' source /home/johannes/.local/libexec/mc/mc-wrapper.fish $argv end Signed-off-by: Johannes Altmanninger <aclopte@gmail.com> --- contrib/.gitignore | 5 +++-- contrib/Makefile.am | 33 ++++++++++++++++++--------------- contrib/mc-wrapper.fish.in | 14 +++++++------- contrib/mc.fish.in | 3 +++ doc/man/es/mc.1.in | 4 +++- doc/man/hu/mc.1.in | 6 ++++-- doc/man/it/mc.1.in | 6 ++++-- doc/man/mc.1.in | 4 +++- doc/man/ru/mc.1.in | 4 +++- doc/man/sr/mc.1.in | 4 +++- 10 files changed, 51 insertions(+), 32 deletions(-) create mode 100644 contrib/mc.fish.in diff --git a/contrib/.gitignore b/contrib/.gitignore index bf2c44a93..8aebcef00 100644 --- a/contrib/.gitignore +++ b/contrib/.gitignore @@ -1,4 +1,5 @@ -mc-wrapper.csh -mc-wrapper.sh +mc-wrapper.* +!mc-wrapper.*.in mc.csh mc.sh +mc.fish diff --git a/contrib/Makefile.am b/contrib/Makefile.am index c3a083d11..0f99f3fee 100644 --- a/contrib/Makefile.am +++ b/contrib/Makefile.am @@ -1,8 +1,20 @@ noinst_DATA = README.xterm -SCRIPTS_IN = mc.csh.in mc.sh.in mc-wrapper.csh.in mc-wrapper.sh.in -SCRIPTS_OUT = mc.csh mc.sh mc-wrapper.csh mc-wrapper.sh +SCRIPTS_IN = \ + mc.csh.in \ + mc.fish.in \ + mc.sh.in \ + mc-wrapper.csh.in \ + mc-wrapper.fish.in \ + mc-wrapper.sh.in +SCRIPTS_OUT = \ + mc.csh \ + mc.fish \ + mc.sh \ + mc-wrapper.csh \ + mc-wrapper.fish \ + mc-wrapper.sh pkglibexec_SCRIPTS = $(SCRIPTS_OUT) @@ -17,17 +29,8 @@ EXTRA_DIST = \ $(SCRIPTS_IN) \ $(noinst_DATA) -mc.csh: $(top_builddir)/config.status $(srcdir)/mc.csh.in - $(SED) "s%@""pkglibexecdir@%$(pkglibexecdir)%" $(srcdir)/mc.csh.in > mc.csh +mc.%: $(top_builddir)/config.status $(srcdir)/mc.%.in + $(SED) "s%@""pkglibexecdir@%$(pkglibexecdir)%" $(srcdir)/mc.$*.in > $@ -mc.sh: $(top_builddir)/config.status $(srcdir)/mc.sh.in - $(SED) "s%@""pkglibexecdir@%$(pkglibexecdir)%" $(srcdir)/mc.sh.in > mc.sh - -mc-wrapper.csh: $(top_builddir)/config.status $(srcdir)/mc-wrapper.csh.in - $(SED) "s%@""bindir@%$(bindir)%" $(srcdir)/mc-wrapper.csh.in > mc-wrapper.csh - -mc-wrapper.sh: $(top_builddir)/config.status $(srcdir)/mc-wrapper.sh.in - $(SED) "s%@""bindir@%$(bindir)%" $(srcdir)/mc-wrapper.sh.in > mc-wrapper.sh - -mc-wrapper.sh: $(top_builddir)/config.status $(srcdir)/mc-wrapper.fish.in - $(SED) "s%@""bindir@%$(bindir)%" $(srcdir)/mc-wrapper.fish.in > mc-wrapper.fish +mc-wrapper.%: $(top_builddir)/config.status $(srcdir)/mc-wrapper.%.in + $(SED) "s%@""bindir@%$(bindir)%" $(srcdir)/mc-wrapper.$*.in > $@ diff --git a/contrib/mc-wrapper.fish.in b/contrib/mc-wrapper.fish.in index 3b9168b04..be8d3047e 100644 --- a/contrib/mc-wrapper.fish.in +++ b/contrib/mc-wrapper.fish.in @@ -1,20 +1,20 @@ if set -q MC_TMPDIR - set MC_PWD_FILE (mktemp "$MC_TMPDIR/mc.pwd.XXXXXX") + set MC_PWD_FILE (mktemp $MC_TMPDIR/mc.pwd.XXXXXX) else if set -q TMPDIR - set MC_PWD_FILE (mktemp "$TMPDIR/mc.pwd.XXXXXX") + set MC_PWD_FILE (mktemp $TMPDIR/mc.pwd.XXXXXX) else - set MC_PWD_FILE (mktemp "/tmp/mc.pwd.XXXXXX") + set MC_PWD_FILE (mktemp /tmp/mc.pwd.XXXXXX) end -@bindir@/mc -P "$MC_PWD_FILE" "$argv" +@bindir@/mc -P "$MC_PWD_FILE" $argv if test -r "$MC_PWD_FILE" set MC_PWD (cat $MC_PWD_FILE) - if test -n "$MC_PWD" && test "$MC_PWD" != "$PWD" && test -d "$MC_PWD" - cd "$MC_PWD" + if test -n "$MC_PWD" && test $MC_PWD != $PWD && test -d $MC_PWD + cd $MC_PWD end set -e MC_PWD end -rm -f "$MC_PWD_FILE" +rm -f $MC_PWD_FILE set -e MC_PWD_FILE diff --git a/contrib/mc.fish.in b/contrib/mc.fish.in new file mode 100644 index 000000000..d48ce2a67 --- /dev/null +++ b/contrib/mc.fish.in @@ -0,0 +1,3 @@ +function mc --description 'Visual shell for Unix-like systems - fish wrapper' + source @pkglibexecdir@/mc-wrapper.fish $argv +end diff --git a/doc/man/es/mc.1.in b/doc/man/es/mc.1.in index a4ac7f33a..ff15ecb73 100644 --- a/doc/man/es/mc.1.in +++ b/doc/man/es/mc.1.in @@ -94,7 +94,9 @@ Midnight Commander. Consúltese en los archivos .B %pkglibexecdir%/mc.sh (usuarios de bash y zsh) y .B %pkglibexecdir%/mc.csh -(usuarios de tcsh) la manera de definir +(usuarios de tcsh) +.B %pkglibexecdir%/mc.fish +(usuarios de fish) la manera de definir .B mc como un alias para el correspondiente guión de shell. .TP diff --git a/doc/man/hu/mc.1.in b/doc/man/hu/mc.1.in index b14631c1b..06a57e550 100644 --- a/doc/man/hu/mc.1.in +++ b/doc/man/hu/mc.1.in @@ -61,9 +61,11 @@ helyett a Midnight Commander által utoljára meglátogatott könyvtárra való Fjerdingstad\-nek és Sergey\-nek közreműködésükért). Kérlek, ne csinálj szó szerinti másolatot a funkció beállításairól. A fájlok forrása a .I %pkglibexecdir%/mc.sh -(bash és zsh felhasználóknak), illetőleg a +(bash és zsh felhasználóknak), .I %pkglibexecdir%/mc.csh -(tcsh felhasználóknak) fájl. Ilyenkor, amikor a funkció beállításokat +(tcsh felhasználóknak) illetőleg a +.I %pkglibexecdir%/mc.fish +(fish felhasználóknak) fájl. Ilyenkor, amikor a funkció beállításokat változtatod, a profil értékeket nem szükséges megváltoztatnod, csak arról gondoskodj, hogy az MC\-t ne fordítsd eltérő beállításokkal. .PP diff --git a/doc/man/it/mc.1.in b/doc/man/it/mc.1.in index 7b2591ad0..e1c806b38 100644 --- a/doc/man/it/mc.1.in +++ b/doc/man/it/mc.1.in @@ -82,9 +82,11 @@ utilizzata da una speciale funzione shell che imposti automaticamente l'ultima directory corrente della shell come l'ultima directory in cui stava il Midnight Commander. Prelevate i file .B %pkglibexecdir%/mc.sh -(utenti bash e zsh) o rispettivamente +(utenti bash e zsh), .B %pkglibexecdir%/mc.csh -(utenti tcsh) per definire +(utenti tcsh) o rispettivamente +.B %pkglibexecdir%/mc.fish +(utenti fish) per definire .B mc come un alias allo script di shell appropriato. .TP diff --git a/doc/man/mc.1.in b/doc/man/mc.1.in index c67dbcbcc..3265dbd46 100644 --- a/doc/man/mc.1.in +++ b/doc/man/mc.1.in @@ -80,7 +80,9 @@ the last directory Midnight Commander was in. Source the file .B %pkglibexecdir%/mc.sh (bash and zsh users) or .B %pkglibexecdir%/mc.csh -(tcsh users) respectively to define +(tcsh users) or +.B %pkglibexecdir%/mc.fish +(fish users) respectively to define .B mc as an alias to the appropriate shell script. .TP diff --git a/doc/man/ru/mc.1.in b/doc/man/ru/mc.1.in index 0c0630951..2ae88751e 100644 --- a/doc/man/ru/mc.1.in +++ b/doc/man/ru/mc.1.in @@ -90,8 +90,10 @@ Commander. .IP Для того чтобы эта функция была определена, используйте файл .B %pkglibexecdir%/mc.sh -для оболочек bash и zsh, а для оболочки tcsh соответственно файл +для оболочек bash и zsh, или для оболочки tcsh файл .B %pkglibexecdir%/mc.csh +а для оболочки fish соответственно файл +.B %pkglibexecdir%/mc.fish .TP .I \-s, \-\-slow Включает медленный режим терминала, в котором программа выводит меньше diff --git a/doc/man/sr/mc.1.in b/doc/man/sr/mc.1.in index 98b16c635..2d2db4ce9 100644 --- a/doc/man/sr/mc.1.in +++ b/doc/man/sr/mc.1.in @@ -64,7 +64,9 @@ termcap/terminfo. Корисно је само на ХП\-овим термин .B %pkglibexecdir%/mc.sh (за кориснике љуски bash и zsh) или .B %pkglibexecdir%/mc.csh -(за кориснике љуске tcsh), тим редом, да бисте задали +(за кориснике љуске tcsh) или +.B %pkglibexecdir%/mc.fish +(за кориснике љуске fish), тим редом, да бисте задали .B mc као надимак за одговарајући спис љуске. .TP -- 2.50.0.rc0.61.gb07857f7dc

My bad I pasted the wrong patch at first. Edited the comment above.

OT: both zsh and fish are not POSIX compatible in the default mode but zsh is closer.
I think the fact that fish rejects some basic constructs is an unfortunate historical accident but I guess for interactive use it may be complete enough.

For mc+fish there is a long-standing bug where things hangs due to a race condition; it seems to trigger when prompt computation takes too long. I haven't encountered it myself, but it would be good to fix that, especially if it happens in practice

My bad I pasted the wrong patch at first. Edited the comment above.

No problem! Getting patches attached is actually very convenient. Thank you!

Confirmed it works (but only when I exit mc with F10, not when I type "exit"
in its subshell)

In this case, I think mc doesn't have the opportunity to exit properly because the subshell terminates it before it can write the working directory to a file. I'm not sure if anything can be done about it, but it's definitely not shell-specific.

Speaking of which, is it possible to trap signals with Fish? If mc crashes or exits with a non-zero code, such as when you type exit in the subshell, #1466 comes back with shell wrappers.

Could some code be added to trap signals and remove the temporary directory, no matter how mc exits? Or, something like || true?

I'm also interested in hearing @ossilator's opinion on that. I think he uses the shell wrappers, if I'm not mistaken.

For mc+fish there is a long-standing bug where things hangs due to a race condition

Yes, there was a recent discussion about that in #4597, but it might be the same as #4071, which is not even FISH-specific. There is also #4258 as far as FISH is concerned.

i don't quite understand why the _sub_shell exiting on its own would kill mc (its parent process). isn't this just mc not handling the unexpected eof properly?

i don't quite understand why the _sub_shell exiting on its own would kill mc (its parent process). isn't this just mc not handling the unexpected eof properly?

Press CTRL+D in CTRL-O:

read (subshell_pty...): Resource temporarily unavailable (35)

If you type exit into the subshell:

read (subshell_pty...): Undefined error: 0 (0)

If you type exit into the prompt, mc exits, but I don't know how cleanly.

And yes, I'm not saying it should kill mc, but it is definitely not working properly.

so let's fix it instead of uglifying the wrappers with workarounds.
it's presumably just a single conditional that should handle read() returning zero separately instead of treating <= 0 as an error.

so let's fix it instead of uglifying the wrappers with workarounds. it's presumably just a single conditional that should handle read() returning zero separately instead of treating <= 0 as an error.

I think this is a tangential issue, mc can still exit with a non-zero code for a variety of reasons. This was just one obvious case that came to mind, so I mentioned it.

Of course, I agree that it would be good to have this fixed. However, I couldn't find an issue for it, which is strange. Also, I wonder what the correct behavior should be. Should we try to restart the subshell? Invoking a clean termination procedure?

Pull requests and issues are welcome.

mc can still exit with a non-zero code for a variety of reasons.

i suppose. but if we want to handle that properly, then that code needs to be propagated.

Also, I wonder what the correct behavior should be.

it should probably pop up a dialog asking what to do.
a much worse alternative would be a clean exit with a message to the tty.
a silent termination is bad, because it leaves the user wondering what just happened, esp. when the panels are off ("huh? the shell exited, but it's still there?"), and it may prevent startup without clear indication what's wrong.
auto-respawning may catch the system in a busy loop.

ossilator · 2025-06-29T17:37:30Z

i don't know fish syntax, but the inconsistent use of quotes around variable expansions seems ... fishy.

Yeah I used the "only quote when needed" style.
Every variable in fish is an array variable, so we need to quote it for
test -r "$foo", lest it expand to noe argument. But if we're
guaranteed the variable is non-empty, or if the behavior for "empty
array" is fine, there's no need.

Happy to add it though, if it helps to stay closer to other integrations.

FWIW there are only three things missing from fish that prevent it from using contrib/mc-wrapper.sh.in as-is:

foo=bar for global assignment (foo=bar somecommand already works)

$@

unset (simple wrapper around fish's set -e)
The first two I have implemented in my fork for now.

It's possible to make the wrapper scripts a bit smaller, by farming
out the management of $MC_PWD_FILE to an external program (written
in any language):

# mc-and-echo-pwd.sh MC_PWD_FILE=$(mktemp) mc -P "$MC_PWD_FILE" wd=$(cat "$MC_PWD_FILE") rm -f "$MC_PWD_FILE" if [ -z "$wd" ]; then wd=. fi printf %s "$wd" # mc.sh alias mc='__mcwrapper' __mcwrapper() { cd "$(mc-and-echo-pwd.sh "$@")" } # mc.fish function mc cd (mc-and-echo-pwd.sh $argv) end

nevermind, that shared logic won't work because stdout is captured i.e redirected

Right sorry about that. It is toally unintentional glitch. I can fix that up.

krobelus · 2025-07-05T08:49:54Z

actually there's another issue: pkglibexecdir is not defined on my system, only libexecdir is.
After installing, man mc renders like:

shell to the last directory Midnight Commander was in. Source the file @pkglibexecdir@/mc.sh (bash and zsh users) or /home/johannes/.local/libexec/mc.csh (tcsh users) respectively to define mc as an alias to the

I'm building with

./autogen.sh && ./configure --prefix=$HOME/.local && make -j3 && make install

found it, will attempt a fix

-Original file line number
+Diff line change
@@ Expand Up @@
     mc-wrapper.sh: $(top_builddir)/config.status $(srcdir)/mc-wrapper.sh.in
     	$(SED) "s%@""bindir@%$(bindir)%" $(srcdir)/mc-wrapper.sh.in > mc-wrapper.sh
+    mc-wrapper.sh: $(top_builddir)/config.status $(srcdir)/mc-wrapper.fish.in
+    	$(SED) "s%@""bindir@%$(bindir)%" $(srcdir)/mc-wrapper.fish.in > mc-wrapper.fish

-Original file line number
+Diff line change
@@ -0,0 +1,20 @@
+    if set -q MC_TMPDIR
+    	set MC_PWD_FILE (mktemp "$MC_TMPDIR/mc.pwd.XXXXXX")
+    else if set -q TMPDIR
+    	set MC_PWD_FILE (mktemp "$TMPDIR/mc.pwd.XXXXXX")
+    else
+    	set MC_PWD_FILE (mktemp "/tmp/mc.pwd.XXXXXX")
+    end
+    @bindir@/mc -P "$MC_PWD_FILE" "$argv"
+    if test -r "$MC_PWD_FILE"
+    	set MC_PWD (cat $MC_PWD_FILE)
+    	if test -n "$MC_PWD" && test "$MC_PWD" != "$PWD" && test -d "$MC_PWD"
+    		cd "$MC_PWD"
+    	end
+    	set -e MC_PWD
+    end
+    rm -f "$MC_PWD_FILE"
+    set -e MC_PWD_FILE

-Original file line number
+Diff line change
@@ Expand Up @@
     the last directory Midnight Commander was in. Source the file
     .B %pkglibexecdir%/mc.sh
     (bash and zsh users) or
-    .B %libexecdir%/mc.csh
+    .B %pkglibexecdir%/mc.csh
     (tcsh users) respectively to define
     .B mc
     as an alias to the appropriate shell script.
@@ Expand Down @@

Uh oh!

mc-wrapper: fish shenanigans #4726

Are you sure you want to change the base?

Uh oh!

mc-wrapper: fish shenanigans #4726

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krobelus Jun 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krobelus Jun 28, 2025 •

edited

Loading