bat icon indicating copy to clipboard operation
bat copied to clipboard

ctrl-c still exits less, even when bat isn't passing `--quit-on-intr`

Open injust opened this issue 3 months ago β€’ 17 comments

What steps will reproduce the bug?

  1. BAT_PAGER='less -R' bat foo
  2. ctrlc

What happens?

less exits.

What did you expect to happen instead?

less shouldn't exit because setting $BAT_PAGER should prevent bat from passing --quit-on-intr to less.

How did you install bat?

Homebrew


bat version and environment

Software version

bat 0.25.0

Operating system

  • OS: macOS 15.7.1 Sequoia
  • Kernel: 24.6.0

Command-line

bat --diagnostic foo

Environment variables

BAT_CACHE_PATH=<not set>
BAT_CONFIG_PATH=<not set>
BAT_OPTS=<not set>
BAT_PAGER='less -R'
BAT_PAGING=<not set>
BAT_STYLE=<not set>
BAT_TABS=<not set>
BAT_THEME=<not set>
COLORTERM=truecolor
LANG=en_CA.UTF-8
LC_ALL=<not set>
LESS='--quit-if-one-screen --ignore-case --jump-target=.1 --LONG-PROMPT --quiet --RAW-CONTROL-CHARS --incsearch --no-vbell'
MANPAGER='sh -c '\''awk '\''\'\'''\''{ gsub(/\x1B\[[0-9;]*m/, "", $0); gsub(/.\x08/, "", $0); print }'\''\'\'''\'' | bat -p -lman'\'''
NO_COLOR=<not set>
PAGER=<not set>
SHELL=/usr/local/bin/fish
TERM=xterm-ghostty
XDG_CACHE_HOME=<not set>
XDG_CONFIG_HOME=<not set>

System Config file

Could not read contents of '/etc/bat/config': No such file or directory (os error 2).

Config file

--theme-light="Catppuccin Latte"
--theme-dark="Catppuccin Mocha"

Custom assets metadata

bat_version: 0.25.0
creation_time:
  secs_since_epoch: 1760872544
  nanos_since_epoch: 104422000

Custom assets

  • metadata.yaml, 97 bytes
  • syntaxes.bin, 969578 bytes
  • themes.bin, 58252 bytes

Compile time information

  • Profile: release
  • Target triple: x86_64-apple-darwin
  • Family: unix
  • OS: macos
  • Architecture: x86_64
  • Pointer width: 64
  • Endian: little
  • CPU features: cmpxchg16b,fxsr,pclmulqdq,popcnt,sse,sse2,sse3,sse4.1,sse4.2,ssse3
  • Host: x86_64-apple-darwin

Less version

> less --version 
less 685 (PCRE2 regular expressions)
Copyright (C) 1984-2025  Mark Nudelman

less comes with NO WARRANTY, to the extent permitted by law.
For information about the terms of redistribution,
see the file named README in the less distribution.
Home page: https://greenwoodsoftware.com/less

injust avatar Oct 19 '25 21:10 injust

This behaviour you're seeing is what's documented in the README, here:

https://github.com/sharkdp/bat/blob/006d77fa393172b7089da27b67d3e0b235363720/README.md?plain=1#L657-L667

These options will not be added if:

  • The BAT_PAGER environment variable contains any command-line arguments (e.g. export BAT_PAGER="less -R")

When you set BAT_PAGER="less -R", bat detects that you've provided command-line arguments (the -R flag). As the README says, bat will then not add any of its automatic options: -R, -F, -K (the --quit-on-intr one), -X, or -S.

So bat passes only what you specified: less -R.

You can unset BAT_PAGER entirely and bat will automatically add -R and -K for you, which is the simplest. Or, set it to less -RK or just less which will also become -RFK)

Hope that addresses it for you.

lmmx avatar Oct 19 '25 21:10 lmmx

When you set BAT_PAGER="less -R", bat detects that you've provided command-line arguments (the -R flag). As the README says, bat will then not add any of its automatic options: -R, -F, -K (the --quit-on-intr one), -X, or -S.

So bat passes only what you specified: less -R.

That's what should be happening. But something is wrong, as ctrlc still exits less, so it seems like bat is still passing -K/--quit-on-intr to less.

injust avatar Oct 19 '25 22:10 injust

