Re-add support for fish shell #19984
Conversation
| end | ||
| nothing | ||
| end | ||
|
|
||
| function shell_wrap_true(shell_name, cmd) | ||
| if shell_name == "fish" | ||
| "begin; $(shell_escape(cmd)); true; end" |
There was a problem hiding this comment.
wouldn't it need to be begin; $(shell_escape(cmd)); and true; end to be the same in both cases?
|
Would be nice to have some tests |
|
This will go well with #19786 (comment), has anybody run the whole testsuite under fish? |
|
Tests fail. Trying to figure out why. Seems like it's only happening when the test suite is run, not individual components, so might be something in the parallel test-running code. |
|
Are we sure we want to enable this if the tests still fail? |
|
I was trying to run tests but they were breaking. That issue was fixed, but I didn't get a change to rerun before I went on holiday. There could very well still be a breakage. |
|
The tests were green so I assumed they had been fixed since that comment. It would be great if people could use reviews to indicate that changes are required on PRs, especially ones with green tests. |
|
Our CI isn't using fish as a shell
|
|
Mentioned in #4918 that I ran |
It was removed in #4924
This adds it back using the
begin; ls; true; endpattern instead of the(ls) && truepattern.