Improve fish 4.0 shell support

[zaytsev]

Patches from the mailing list:

v1:

• ​https://lists.midnight-commander.org/pipermail/mc-devel/2024-October/011234.html
• ​https://lists.midnight-commander.org/pipermail/mc-devel/2024-October/011235.html

v2:

• ​https://lists.midnight-commander.org/pipermail/mc-devel/2024-October/011238.html
• ​https://lists.midnight-commander.org/pipermail/mc-devel/2024-October/011239.html
• ​https://lists.midnight-commander.org/pipermail/mc-devel/2024-October/011240.html

[zaytsev]

Yes, I don't like the environment variable thing all... Is it possible for fish to parse our version from mc --version? If this is not practical, maybe we could consider exporting a generic MC_VERSION variable?

[johannes]

I just sent an updated patch series to the mailing list,
to use MC_VERSION instead.
That third patch is not absolutely necessary but it would be nice, to allow users to converge quicklier on the new keyboard input style.

Let me know if there is anything else for me to do; I realize I didn't add a changelog entry; not sure if that's necessary.

[zaytsev]

Could you please just attach them here (git format-patch HEAD~~)? I'm not sure if there are any differences otherwise and it's really painful to save messages and upload. I will remove my first comment.

[johannes]

Ok done.
Indeed git format-patch is exactly what git send-email sends, so running git am will give the same result.
I wasn't familiar with Trac, maybe the "Reporting problems" section in the README should say that it's preferred to submit patches on Trac.
(I also somehow didn't receive an email notification for comments)

[zaytsev]

Thank you! I hope we solve the familiarity problem by dropping Trac one day... instead of adding more info to the README. But good point, need to check what it says now. Not very smart if it currently says to send patches to the mailing list, then ask people to post to Trac on the mailing list.

[johannes]

I personally enjoy using mailing lists but I can see how they are not for everyone. There's an initial barrier to entry that most potential contributors won't cross. Some projects offer multiple avenues for sending patches (for example both Github and a mailing list), I think that's often a good choice

[johannes]

Do you feel like there should be more thought spent on "MC_VERSION"?
If preferred, I can try parsing "mc --version" instead.

[zaytsev]

Do you feel like there should be more thought spent on "MC_VERSION"?

I really don't like exporting it. It would be good to know what Andrew thinks about it...

Regarding lists, it's just an impractical way for us to keep track of patches right now. We need a central place, or things are bound to get lost or forgotten, as is currently the case with mailing lists and GitHub, and currently that central place is Trac. I hope that one day we'll get to GitHub and decommission everything else (Trac), but there's still a lot of work to be done to make that happen.

[ossi]

i suggest emulating what shells do: set the version variable without exporting it. that would imply injecting an additional command early during the subshell setup.

[zaytsev]

Branch: 4597_fish_csi
Initial changeset:0ea77d2ec71d42c28c28573794a4ac95bb3a912a

I like the second option, agree with Andrew's fixups.

[andrew_b]

There is no need to write "Ticket #XXXX:" in the 2nd, 3rd, etc. commits in the branch.

[johannes]

I see, I didn't realize there will already be a merge commit to group commits

[zaytsev]

I will clean up, rebase and merge as soon as CI is in.

[zaytsev]

Rebased on current master so that this branch gets CI and cleaned up as promised.

[zaytsev]

Hmmm, build on macOS breaks:

common.c:185:53: error: initializer element is not a compile-time constant
static const size_t subshell_switch_key_csi_u_len = strlen (subshell_switch_key_csi_u);
                                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[johannes]

Could probably use sizeof(subshell_switch_key_csi_u) - 1 as a workaround.
Actually it seems better to get rid of this constant; it's used twice but one of those uses can be elided by switching from memcmp to strcmp (no other change should be necessary)

[zaytsev]

I've pushed a fixup and it indeed fixed the CI! Actually, this sounds like a compiler bug to me. I can't reproduce it locally with clang 16, but it does fail on macOS inside GitHub. No other platform with clang is failing though (FreeBSD is fine)... oh well, if it works - so be it.

[zaytsev]

Merged to master as fc3ad5322848dba0d6cc180bbea1d2176398f567 .

Thank you everybody!

[johannes]

Thanks! The corresponding change has been merged to fish as well.
LMK if there are more fish-related issues; I don't really use mc myself yet but I'm happy to help.

[zaytsev]

Fantastic, whenever I stumble upon a FISH ticket while triaging, I'll put you on CC. It's always of great help to have someone in the know... Schönes Wochenende!