Add $LESSOPEN and $LESSCLOSE support

[Anomalocaridid]

Addresses #1739 and offers an alternative to #1597.

This pull request adds support for $LESSOPEN and $LESSCLOSE to bat. I tried to be as compatible with less's implementation as I could, and I tried to make sure it could handle each of the cases outlined in the "Input Preprocessor' section of less's man page. These cases are:

• $LESSOPEN writing output to a temporary file that gets cleaned up by $LESSCLOSE
• $LESSOPEN directly piping data and using empty output to signal to fall back to the original file
• $LESSOPEN directly piping data and using the exit code to determine whether or not to fall back to the original file
• The previous two cases, but also with $LESSCLOSE
• All of the above cases, but reading from stdin instead of a file

Basically, the $LESSOPEN preprocessor is used to create an OpenedInput, falling back to the default file opening method if unable to preprocess the file. A type called Preprocessed, which implements BufRead, is used to wrap either a temporary file or data directly output by $LESSOPEN and used to create an InputReader. When $LESSCLOSE is set, it is executed when the Preprocessed is dropped.

Rather than just using std::process::Command, I used a crate called run_script, which runs shell commands in the shell rather than running a program and passing arguments directly, so that users can use things like input and output redirection directly like with less.

I also added a --no-lessopen flag similar to what less has, although I obviously could not add the corresponding -L flag since it is already in use. I updated the completion scripts where applicable.

In addition, I made sure to be as thorough as I reasonably could with the regression tests. However, I disabled the integration tests on Windows and other non-Unix-like systems with #[cfg(unix)] because they were not written with Windows in mind. Is this something that I should try to fix first?

I referenced #1597 to help me figure out an approach, although my implementation is still rather different and does not use any unsafe code.

[Enselic]

I've done a quick skim-through of the code and to me this looks very good in general. The size of the PR makes it a bit hard to find a time slot and the energy to do a detailed review. It is not obvious to me that this PR can be split up into smaller chunks to mitigate that, however.

Just wanted to share some encouraging words. I know this PR have been sitting here for quite a while now. Hopefully someone will find the time to look into this PR in more detail in not a too distant future.

[Anomalocaridid]

I understand. Thanks for letting me know.

[Anomalocaridid]

Is there anything at all I can try to make my PR less cumbersome to review? I understand you all are busy and I would like to do whatever I can to make reviewing this pull request less of a burden.

[Enselic]

I think we should merge this and see what happens. If there are bugs, they can be fixed. Absolute worst case we have to revert this again, but just looking at the way you write code, tests, comments, that would be very surprising to me.

I don't have the time or energy to do a detailed review and understand this code. But I don't need to. If it works, it works. It always looks reasonable to me when I glance it over.

Can you rebase on latest master please?

[Anomalocaridid]

Rebased. Thank both of you so much for taking another look at my pull request!

[sharkdp]

Thank you very much for working on this!

[Anomalocaridid]

It was my pleasure! Thank you too!

[Aaron-Rumpler]

Maybe this should be opt-in instead of opt-out? Especially as this breaks spectacularly when $LESSOPEN is bat.

[Aaron-Rumpler]

Also, should bat be calling less with --no-lessopen by default? Right now, the file goes through $LESSOPEN twice (once by bat, and again by less).

[Anomalocaridid]

Maybe this should be opt-in instead of opt-out? Especially as this breaks spectacularly when $LESSOPEN is bat.

Good point. I failed to consider people that already use bat in $LESSOPEN.

Also, should bat be calling less with --no-lessopen by default? Right now, the file goes through $LESSOPEN twice (once by bat, and again by less).

Great catch! This helped significantly to figure out the cause of a bug in one of the tests that made it intermittently fail. It was somehow cause by less seeing $LESSOPEN.

Thanks for pointing these out. I'll make sure to address both of these concerns when I get the chance.

[Enselic]

@Aaron-Rumpler Thank you for that crucial feedback 👍

@Anomalocaridid Thank you for taking care of these bugs!

Let's not enable lessopen by default (#2659) until we've had time to at least fix these known bugs. That way we are not blocked on making bat releases, and you @Anomalocaridid do not have any pressure to fix the bugs quickly.

[Enselic]

@Aaron-Rumpler Can you please confirm that the problems you reported do not manifest on latest git master now that the feature is not enabled by default? (#2659)

[Aaron-Rumpler]

Yep, bat no longer uses $LESSOPEN by default on f49278c. bat still doesn't prevent less from using $LESSOPEN itself, but that's not new behaviour.

[Aaron-Rumpler]

Also, the man page still claims that $LESSOPEN is the default.

[Anomalocaridid]

Also, the man page still claims that $LESSOPEN is the default.

I'll take care of that as well.

[eth-p]

Oh this is awesome! Thanks for implementing this properly and taking the time to do it properly (unlike my original attempt).