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).