Repository: sharkdp/bat
File: src/output.rs (Rust)

Code before:

        if pager.kind == PagerKind::Bat {
            return Err(Error::InvalidPagerValueBat);
        }

        let resolved_path = match grep_cli::resolve_binary(&pager.bin) {
            Ok(path) => path,
            Err(_) => {
                return Ok(OutputType::stdout());
            }
        };

        let mut p = Command::new(resolved_path);
        let args = pager.args;

        if pager.kind == PagerKind::Less {
            // less needs to be called with the '-R' option in order to properly interpret the
            // ANSI color sequences printed by bat. If someone has set PAGER="less -F", we
            // therefore need to overwrite the arguments and add '-R'.
            //
            // We only do this for PAGER (as it is not specific to 'bat'), not for BAT_PAGER
            // or bats '--pager' command line option.
            let replace_arguments_to_less = pager.source == PagerSource::EnvVarPager;

            if args.is_empty() || replace_arguments_to_less {
                // Passing '--no-init' fixes a bug with '--quit-if-one-screen' in older
                // versions of 'less'. Unfortunately, it also breaks mouse-wheel support.
                //
                // See: http://www.greenwoodsoftware.com/less/news.530.html
                //
                // For newer versions (530 or 558 on Windows), we omit '--no-init' as it
                // is not needed anymore.
                let less_version = retrieve_less_version(&pager.bin);
                match less_version {
                    None => {
                        p.arg("--no-init");
                    }
                    Some(LessVersion::Less(version))
                        if (version < 530 || (cfg!(windows) && version < 558)) =>
                    {
                        p.arg("--no-init");
                    }
                    Some(LessVersion::BusyBox) => {
                        // This makes BusyBox less filter out color escapes
                        p.arg("-R");
                    }
                    _ => {}
                }
                if less_version != Some(LessVersion::BusyBox) {
                    p.arg("--RAW-CONTROL-CHARS");
                    if single_screen_action == SingleScreenAction::Quit {

Review comment (suggestion):
If busybox less understands `-R` but not `--RAW-CONTROL-CHARS`, it would IMO make sense to change

```
p.arg("--RAW-CONTROL-CHARS");
```

into 

```
p.arg("-R"); // Short version of --RAW-CONTROL-CHARS to maximize compatibility
```

Same for `--quit-if-one-screen` and `-F`.

Does busybox have a short variant of `--chop-long-lines`? If so that would be great, because then we could avoid moving this block of code down, and instead reuse the logic for both buxybox less and less proper. The old code is like bomb waiting to explode with regressions, so it would be great if we could try to stick to "obviously safe" changes, with a minimal "structural" diff.

Code after:

        if pager.kind == PagerKind::Bat {
            return Err(Error::InvalidPagerValueBat);
        }

        let resolved_path = match grep_cli::resolve_binary(&pager.bin) {
            Ok(path) => path,
            Err(_) => {
                return Ok(OutputType::stdout());
            }
        };

        let mut p = Command::new(resolved_path);
        let args = pager.args;

        if pager.kind == PagerKind::Less {
            // less needs to be called with the '-R' option in order to properly interpret the
            // ANSI color sequences printed by bat. If someone has set PAGER="less -F", we
            // therefore need to overwrite the arguments and add '-R'.
            //
            // We only do this for PAGER (as it is not specific to 'bat'), not for BAT_PAGER
            // or bats '--pager' command line option.
            let replace_arguments_to_less = pager.source == PagerSource::EnvVarPager;

            if args.is_empty() || replace_arguments_to_less {
                p.arg("-R"); // Short version of --RAW-CONTROL-CHARS for maximum compatibility
                if single_screen_action == SingleScreenAction::Quit {
                    p.arg("-F"); // Short version of --quit-if-one-screen for compatibility
                }

                if wrapping_mode == WrappingMode::NoWrapping(true) {
                    p.arg("-S"); // Short version of --chop-long-lines for compatibility
                }

                // Passing '--no-init' fixes a bug with '--quit-if-one-screen' in older
                // versions of 'less'. Unfortunately, it also breaks mouse-wheel support.
                //
                // See: http://www.greenwoodsoftware.com/less/news.530.html
                //
                // For newer versions (530 or 558 on Windows), we omit '--no-init' as it
                // is not needed anymore.
                match retrieve_less_version(&pager.bin) {
                    None => {
                        p.arg("--no-init");
                    }
                    Some(LessVersion::Less(version))
                        if (version < 530 || (cfg!(windows) && version < 558)) =>
                    {
                        p.arg("--no-init");
                    }
                    _ => {}