glow
glow copied to clipboard
fix: handle PAGER paths with spaces
fixes #385
This fixes the handling of pager paths pulled from environment variables. It works on the assumption that the value of the PAGER
environment variable will be the path to an executable, and not an actual command (with arguments and what not).
The logic has been moved inside a utility function for the following reasons:
- If, in the future, we need to support more complex logic for building the actual command to be run for using pagers, we can add it in this function instead of bloating our main code.
- Makes the code re-usable, if we need to support such logic for other options (after renaming).
- Makes it easier to test.
@bashbunni do you mind reviewing this PR?
@muesli I see you added the bug label to this PR. Silly question, but is it because it solves a bug, or are the changes this PR introduced not working as expected? 😐
It's there to indicate it addresses a bug in glow.
@muesli do the changes look good to you? Do we need something else as well here?
Looks pretty good already, see the two comments I left tho. Extract credits for adding a test case! :+1:
@muesli do you mind giving this a look again?
Sorry for the delay @Shreya-7 we will get to it, there is just some pending functionality we need to figure out before merging
@bashbunni do we still need this bugfix PR?