Make more of share/functions print error messages to stderr

[thomcc]

Description

Hi. I was using some_fish_func > some_file.something and had a bug in my program (where I misused pushd in some irrelevant way). This caused me to notice that pushd logged its error message to stdout and not stderr, since it sort of¹ corrupted the file.

My personal feelings here is that it is typically Bad Form for programs to use stdout for writing messages intended for the user, especially error messages (with some exceptions of course).

I have fish-shell checked out from prior patches so I took a peek. I noticed that most functions do the "right thing" and log their complaints to stderr, which I interpreted to mean that probably a patch fixing pushd would probably be appreciated. Unfortunately, at this point my "What If I Fixed It All" brain kicked in, so I started looking to see if there's anything else that this applies to, and there is.

That dream was pretty short-lived though, since fixing each part requires figuring out if it's program output vs an error... and I really don't want to break anything in this patch. So I ended up just doing the obvious bits.

... And after a while of doing with that, I decided that I really don't have time to shave this yak, especially given that I'm probably missing a lot of places anyway. Really, someone who knows the code better is probably the person who should finish that task (or not²).

So this is just a start on the problem, and only addresses a small handful of cases. I don't think it makes anything worse though, so at least there's that.

---

Anyway, of the changes in this patch, I think the directory manipulation stuff and seq are the most important. It's likely that those are used often enough in scripts for their stdout to frequently be captured as a string, which would just result in bad behavior.

I think the cases which are least important are ones that are almost certainly only used interactively. For some reason, I still touched a few of these. That said, I'm (of course) willing to revert any of these changes if you'd prefer.

TODOs:

• Changes to fish usage are reflected in user documentation/manpages.

  No changes.

• Tests have been added for regressions fixed

  Adding tests for this would be a nightmare. Please do not ask this of me.

• User-visible changes noted in CHANGELOG.rst

  I think this is technically user visible, but in practice most of the users who notice will just be finding new bugs in their code.

  I guess maybe some code attempts to parse one of these error messages from stdout, but such code is very fragile anyway.

---

P.S. While I'm here, since it's technically related... I think it would be good for fish to have printf/echo variants³ that automatically use stderr. (TBH I mostly want this as a user, because the redirect syntax is weird and hard for me to remember I have to look it up every time). That said, if you used it consistently, it would make auditing stuff like this in the future easier... at least after you switched to use it (it seems unlikely to make it any easier to do the first time).

Footnotes

1. Well, the file would be corrupted anyway because of the bug, this just made it happen slightly sooner. ↩

2. I won't judge if nothing happens after this PR. I get it -- it's very hard in open source to find the resources for projects like "ensure all the sites where we $frobnicate are Good And Proper", especially if it only gets hit in failure modes.

   For stuff like this it is especially hard, since it seems somewhat resistant to automation -- any tooling would have to figure out what is an error message vs what is the real output. (But maybe there are some heuristics which would work enough of the time to be useful here) ↩

3. For example, they could be eprintf and... uh... eecho?... errcho? ... Uh, coming up with a name better than those is left as an exercise for the reader. ↩

[thomcc]

Dang. Should have figured there's a test somewhere in the repo that checks output.

[faho]

Should have figured there's a test somewhere in the repo that checks output.

The current test failure is, as best as I can tell, Github Action's Ubuntu's ASAN being broken.

The first failure happened after an unrelated change and it keeps happening, we probably want to disable it for the time being.

(typically if a test for output fails it'll fail on most/all of the machines)

[thomcc]

Ah, so I probably don't have to look into this failure? (I hadn't gotten around to it yet, so that's good news)

[faho]

Merged, thanks!