Skip to content

Conversation

theturtle32
Copy link

Description

Delta was incorrectly stripping arguments from complex PAGER environment variable commands, causing failures when used with tools that set sophisticated pager configurations.

Problem

The issue occurred because delta was using bat::config::get_pager_executable(None) to process the PAGER environment variable. This function was designed to return only the executable path, stripping all arguments from the command. For simple pagers like less or bat, this worked fine. However, for complex shell commands like /bin/sh -c "head -10000 | cat", this caused serious problems:

  1. Arguments were stripped: /bin/sh -c "head -10000 | cat" became just /bin/sh
  2. Shell misinterpretation: When /bin/sh received delta's output, it tried to execute each line as a shell command
  3. Command not found errors: This resulted in numerous "command not found" errors for every line of diff output

Real-world Impact

This bug particularly affected users of Cursor IDE's AI Agent feature, which automatically sets:

export PAGER='/bin/sh -c "head -10000 | cat"'

This configuration is not user-configurable in Cursor, causing git commands like git diff --cached to fail consistently with "command not found" errors when delta was configured as the git pager.

Root Cause

The problem was in src/env.rs where delta called:

bat::config::get_pager_executable(None)

This function was designed to extract just the executable name for validation purposes, not to preserve the full command with arguments for execution.

Solution

The fix reimplements bat's pager detection logic locally in delta to:

  1. Preserve full commands: Complex pager commands with arguments are kept intact
  2. Maintain shell quoting: Original command strings are preserved to maintain proper quoting
  3. Handle problematic pagers: Still replaces known problematic pagers (more, most) with less
  4. Prevent recursion: Avoids infinite loops when delta is set as the pager
  5. Respect precedence: Maintains the correct priority order (DELTA_PAGER > BAT_PAGER > PAGER)

Changes Made

  • Replaced bat::config::get_pager_executable(None) with custom get_pager_from_env() function
  • Added comprehensive test coverage for complex pager commands
  • Added integration test to verify pager execution works correctly
  • Preserved all existing behavior for simple pagers and problematic pager replacement

Testing

The fix includes extensive test coverage:

  • Complex shell pager commands (/bin/sh -c "head -10000 | cat")
  • Simple pagers with arguments (less -R -F -X)
  • Problematic pager replacement (moreless)
  • Environment variable precedence (DELTA_PAGER vs PAGER)
  • Integration testing with actual pager execution

Compatibility

This fix maintains full backward compatibility:

  • ✅ Simple pagers (less, bat) work unchanged
  • ✅ Problematic pagers (more, most) still get replaced with less
  • ✅ All existing environment variable behavior preserved
  • ✅ No breaking changes to the public API

The fix specifically resolves the Cursor IDE integration issue while improving delta's robustness for any environment that uses complex pager configurations.

Resolves #2033

Previously, delta used bat::config::get_pager_executable() which would
strip arguments from complex PAGER commands like '/bin/sh -c "head -10000 | cat"',
leaving only the executable path. This caused "command not found" errors
when the pager tried to execute.

This commit:
- Reimplements bat's pager detection logic to preserve full PAGER commands
- Adds comprehensive test coverage for various pager scenarios
- Introduces EnvVarGuard utility for safe environment variable testing
- Maintains compatibility with problematic pagers (more, most) by replacing them with 'less'
- Preserves DELTA_PAGER precedence over PAGER

The fix ensures that complex shell commands used as pagers work correctly
while maintaining all existing behavior for standard pagers.

Fixes issue where complex PAGER environment variables would fail with
"command not found" errors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Delta broken when PAGER env var has arguments
1 participant