From 17110bfc611209f00a3faeeb318aef67d636a0e3 Mon Sep 17 00:00:00 2001 From: Wenxuan Date: Wed, 8 Apr 2026 10:40:03 +0800 Subject: [PATCH] fix: preserve add paths across special filenames (#506) Keep `forgit add` on the porcelain `git status --porcelain -zs` path so filenames containing backslashes continue to work, while restoring the behaviors that regressed when we moved away from Git's cwd-relative output. This change makes the status picker emit a display label and a hidden absolute-path payload separately. That lets the UI keep showing intuitive cwd-relative paths, while preview, edit, and add actions operate on the real path instead of reparsing the rendered status line. As a result, untracked files, subdirectory workflows, and special filenames now share one consistent path flow. It also restores the old-Git fallback for plain `?? path` output before status filtering, raises the required fzf version for `--accept-nth`, and adds regression coverage for backslashes, spaces, tabs, subdirectory entries, sibling directories, and logical symlink paths. We explored a few alternatives before landing here. Keeping a single human-readable line and reparsing it downstream remained too fragile for quoted paths and backslashes. Shell-only display-path rewriting worked for some cases but stayed brittle across logical vs physical paths and still failed in macOS CI. A per-path `realpath` approach would have been easier to read, but GNU-style relative-path support is not portable across the platforms we test and would add one external process per file in a hot path. The final tradeoff keeps the pipeline batch-oriented and portable by doing the path normalization once in a single helper step, even though that is less lightweight than the earlier shell-only versions. That complexity is justified here because it fixes the old-Git untracked regression, preserves correct preview/add behavior from subdirectories, and avoids reintroducing long-standing filename parsing bugs. --- bin/git-forgit | 111 +++++++++++++++++++---- tests/fzf.test.sh | 10 ++- tests/working-tree-changes.test.sh | 137 +++++++++++++++++++++++++++++ 3 files changed, 236 insertions(+), 22 deletions(-) diff --git a/bin/git-forgit b/bin/git-forgit index cd7b2da..55c8bca 100755 --- a/bin/git-forgit +++ b/bin/git-forgit @@ -12,7 +12,10 @@ # This gives users the choice to set aliases inside of their git config instead # of their shell config if they prefer. -REQUIRED_FZF_VERSION="0.49.0" +REQUIRED_FZF_VERSION="0.60.0" + +# forgit-fzf separator used between the visible label and hidden payload. +_ffsep=$'\x1f\x1e' FORGIT_FZF_DEFAULT_OPTS=" $FZF_DEFAULT_OPTS @@ -217,17 +220,91 @@ _forgit_list_files() { # # Always includes modified and unmerged files. Includes untracked files when # status.showUntrackedFiles is true or unset. Never includes staged files. +# +# The output is formatted for `forgit add` and contains two fields separated by +# `$_ffsep`: +# 1. the human-readable status line shown in fzf +# 2. the absolute path payload used by preview/edit/add actions +# fzf only shows field 1 and uses field 2 for actions. +# +# Keeping the display text separate from the payload avoids reparsing quoted +# `git status` output, which breaks for filenames containing backslashes. _forgit_worktree_changes() { - local changed unmerged untracked show_untracked + local changed reset rootdir show_untracked unmerged untracked changed=$(git config --get-color color.status.changed red) unmerged=$(git config --get-color color.status.unmerged red) untracked=$(git config --get-color color.status.untracked red) + reset=$(git config --get-color '' reset) show_untracked=$(git config status.showUntrackedFiles) + rootdir=$(git rev-parse --show-toplevel) - git -c color.status=always -c status.relativePaths=true status --porcelain -zs --untracked="${show_untracked:-all}" | + git -c color.status=always status --porcelain -zs --untracked="${show_untracked:-all}" | tr '\0' '\n' | + _forgit_restore_untracked_color "$untracked" "$reset" | grep -F -e "$changed" -e "$unmerged" -e "$untracked" | - sed -E 's/^(..[^[:space:]]*)[[:space:]]+(.*)$/[\1] \2/' + _forgit_build_status_entries "$rootdir" +} + +# Normalize plain `?? path` rows from older Git versions before the add-list +# color filter runs. +# +# Input: +# stdin - one status line per entry +# $1 untracked - color sequence to apply to the `??` marker +# $2 reset - reset sequence appended after the colored marker +# +# Output: +# Writes the input lines back to stdout, but rewrites plain `?? path` rows as +# colored `??` rows so untracked entries remain visible after filtering. +_forgit_restore_untracked_color() { + local reset untracked + untracked=$1 + reset=$2 + + awk -v untracked="$untracked" -v reset="$reset" ' + /^\?\? / { sub(/^\?\? /, untracked "??" reset " ") } + { print } + ' +} + +# Build fzf-friendly entries from colored porcelain status lines. +# +# Input: +# stdin - colored porcelain `git status --porcelain -z` rows, converted +# to one line per entry +# $1 rootdir - absolute repo root used to build the hidden payload +# +# Output: +# Writes one line per entry with two `$_ffsep`-separated fields: +# 1. the human-readable status line shown in fzf +# 2. the absolute-path payload used by preview/edit/add actions +_forgit_build_status_entries() { + local cwd rootdir + rootdir=$1 + cwd=$(pwd -P) + + # Use a single Perl process so path normalization stays portable while the + # full add-list transformation still runs as one batch pipeline stage. + perl -MCwd=realpath -MFile::Spec -e ' + use strict; + use warnings; + + my ($rootdir, $cwd, $separator) = @ARGV; + my $normalized_rootdir = realpath($rootdir); + my $normalized_cwd = realpath($cwd); + + while (my $line = ) { + chomp $line; + next unless $line =~ /^(..[^[:space:]]*)( )(.*)$/; + + my ($status, $repo_path) = ($1, $3); + my $absolute_path = "$normalized_rootdir/$repo_path"; + my $display_path = File::Spec->abs2rel(realpath($absolute_path) // $absolute_path, $normalized_cwd); + $display_path = "." if $display_path eq q{}; + + print "[$status] ${display_path}${separator}${normalized_rootdir}/${repo_path}\n"; + } + ' "$rootdir" "$cwd" "$_ffsep" } _forgit_is_submodule() { @@ -496,7 +573,8 @@ _forgit_show() { } _forgit_add_preview() { - file=$(echo "$1" | _forgit_get_single_file_from_add_line) + local file + file=$1 # $file can be a directory when status.showUntrackedFiles is set to 'normal' # When this is the case and the directory is not a submodule show the content of the directory and return if [[ -d "$file" ]] && ! _forgit_is_submodule "$file"; then @@ -516,17 +594,9 @@ _forgit_git_add() { git add "${_forgit_add_git_opts[@]}" "$@" } -_forgit_get_single_file_from_add_line() { - # NOTE: paths listed by 'git status -su' mixed with quoted and unquoted style - # remove indicators | remove original path for rename case | remove surrounding quotes - sed 's/^.*] //' | - sed 's/.* -> //' | - sed -e 's/^\"//' -e 's/\"$//' -} - _forgit_edit_add_file() { - local input_line=$1 - filename=$(echo "$input_line" | _forgit_get_single_file_from_add_line) + local filename + filename=$1 $EDITOR "$filename" >/dev/tty ' '') + + assert_same '?? plain.txt' "$output" + } + + function test_forgit_restore_untracked_color_leaves_colored_lines_unchanged() { + local colored output + + colored=$'\033[33m??\033[m plain.txt' + output=$(printf '%s\n' "$colored" | _forgit_restore_untracked_color '' '') + + assert_same "$colored" "$output" + } + + function test_forgit_worktree_changes_preserves_special_characters_in_payload() { + local output path rootdir + + path=$'tab\t space \\ name.txt' + touch "$path" + rootdir=$(git rev-parse --show-toplevel) + + output=$(_forgit_worktree_changes) + + assert_contains "${path}${_ffsep}${rootdir}/${path}" "$output" +} + + function test_forgit_fzf_separator_does_not_use_literal_tabs() { + local delimiter + + delimiter=$_ffsep + + assert_not_contains $'\t' "$delimiter" + } + + function test_forgit_worktree_changes_works_in_zsh() { + local output + + output=$( + zsh -c ' + source "'"$FORGIT_REPO_ROOT"'/bin/git-forgit" + cd "$(mktemp -d)" || exit 1 + git init --quiet + touch "space name.txt" "back\\slash.txt" $'"'"'tab\tname.txt'"'"' + _forgit_worktree_changes + ' + ) + + assert_contains 'space name.txt' "$output" + assert_contains 'back\slash.txt' "$output" + assert_contains 'tab' "$output" + }