From f7e2d436deb93127fd9bc0c50c0b11cf096310dc Mon Sep 17 00:00:00 2001 From: Wenxuan Date: Sat, 14 Mar 2026 10:38:44 +0800 Subject: [PATCH] fix: use non-interactive pager in fzf preview for TUI pagers (#498) When users configure a TUI/interactive diff pager (e.g., diffnav, tig) via `git config pager.diff`, fzf preview panes show blank output because TUI pagers require a TTY that fzf previews don't provide. Add FORGIT_PREVIEW_PAGER env var that overrides the diff pager only in fzf preview context. Preview detection relies on a FORGIT_IN_PREVIEW marker set by a new `_forgit_preview` wrapper, so the override applies only to preview commands and leaves fullscreen behavior unchanged. Fixes #491 --- README.md | 3 ++ bin/git-forgit | 71 ++++++++++++++++++++++------------- tests/preview-context.test.sh | 68 +++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 27 deletions(-) create mode 100644 tests/preview-context.test.sh diff --git a/README.md b/README.md index 1d78e48..ca0cf68 100644 --- a/README.md +++ b/README.md @@ -299,6 +299,9 @@ variables: | `FORGIT_BLAME_PAGER` | `git config pager.blame` _or_ `$FORGIT_PAGER` | | `FORGIT_IGNORE_PAGER` | `bat -l gitignore --color always` _or_ `cat` | | `FORGIT_ATTRIBUTES_PAGER` | `bat -l gitattributes --color always` _or_ `cat` | +| `FORGIT_PREVIEW_PAGER` | Normal pager resolution* | + +* If your pager is a TUI program (e.g., `diffnav`, `tig`), fzf preview panes will be blank because they run without a TTY. Set `FORGIT_PREVIEW_PAGER` to a non-interactive pager (e.g., `delta`) to fix this. When set, it overrides all other `FORGIT_*_PAGER` settings in fzf preview context. ### FZF Options diff --git a/bin/git-forgit b/bin/git-forgit index 57ab4e5..c50994a 100755 --- a/bin/git-forgit +++ b/bin/git-forgit @@ -131,11 +131,27 @@ _forgit_dir_view=${FORGIT_DIR_VIEW:-$(hash tree &> /dev/null && echo 'tree' || e _forgit_pager() { local pager - pager=$(_forgit_get_pager "$1") + # Preview mode must be marked explicitly by forgit. Inferred signals such + # as FZF_PREVIEW_COLUMNS or a non-TTY stdout also show up in execute/fullscreen + # paths, which would incorrectly route Enter actions to FORGIT_PREVIEW_PAGER. + if [[ -n "$FORGIT_IN_PREVIEW" ]] && [[ -n "$FORGIT_PREVIEW_PAGER" ]]; then + pager="$FORGIT_PREVIEW_PAGER" + else + pager=$(_forgit_get_pager "$1") + fi [[ -z "${pager}" ]] && exit 1 eval "${pager} ${*:2}" } +_forgit_preview() { + local cmd=$1 + shift + # Funnel all fzf --preview commands through a single wrapper so preview-only + # pager behavior is controlled by an explicit marker instead of ambient env. + export FORGIT_IN_PREVIEW=1 + _forgit_"${cmd}" "$@" +} + _forgit_get_pager() { local pager pager=${1:-core} @@ -225,7 +241,7 @@ _forgit_log() { +s +m --tiebreak=index --bind=\"enter:execute($FORGIT log_enter {} $quoted_files)\" --bind=\"ctrl-y:execute-silent($FORGIT yank_sha {})\" - --preview=\"$FORGIT log_preview {} $quoted_files\" + --preview=\"$FORGIT preview log_preview {} $quoted_files\" $FORGIT_LOG_FZF_OPTS " graph=() @@ -252,7 +268,7 @@ _forgit_reflog() { +s +m --tiebreak=index --bind=\"enter:execute($FORGIT log_enter {})\" --bind=\"ctrl-y:execute-silent($FORGIT yank_sha {})\" - --preview=\"$FORGIT log_preview {}\" + --preview=\"$FORGIT preview log_preview {}\" $FORGIT_REFLOG_FZF_OPTS " reflog_format=${FORGIT_GRL_FORMAT:-$_forgit_log_format} @@ -357,7 +373,7 @@ _forgit_diff() { opts=" $FORGIT_FZF_DEFAULT_OPTS +m -0 --bind=\"enter:execute($FORGIT diff_enter {} $escaped_commits | $FORGIT pager enter)\" - --preview=\"$FORGIT diff_view {} '$_forgit_preview_context' $escaped_commits\" + --preview=\"$FORGIT preview diff_view {} '$_forgit_preview_context' $escaped_commits\" --bind=\"alt-e:execute($FORGIT edit_diffed_file {})+refresh-preview\" $FORGIT_DIFF_FZF_OPTS --prompt=\"${commits[*]} > \" @@ -427,7 +443,7 @@ _forgit_show() { opts=" $FORGIT_FZF_DEFAULT_OPTS +m -0 --bind=\"enter:execute($FORGIT show_enter {} $escaped_commit | $FORGIT pager enter)\" - --preview=\"$FORGIT show_preview {} '$_forgit_preview_context' $escaped_commit\" + --preview=\"$FORGIT preview show_preview {} '$_forgit_preview_context' $escaped_commit\" --preview-label=\" Diff \" --bind=\"alt-e:execute($FORGIT edit_diffed_file {})+refresh-preview\" --bind=\"alt-t:transform:[[ ! \\\"\$FZF_PREVIEW_LABEL\\\" =~ 'Diff' ]] && @@ -499,7 +515,7 @@ _forgit_add() { opts=" $FORGIT_FZF_DEFAULT_OPTS -0 -m --nth 2..,.. - --preview=\"$FORGIT add_preview {}\" + --preview=\"$FORGIT preview add_preview {}\" --bind=\"alt-e:execute($FORGIT edit_add_file {})+refresh-preview\" $FORGIT_ADD_FZF_OPTS " @@ -536,7 +552,7 @@ _forgit_reset_head() { opts=" $FORGIT_FZF_DEFAULT_OPTS -m -0 - --preview=\"$FORGIT reset_head_preview '$rootdir'/{}\" + --preview=\"$FORGIT preview reset_head_preview '$rootdir'/{}\" $FORGIT_RESET_HEAD_FZF_OPTS " files=() @@ -576,7 +592,7 @@ _forgit_stash_show() { $FORGIT_FZF_DEFAULT_OPTS +s +m -0 --tiebreak=index --bind=\"enter:execute($FORGIT stash_show_enter {})\" --bind=\"ctrl-y:execute-silent($FORGIT yank_stash_name {})\" - --preview=\"$FORGIT stash_show_preview {}\" + --preview=\"$FORGIT preview stash_show_preview {}\" $FORGIT_STASH_FZF_OPTS " git stash list "${_forgit_stash_show_git_opts[@]}" | FZF_DEFAULT_OPTS="$opts" fzf @@ -622,7 +638,7 @@ _forgit_stash_push() { opts=" $FORGIT_FZF_DEFAULT_OPTS -m - --preview=\"$FORGIT stash_push_preview {}\" + --preview=\"$FORGIT preview stash_push_preview {}\" $FORGIT_STASH_PUSH_FZF_OPTS " # Show both modified and untracked files @@ -649,7 +665,7 @@ _forgit_clean_select_files() { local opts opts=" $FORGIT_FZF_DEFAULT_OPTS - --preview=\"$FORGIT clean_preview {}\" + --preview=\"$FORGIT preview clean_preview {}\" -m -0 $FORGIT_CLEAN_FZF_OPTS " @@ -693,7 +709,7 @@ _forgit_cherry_pick() { opts=" $FORGIT_FZF_DEFAULT_OPTS - --preview=\"$FORGIT cherry_pick_preview {}\" + --preview=\"$FORGIT preview cherry_pick_preview {}\" --multi --ansi --with-nth 2.. -0 --tiebreak=index $FORGIT_CHERRY_PICK_FZF_OPTS " @@ -736,7 +752,7 @@ _forgit_cherry_pick_from_branch() { opts=" $FORGIT_FZF_DEFAULT_OPTS +s +m --tiebreak=index --header-lines=1 - --preview=\"$FORGIT cherry_pick_from_branch_preview '$base' {}\" + --preview=\"$FORGIT preview cherry_pick_from_branch_preview '$base' {}\" $FORGIT_CHERRY_PICK_FROM_BRANCH_FZF_OPTS " # loop until either the branch selector is closed or a commit to be cherry @@ -771,7 +787,7 @@ _forgit_rebase() { $FORGIT_FZF_DEFAULT_OPTS +s +m --tiebreak=index --bind=\"ctrl-y:execute-silent($FORGIT yank_sha {})\" - --preview=\"$FORGIT file_preview {}\" + --preview=\"$FORGIT preview file_preview {}\" $FORGIT_REBASE_FZF_OPTS " target_commit=$( @@ -818,7 +834,7 @@ _forgit_edit_commit() { $FORGIT_FZF_DEFAULT_OPTS +s +m --tiebreak=index --bind=\"ctrl-y:execute-silent($FORGIT yank_sha {})\" - --preview=\"$FORGIT file_preview {} $quoted_files\" + --preview=\"$FORGIT preview file_preview {} $quoted_files\" $fzf_opts " target_commit=$( @@ -847,7 +863,7 @@ _forgit_reword() { $FORGIT_FZF_DEFAULT_OPTS +s +m --tiebreak=index --bind=\"ctrl-y:execute-silent($FORGIT yank_sha {})\" - --preview=\"$FORGIT file_preview {} $quoted_files\" + --preview=\"$FORGIT preview file_preview {} $quoted_files\" $FORGIT_REWORD_FZF_OPTS " target_commit=$( @@ -882,7 +898,7 @@ _forgit_checkout_file() { opts=" $FORGIT_FZF_DEFAULT_OPTS -m -0 - --preview=\"$FORGIT checkout_file_preview {}\" + --preview=\"$FORGIT preview checkout_file_preview {}\" $FORGIT_CHECKOUT_FILE_FZF_OPTS " files=() @@ -918,7 +934,7 @@ _forgit_checkout_branch() { opts=" $FORGIT_FZF_DEFAULT_OPTS +s +m --tiebreak=index --header-lines=1 - --preview=\"$FORGIT branch_preview {}\" + --preview=\"$FORGIT preview branch_preview {}\" $FORGIT_CHECKOUT_BRANCH_FZF_OPTS " _forgit_checkout_branch_branch_git_opts=() @@ -964,7 +980,7 @@ _forgit_switch_branch() { opts=" $FORGIT_FZF_DEFAULT_OPTS +s +m --tiebreak=index --header-lines=1 - --preview=\"$FORGIT branch_preview {}\" + --preview=\"$FORGIT preview branch_preview {}\" $FORGIT_SWITCH_BRANCH_FZF_OPTS " _forgit_switch_branch_branch_git_opts=() @@ -1001,7 +1017,7 @@ _forgit_checkout_tag() { opts=" $FORGIT_FZF_DEFAULT_OPTS +s +m --tiebreak=index - --preview=\"$FORGIT branch_preview {}\" + --preview=\"$FORGIT preview branch_preview {}\" $FORGIT_CHECKOUT_TAG_FZF_OPTS " tag="$(git tag -l --sort=-v:refname | FZF_DEFAULT_OPTS="$opts" fzf)" @@ -1030,7 +1046,7 @@ _forgit_checkout_commit() { $FORGIT_FZF_DEFAULT_OPTS +s +m --tiebreak=index --bind=\"ctrl-y:execute-silent($FORGIT yank_sha {})\" - --preview=\"$FORGIT checkout_commit_preview {}\" + --preview=\"$FORGIT preview checkout_commit_preview {}\" $FORGIT_CHECKOUT_COMMIT_FZF_OPTS " graph=() @@ -1063,7 +1079,7 @@ _forgit_branch_delete() { opts=" $FORGIT_FZF_DEFAULT_OPTS +s --multi --tiebreak=index --header-lines=1 - --preview=\"$FORGIT branch_preview {}\" + --preview=\"$FORGIT preview branch_preview {}\" $FORGIT_BRANCH_DELETE_FZF_OPTS " for branch in $(_forgit_branch_list | FZF_DEFAULT_OPTS="$opts" fzf | _forgit_extract_branch_name) @@ -1094,7 +1110,7 @@ _forgit_revert_commit() { $FORGIT_FZF_DEFAULT_OPTS -m +s --tiebreak=index --ansi --with-nth 2.. - --preview=\"$FORGIT revert_preview {}\" + --preview=\"$FORGIT preview revert_preview {}\" $FORGIT_REVERT_COMMIT_FZF_OPTS " graph=() @@ -1149,7 +1165,7 @@ _forgit_blame() { done < <(git rev-parse --flags "$@") opts=" $FORGIT_FZF_DEFAULT_OPTS - --preview=\"$FORGIT blame_preview {} ${flags[*]}\" + --preview=\"$FORGIT preview blame_preview {} ${flags[*]}\" $FORGIT_BLAME_FZF_OPTS " # flags is not quoted here, which is fine given that they are retrieved @@ -1184,7 +1200,7 @@ _forgit_ignore() { opts=" $FORGIT_FZF_DEFAULT_OPTS -m --preview-window='right:70%' - --preview=\"$FORGIT path_preview $FORGIT_GI_TEMPLATES {2} .gitignore ignore\" + --preview=\"$FORGIT preview path_preview $FORGIT_GI_TEMPLATES {2} .gitignore ignore\" $FORGIT_IGNORE_FZF_OPTS " args=("$@") @@ -1212,7 +1228,7 @@ _forgit_attributes() { opts=" $FORGIT_FZF_DEFAULT_OPTS -m --preview-window='right:70%' - --preview=\"$FORGIT path_preview $FORGIT_ATTR_TEMPLATES {2} .gitattributes attributes\" + --preview=\"$FORGIT preview path_preview $FORGIT_ATTR_TEMPLATES {2} .gitattributes attributes\" $FORGIT_ATTRIBUTES_FZF_OPTS " args=("$@") @@ -1371,7 +1387,7 @@ _forgit_worktree() { opts=" $FORGIT_FZF_DEFAULT_OPTS +s +m --tiebreak=index - --preview=\"$FORGIT worktree_preview {}\" + --preview=\"$FORGIT preview worktree_preview {}\" --bind=\"ctrl-y:execute-silent($FORGIT worktree_yank_path {})\" --bind=\"alt-l:execute-silent($FORGIT worktree_toggle_lock {})+reload($FORGIT worktree_list)\" $FORGIT_WORKTREE_FZF_OPTS @@ -1397,7 +1413,7 @@ _forgit_worktree_delete() { opts=" $FORGIT_FZF_DEFAULT_OPTS +s --multi --tiebreak=index - --preview=\"$FORGIT worktree_preview {}\" + --preview=\"$FORGIT preview worktree_preview {}\" --bind=\"ctrl-y:execute-silent($FORGIT worktree_yank_path {})\" --bind=\"alt-l:execute-silent($FORGIT worktree_toggle_lock {})+reload($FORGIT worktree_list_deletable)\" $FORGIT_WORKTREE_DELETE_FZF_OPTS @@ -1551,6 +1567,7 @@ PRIVATE_COMMANDS=( "exec_show" "file_preview" "path_preview" + "preview" "revert_preview" "reset_head_preview" "show_enter" diff --git a/tests/preview-context.test.sh b/tests/preview-context.test.sh new file mode 100644 index 0000000..a439a3d --- /dev/null +++ b/tests/preview-context.test.sh @@ -0,0 +1,68 @@ +#!/usr/bin/env bash + +function set_up_before_script() { + source bin/git-forgit + FORGIT="bin/git-forgit" + + # Ignore global git config files + export GIT_CONFIG_SYSTEM=/dev/null + export GIT_CONFIG_GLOBAL=/dev/null + + cd "$(bashunit::temp_dir)" || return 1 + git init --quiet + git config user.name "Test User" + git config user.email "test@example.com" + touch tracked.txt + git add tracked.txt + git commit -m "init" --quiet +} + +function test_forgit_pager_uses_preview_pager_only_in_explicit_preview_context() { + local without_marker with_marker + + without_marker=$( + FORGIT_DIFF_PAGER="printf diff" \ + FORGIT_PREVIEW_PAGER="printf preview" \ + FZF_PREVIEW_COLUMNS=80 \ + _forgit_pager diff + ) + with_marker=$( + FORGIT_DIFF_PAGER="printf diff" \ + FORGIT_PREVIEW_PAGER="printf preview" \ + FORGIT_IN_PREVIEW=1 \ + _forgit_pager diff + ) + + assert_same "diff" "$without_marker" + assert_same "preview" "$with_marker" +} + +function _forgit_capture_preview_flag() { + printf '%s' "${FORGIT_IN_PREVIEW:-unset}" +} + +function test_forgit_preview_wrapper_marks_preview_context() { + local actual + + actual=$(_forgit_preview capture_preview_flag 2>/dev/null || true) + + assert_same "1" "$actual" +} + +function test_forgit_diff_preview_command_uses_preview_wrapper() { + bashunit::mock "fzf" 'printf "%s" "$FZF_DEFAULT_OPTS"' + + local output + output=$(_forgit_diff) + + assert_contains "--preview=\"$FORGIT preview diff_view {}" "$output" +} + +function test_forgit_show_preview_command_uses_preview_wrapper() { + bashunit::mock "fzf" 'printf "%s" "$FZF_DEFAULT_OPTS"' + + local output + output=$(_forgit_show HEAD) + + assert_contains "--preview=\"$FORGIT preview show_preview {}" "$output" +}