Context:

I've configured my own default options for less in $LESS. However, bat appends its own options.

Most of bat's less options are good defaults (I use --RAW-CONTROL-CHARS and --quit-if-one-screen in my $LESS) , but --quit-on-intr breaks ctrlc functionality within less. From the less man page (emphasis mine):

-K or --quit-on-intr Causes less to exit immediately (with status 2) when an interrupt character (usually ^C) is typed. Normally, an interrupt character causes less to stop whatever it is doing and return to its command prompt. Note that use of this option makes it impossible to return to the command prompt from the "F" command.

ctrlc is also used in less for cancelling a search, which is what I'm used to doing.

bat makes less exit on ctrlc and I don't like that, but I can't stop it:

  • less doesn't have an option to negate --quit-on-intr
  • I don't want to set --pager everywhere I use bat (e.g. from fzf), but even if I call bat with --pager='less -R', less still exits on ctrlc
  • Setting BAT_PAGER='less -R' should stop bat from passing --quit-on-intr to less, but less still exits on ctrlc

injust avatar Oct 19 '25 22:10 injust

If I set BAT_PAGER='less -F' or use --pager='less -F', it's clear that bat isn't passing --RAW-CONTROL-CHARS anymore, since the colour codes aren't interpreted by less anymore.

So I think bat is (correctly) not passing --quit-on-intr. But if that's the case, why does less exit on ctrlc?

injust avatar Oct 19 '25 22:10 injust

Hmm I thought I understood the issue and had repro'd, now I'm not sure.

These 2 commands behave differently for me

louis 🚢 ~/dev/bat $ echo foo | BAT_PAGER='less -R' bat
^C
louis 🚢 ~/dev/bat $ echo foo | BAT_PAGER='less -RK' bat

louis 🚢 ~/dev/bat $ 

The first one does not drop out of less on Ctrl + C (in the same way q exits less), the second does.

Can I double check that you see the same? I think what I am seeing is correct as per the documented expectation

Specifically though, it exits less but doesn't fully restore the terminal to normal. The screen state seems to be left in whatever mode less was using rather than being properly cleaned up. (Not sure of the exact terminology here)

Actually I think I see what you mean now, comparing to

louis 🚢 ~ $ echo foo | less -K
louis 🚢 ~ $ echo foo | less

I can see that Ctrl + C in regular less (which for me is aliased to less -R so still no -K involvement) does immediately restore the terminal to normal, so yeah I agree bat is probably doing something it shouldn't here

lmmx avatar Oct 19 '25 23:10 lmmx

The first one does not drop out of less on Ctrl + C (in the same way q exits less), the second does.

Specifically though, it exits less but doesn't fully restore the terminal to normal. The screen state seems to be left in whatever mode less was using rather than being properly cleaned up. (Not sure of the exact terminology here)

The first one completely messes up my terminal state. After ctrlc, I get a new prompt, but less is actually still running. This is probably the motivation for bat passing --quit-on-intr to less.

Can I double check that you see the same? I think what I am seeing is correct as per the documented expectation

The second one exits less for me too.

injust avatar Oct 19 '25 23:10 injust

When I run:

$ echo foo | less -RK
$ echo foo | less -R

ctrlc exits less in the first command, but not the second.

injust avatar Oct 19 '25 23:10 injust

Got you, bat exitting shouldn't stop less basically. You should still be able to scroll around...

I can do that when I run

for i in {0..10}; do echo $i; done | less -R

but not in an equivalent bat command

for i in {0..10}; do echo $i; done |  BAT_PAGER='less -R' bat 

Hmmm maybe best place to debug that is to snoop on exactly what args get passed to less πŸ€” or maybe it's to do with how the command is called...

https://github.com/sharkdp/bat/blob/006d77fa393172b7089da27b67d3e0b235363720/src/output.rs#L208

lmmx avatar Oct 19 '25 23:10 lmmx

Possibly SIGINT is going to both bat and less, or the pipe handling is wrong, or something to do with the process that's called...

Is it that Command::spawn() creates the child process in the same process group so Ctrl+C sends SIGINT to the entire process group (killing less along with bat)? If bat exits first and closes the pipe, less might die instead? We can use setsid to isolate less to avoid that [to debug/diagnose if it is that]...

