You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This pull request adds opt-in support for preprocessors using the LESSOPEN and LESSCLOSE environment variables.
To enable the preprocessor, --preprocessor=lessopen must be set either be passed as a command line argument or set in the config file. This was made opt-in to avoid breaking default installations that provide an incompatible LESSOPEN program.
When used with something like lesspipe.sh, this can potentially fix#237, #642, #748, and #971.
Implementation (Preprocessor Abstraction)
The preprocessor implementation is basically a map operation on Input<'a> -> OpenedInput<'a>.
It can either open the source input and return that directly (e.g. nothing to be done), or create a new Input<'a> and return that instead.
Preprocessors (or inputs like InputKind::StdIn) can attach "handles" to OpenedInput<'a> structs.
Handles can be used to perform cleanup actions (like deleting temporary files), or in the case of InputKind::StdIn, used to (unsafely) keep a reference to a Stdin object alive as long as its corresponding InputReader.
Note: handles will either be closed automatically when dropped (panicking on errors), or explicitly closed with OpenedInput::close.
Implementation (Lessopen)
Using the new preprocessor abstraction, basic LESSOPEN/LESSCLOSE support was added to bat. Note: This only supports file inputs.
If LESSOPEN is defined and invalid (as determined by shell_words::split), bat will silently do nothing.
This doesn't follow the behavior of less, but it's more consistent with how bat handles the pager.
If LESSOPEN is defined but the process could not be spawned, bat will print a warning.
For consistency with less (which simply ignores the exit code), this does not include the exit code of an unsuccessfully-exited command.
If LESSOPEN is defined and starts with "|", bat will use its standard output as the preprocessed file contents.
If no output is provided by the LESSOPEN function, bat will use the original file.
If LESSOPEN is defined and does not start with "|", bat will read the first line of output as the path to a temporary file containing the preprocessed file contents.
If any preprocessor command was executed and LESSCLOSE is defined, bat will execute LESSCLOSE.
If the process could not be spawned, bat will print a warning.
Caveats
Git changes will be displayed with LESSOPEN, even if it outputs something different than the original file.
This is intentional, and it's an unfortunate side-effect of the way that lesspipe.sh will always cat the file, even if it doesn't need to preprocess it.
Without this, git changes would not be displayed at all.
Due to some rewrites with the input module in order to add support for preprocessors, two instances of unsafe were added.
A comment with safety information was included.
It is not 100% compatible with less's implementation.
This implementation tries to avoid invoking the user's shell, which limits the support to a shell-quoted list of arguments.
This implementation only replaces substitutes %s if it is an entire argument. Something like LESSOPEN='| echo File:%s' will not work.
No tests were added. I couldn't think of a way to add tests without writing to the filesystem.
We currently have no unsafe code in the app code, only in tests. Personally, I would consider the addition of unsafe to the "real" code to be a dramatic change for the project. Please accept my apologies in advance, if you have already discussed this and agreed that starting to use unsafe in the app code is no big deal.
I'm tentatively marking as needs-work for now. (There are also some conflicts by now that needs to be sorted out.)
Oh man, it's been a while. Honestly, this was more of a proof-of-concept idea/prototype than anything. Looking back at the implementation, I agree that I shouldn't have used unsafe here and that we don't need to introduce any unsafe code unless strictly necessary.
I'm actually going to close this. If we end up with more interest in supporting LESSOPEN thought, I would be happy to look into rewriting it at a later date when I have more free time.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request adds opt-in support for preprocessors using the
LESSOPENandLESSCLOSEenvironment variables.To enable the preprocessor,
--preprocessor=lessopenmust be set either be passed as a command line argument or set in the config file. This was made opt-in to avoid breaking default installations that provide an incompatibleLESSOPENprogram.When used with something like
lesspipe.sh, this can potentially fix #237, #642, #748, and #971.Implementation (Preprocessor Abstraction)
The preprocessor implementation is basically a
mapoperation onInput<'a> -> OpenedInput<'a>.It can either open the source input and return that directly (e.g. nothing to be done), or create a new
Input<'a>and return that instead.Preprocessors (or inputs like
InputKind::StdIn) can attach "handles" toOpenedInput<'a>structs.Handles can be used to perform cleanup actions (like deleting temporary files), or in the case of
InputKind::StdIn, used to (unsafely) keep a reference to aStdinobject alive as long as its correspondingInputReader.Note: handles will either be closed automatically when dropped (panicking on errors), or explicitly closed with
OpenedInput::close.Implementation (Lessopen)
Using the new preprocessor abstraction, basic
LESSOPEN/LESSCLOSEsupport was added tobat.Note: This only supports file inputs.
If
LESSOPENis defined and invalid (as determined byshell_words::split),batwill silently do nothing.This doesn't follow the behavior of
less, but it's more consistent with howbathandles the pager.If
LESSOPENis defined but the process could not be spawned,batwill print a warning.For consistency with
less(which simply ignores the exit code), this does not include the exit code of an unsuccessfully-exited command.If
LESSOPENis defined and starts with"|",batwill use its standard output as the preprocessed file contents.If no output is provided by the LESSOPEN function,
batwill use the original file.If
LESSOPENis defined and does not start with"|",batwill read the first line of output as the path to a temporary file containing the preprocessed file contents.If any preprocessor command was executed and
LESSCLOSEis defined,batwill executeLESSCLOSE.If the process could not be spawned,
batwill print a warning.Caveats
Git changes will be displayed with LESSOPEN, even if it outputs something different than the original file.
This is intentional, and it's an unfortunate side-effect of the way that
lesspipe.shwill alwayscatthe file, even if it doesn't need to preprocess it.Without this, git changes would not be displayed at all.
Due to some rewrites with the
inputmodule in order to add support for preprocessors, two instances ofunsafewere added.It is not 100% compatible with
less's implementation.%sif it is an entire argument. Something likeLESSOPEN='| echo File:%s'will not work.No tests were added. I couldn't think of a way to add tests without writing to the filesystem.