The Wayback Machine - https://web.archive.org/web/20220614094124/https://github.com/sharkdp/bat/issues/2023
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PAGER=batcat turns into a fork nightmare #2023

Open
rsamuelklatchko opened this issue Jan 12, 2022 · 4 comments
Open

PAGER=batcat turns into a fork nightmare #2023

rsamuelklatchko opened this issue Jan 12, 2022 · 4 comments
Labels
bug good first issue help wanted

Comments

@rsamuelklatchko
Copy link

@rsamuelklatchko rsamuelklatchko commented Jan 12, 2022

I decided to make batcat my PAGER and when I tried setting:

PAGER=batcat

it didn't work. After some troubleshooting I realized that batcat was forking/execing itself and was able to fix it by also adding:

BAT_PAGER=less

while this solved the problem it was not obvious. One suggestion is when reading PAGER, if the value is the binary name (or ends with /{binary_name}) the value is ignored and the default is used instead.

@Enselic
Copy link
Collaborator

@Enselic Enselic commented Jan 12, 2022

Thanks for reporting!

There is code to prevent that from happening, but it does not take into account that the binary can be called something other than bat.

See

bat/src/pager.rs

Lines 105 to 106 in 9287cf6

// If PAGER=bat, silently use 'less' instead to prevent
// recursion.

I would be happy to review PRs that solves this

@Enselic Enselic added the bug label Jan 12, 2022
@rsamuelklatchko
Copy link
Author

@rsamuelklatchko rsamuelklatchko commented Jan 12, 2022

My thought would be to modify PagerKind::from_bin to support other names. I have a couple of ideas:

  1. Just add "batcat" (I'm on Debian which is what the binary is named) and possibly any other common alternate names if you know of any.
  2. Find the name the binary is being run as using env::args.

For #2, I'm thinking to reuse

bat/src/pager.rs

Lines 43 to 46 in 9287cf6

match Path::new(bin)
.file_stem()
.map(|s| s.to_string_lossy())
.as_deref()
to get the name from env::args[0].

@Enselic
Copy link
Collaborator

@Enselic Enselic commented Jan 13, 2022

My first choice would be 2., but the code to get the program name is something more like std::env::args().nth(0)

Or even better is std::env::args_os().nth(0) but that might require a bigger change

@rsamuelklatchko
Copy link
Author

@rsamuelklatchko rsamuelklatchko commented Jan 15, 2022

I have a code fix but having not used GitHub I'm not sure if exactly how to send it to you. What I'm suggesting is this:

impl PagerKind {
    fn from_bin(bin: &str) -> PagerKind {
        use std::path::Path;

        let file_stem = Path::new(bin).file_stem();

        match file_stem.map(|s| s.to_string_lossy()).as_deref() {
            Some("bat") => PagerKind::Bat,
            Some("less") => PagerKind::Less,
            Some("more") => PagerKind::More,
            Some("most") => PagerKind::Most,
            _ => {
                if env::args_os()
                    .nth(0)
                    .map(|arg0| Path::new(&arg0).file_stem() == file_stem)
                    .unwrap_or(false)
                {
                    PagerKind::Bat
                } else {
                    PagerKind::Unknown
                }
            }
        }
    }
}
@Enselic Enselic added help wanted good first issue labels Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug good first issue help wanted
2 participants