https://github.com/sharkdp/bat/blob/006d77fa393172b7089da27b67d3e0b235363720/src/output.rs#L167-L171

This stays open (cannot kill with Ctrl + C) so maybe process groups are the cause

echo foo | setsid env BAT_PAGER='less -R' bat

edit: no, this script doesn't stay open so I don't think it's the cause after all

# Create a wrapper that doesn't close stdin immediately
cat > /tmp/bat-wrapper.sh << 'EOF'
#!/bin/bash
bat "$@"
sleep 10  # Keep the pipe open for 10 seconds after bat finishes
EOF

chmod +x /tmp/bat-wrapper.sh

echo foo | BAT_PAGER='less -R' /tmp/bat-wrapper.sh

lmmx avatar Oct 20 '25 00:10 lmmx

I've tried debugging what's happening here.

I confirmed that less receives SIGINT and exits with code 1 immediately. Without -K, less should interrupt its current operation but stay open for navigation but it's exitting entirely instead.

Then strace traced the exact steps:

  1. Less reads all content from stdin (fd 0) and receives EOF
  2. Less waits for terminal input on fd 3 (normal behavior for staying open)
  3. SIGINT arrives while less is waiting
  4. Less handles SIGINT and attempts to continue reading from the terminal
  5. Less receives EIO (Input/output error) on fd 3 and exits with code 1

The strace logs show the terminal becoming unavailable to less after it gets SIGINT, which I read as related to bat's cleanup (command.wait() in the Drop impl in src/output.rs) but I wasn't able to modify that without breaking bat. When bat gets SIGINT and starts cleanup while waiting on the less process, something about the terminal ownership/state causes less to lose access to the terminal (and here ends my ability to work out what goes on).

I tried to patch by creating a new process group with .process_group(0) and modifying the Drop to take or try_wait but no help. It's something between the signal handling, process spawning, and terminal ownership.

Hope that helps somewhat!

lmmx avatar Oct 20 '25 00:10 lmmx

Maybe we should move the adding if the -K arg which bat passes to less into the if args.is_empty() branch: https://github.com/sharkdp/bat/pull/3376

keith-hall avatar Oct 20 '25 02:10 keith-hall

The report is about avoiding -K behaviour, adding -K would cause the immediate exit the user is trying to prevent:

bat makes less exit on ctrlc and I don't like that, but I can't stop it:

The user wants to stay in less after Ctrl+C to continue browsing buffered data. Adding -K does the opposite: it exits less and clears the pager.

The debugging shows bat isn't passing -K when BAT_PAGER='less -R' is set (I think we agreed this was correct and "as advertised" in the README). The issue is that less exits anyway due to terminal ownership problems during bat's cleanup (less receives EIO when bat handles SIGINT).

I think #3376 is relevant though, it sounds like a previous report of the same underlying issue.

...an issue on Windows with PowerShell, where the subprocess for less is not terminated correctly.

  • Desired behaviour: echo foo | less -R + Ctrl+C (bat disconnects from less but less runs normally)
  • Current bug: echo foo | BAT_PAGER='less -R' bat + Ctrl+C (ends up halfway back to the command line, can't scroll)

lmmx avatar Oct 20 '25 08:10 lmmx

I think we want to ignore SIGINT while the pager is active, preventing our teardown from killing the pager, then restore the handler after less exits.

signal-hook crate is the way to do this, I came across an example of it here but that’s just recording SIGINT not temporarily ignoring it. It’s unix-only so would be cfg gated in. Original report was on macOS and I can repro on Linux, this wouldn’t be for Windows users (SIGINT is POSIX only)

lmmx avatar Oct 20 '25 08:10 lmmx

  • Patched in #3447
Click to show minimal patch:
diff --git a/Cargo.toml b/Cargo.toml
index d64f3fcc..a5ecca9a 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -75,6 +75,7 @@ execute = { version = "0.2.13", optional = true }
 terminal-colorsaurus = "1.0"
 unicode-segmentation = "1.12.0"
 itertools = "0.14.0"
+signal-hook = "0.3.18"
 
 [dependencies.git2]
 version = "0.20"
diff --git a/src/output.rs b/src/output.rs
index aea37333..a5dd0618 100644
--- a/src/output.rs
+++ b/src/output.rs
@@ -13,6 +13,35 @@ use crate::paging::PagingMode;
 #[cfg(feature = "paging")]
 use crate::wrapping::WrappingMode;
 
+#[cfg(all(unix, feature = "paging"))]
+use std::sync::atomic::AtomicBool;
+#[cfg(all(unix, feature = "paging"))]
+use std::sync::Arc;
+#[cfg(all(unix, feature = "paging"))]
+struct IgnoreSigint {
+    _handle: signal_hook::SigId,
+}
+
+
+#[cfg(all(unix, feature = "paging"))]
+impl IgnoreSigint {
+    fn new() -> Self {
+        let handle = signal_hook::flag::register(
+            signal_hook::consts::SIGINT,
+            Arc::new(AtomicBool::new(false))
+        ).expect("failed to ignore SIGINT");
+        
+        Self { _handle: handle }
+    }
+}
+
+#[cfg(feature = "paging")]
+pub struct PagerProc {
+    child: Child,
+    #[cfg(unix)]
+    _sigint_guard: IgnoreSigint,
+}
+
 #[cfg(feature = "paging")]
 pub struct BuiltinPager {
     pager: minus::Pager,
@@ -50,10 +79,9 @@ enum SingleScreenAction {
     Nothing,
 }
 
-#[derive(Debug)]
 pub enum OutputType {
     #[cfg(feature = "paging")]
-    Pager(Child),
+    Pager(PagerProc),
     #[cfg(feature = "paging")]
     BuiltinPager(BuiltinPager),
     Stdout(io::Stdout),
@@ -165,9 +193,13 @@ impl OutputType {
         };
 
         Ok(p.stdin(Stdio::piped())
-            .spawn()
-            .map(OutputType::Pager)
-            .unwrap_or_else(|_| OutputType::stdout()))
+			.spawn()
+			.map(|child| OutputType::Pager(PagerProc {
+				child,
+				#[cfg(unix)]
+				_sigint_guard: IgnoreSigint::new(),
+			}))
+			.unwrap_or_else(|_| OutputType::stdout()))
     }
 
     pub(crate) fn stdout() -> Self {
@@ -187,12 +219,12 @@ impl OutputType {
     pub fn handle<'a>(&'a mut self) -> Result<OutputHandle<'a>> {
         Ok(match *self {
             #[cfg(feature = "paging")]
-            OutputType::Pager(ref mut command) => OutputHandle::IoWrite(
-                command
-                    .stdin
-                    .as_mut()
-                    .ok_or("Could not open stdin for pager")?,
-            ),
+            OutputType::Pager(ref mut proc) => OutputHandle::IoWrite(
+				proc.child
+					.stdin
+					.as_mut()
+					.ok_or("Could not open stdin for pager")?,
+			),
             #[cfg(feature = "paging")]
             OutputType::BuiltinPager(ref mut pager) => OutputHandle::FmtWrite(&mut pager.pager),
             OutputType::Stdout(ref mut handle) => OutputHandle::IoWrite(handle),
@@ -204,9 +236,9 @@ impl OutputType {
 impl Drop for OutputType {
     fn drop(&mut self) {
         match *self {
-            OutputType::Pager(ref mut command) => {
-                let _ = command.wait();
-            }
+			OutputType::Pager(ref mut proc) => {
+				let _ = proc.child.wait();
+			}
             OutputType::BuiltinPager(ref mut pager) => {
                 if pager.handle.is_some() {
                     let _ = pager.handle.take().unwrap().join().unwrap();

Add the guard:


#[cfg(all(unix, feature = "paging"))]
use std::sync::atomic::AtomicBool;
#[cfg(all(unix, feature = "paging"))]
use std::sync::Arc;
#[cfg(all(unix, feature = "paging"))]
struct IgnoreSigint {
    _handle: signal_hook::SigId,
}


#[cfg(all(unix, feature = "paging"))]
impl IgnoreSigint {
    fn new() -> Self {
        let handle = signal_hook::flag::register(
            signal_hook::consts::SIGINT,
            Arc::new(AtomicBool::new(false))
        ).expect("failed to ignore SIGINT");
        
        Self { _handle: handle }
    }
}

#[cfg(feature = "paging")]
pub struct PagerProc {
    child: Child,
    #[cfg(unix)]
    _sigint_guard: IgnoreSigint,
}
  • Then swap Pager(Child) for Pager(PagerProc) in the OutputType enum
  • Then swap the spawning for a map over a closure that creates it
        Ok(p.stdin(Stdio::piped())
			.spawn()
			.map(|child| OutputType::Pager(PagerProc {
				child,
				#[cfg(unix)]
				_sigint_guard: IgnoreSigint::new(),
			}))
			.unwrap_or_else(|_| OutputType::stdout()))
    }
  • Then swap the pager command for proc and use proc.child in the handle and drop methods too
    pub fn handle<'a>(&'a mut self) -> Result<OutputHandle<'a>> {
        Ok(match *self {
            #[cfg(feature = "paging")]
            OutputType::Pager(ref mut proc) => OutputHandle::IoWrite(
				proc.child
					.stdin
					.as_mut()
					.ok_or("Could not open stdin for pager")?,
impl Drop for OutputType {
    fn drop(&mut self) {
        match *self {
			OutputType::Pager(ref mut proc) => {
				let _ = proc.child.wait();
			}

lmmx avatar Oct 20 '25 09:10 lmmx

Looking at htop (F4: filter bat|less then run echo | BAT_PAGER='less -R' ./target/debug/bat)

I can see this patch is qualitatively different: with the existing bat

Existing

    PID USER       PRI  NI  VIRT   RES   SHR S  CPU%β–½MEM%   TIME+  Command (merged)                                                                      
1548091 louis       20   0 79660  9600  6720 S   0.0  0.0  0:00.00 β”‚  β”‚  β”‚  └─ /home/louis/.cargo/bin/bat                                                
1548093 louis       20   0  8672  2560  2240 S   0.0  0.0  0:00.00 β”‚  β”‚  β”‚     └─ /usr/bin/less -R

Ctrl + C leaves less open (it just loses access to the tty)

    PID USER       PRI  NI  VIRT   RES   SHR S  CPU%β–½MEM%   TIME+  Command (merged)                                                                      
1548017 louis       20   0  8672  2560  2240 S   0.0  0.0  0:00.00 β”‚  β”œβ”€ /usr/bin/less -R  

Patched

Whereas now, looks the same with less as a child process of bat

    PID USER       PRI  NI  VIRT   RES   SHR S  CPU%β–½MEM%   TIME+  Command (merged)                                                                      
1548148 louis       20   0 88812 17280 13760 S   0.0  0.0  0:00.06 β”‚  β”‚  β”‚  └─ /home/louis/dev/bat/target/debug/batβ”‚./target/debug/bat                   
1548150 louis       20   0  8672  2560  2240 S   0.0  0.0  0:00.00 β”‚  β”‚  β”‚     └─ /usr/bin/less -R

but pressing Ctrl + C is simply a no-op, bat continues to run

    PID USER       PRI  NI  VIRT   RES   SHR S  CPU%β–½MEM%   TIME+  Command (merged)                                                                      
1548148 louis       20   0 88812 17280 13760 S   0.0  0.0  0:00.06 β”‚  β”‚  β”‚  └─ /home/louis/dev/bat/target/debug/batβ”‚./target/debug/bat                   
1548150 louis       20   0  8700  2720  2400 S   0.0  0.0  0:00.00 β”‚  β”‚  β”‚     └─ /usr/bin/less -R

So I don't know if this is the best solution but it's a solution? What are your thoughts @injust is this an unsatisfying answer? I'd imagine you want to definitively close the bat process in case it's sending lots of data, so this approach doesn't really help.

lmmx avatar Oct 20 '25 11:10 lmmx

So I don't know if this is the best solution but it's a solution? What are your thoughts @injust is this an unsatisfying answer? I'd imagine you want to definitively close the bat process in case it's sending lots of data, so this approach doesn't really help.

My motivation for changing the behaviour of ctrlc is purely to use it within less. For me, bat continuing to send data to less isn't a problem (but I haven't thought about scenarios where that might be useful).

injust avatar Oct 20 '25 11:10 injust

OK, I've marked #3447 as ready for review in that case, if that's an acceptable solution to this then it is a working option.

lmmx avatar Oct 20 '25 13:10 lmmx