Skip to content

Commit 17110bf

Browse files
authored
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.
1 parent d4bece4 commit 17110bf

3 files changed

Lines changed: 236 additions & 22 deletions

File tree

bin/git-forgit

Lines changed: 92 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@
1212
# This gives users the choice to set aliases inside of their git config instead
1313
# of their shell config if they prefer.
1414

15-
REQUIRED_FZF_VERSION="0.49.0"
15+
REQUIRED_FZF_VERSION="0.60.0"
16+
17+
# forgit-fzf separator used between the visible label and hidden payload.
18+
_ffsep=$'\x1f\x1e'
1619

1720
FORGIT_FZF_DEFAULT_OPTS="
1821
$FZF_DEFAULT_OPTS
@@ -217,17 +220,91 @@ _forgit_list_files() {
217220
#
218221
# Always includes modified and unmerged files. Includes untracked files when
219222
# status.showUntrackedFiles is true or unset. Never includes staged files.
223+
#
224+
# The output is formatted for `forgit add` and contains two fields separated by
225+
# `$_ffsep`:
226+
# 1. the human-readable status line shown in fzf
227+
# 2. the absolute path payload used by preview/edit/add actions
228+
# fzf only shows field 1 and uses field 2 for actions.
229+
#
230+
# Keeping the display text separate from the payload avoids reparsing quoted
231+
# `git status` output, which breaks for filenames containing backslashes.
220232
_forgit_worktree_changes() {
221-
local changed unmerged untracked show_untracked
233+
local changed reset rootdir show_untracked unmerged untracked
222234
changed=$(git config --get-color color.status.changed red)
223235
unmerged=$(git config --get-color color.status.unmerged red)
224236
untracked=$(git config --get-color color.status.untracked red)
237+
reset=$(git config --get-color '' reset)
225238
show_untracked=$(git config status.showUntrackedFiles)
239+
rootdir=$(git rev-parse --show-toplevel)
226240

227-
git -c color.status=always -c status.relativePaths=true status --porcelain -zs --untracked="${show_untracked:-all}" |
241+
git -c color.status=always status --porcelain -zs --untracked="${show_untracked:-all}" |
228242
tr '\0' '\n' |
243+
_forgit_restore_untracked_color "$untracked" "$reset" |
229244
grep -F -e "$changed" -e "$unmerged" -e "$untracked" |
230-
sed -E 's/^(..[^[:space:]]*)[[:space:]]+(.*)$/[\1] \2/'
245+
_forgit_build_status_entries "$rootdir"
246+
}
247+
248+
# Normalize plain `?? path` rows from older Git versions before the add-list
249+
# color filter runs.
250+
#
251+
# Input:
252+
# stdin - one status line per entry
253+
# $1 untracked - color sequence to apply to the `??` marker
254+
# $2 reset - reset sequence appended after the colored marker
255+
#
256+
# Output:
257+
# Writes the input lines back to stdout, but rewrites plain `?? path` rows as
258+
# colored `??` rows so untracked entries remain visible after filtering.
259+
_forgit_restore_untracked_color() {
260+
local reset untracked
261+
untracked=$1
262+
reset=$2
263+
264+
awk -v untracked="$untracked" -v reset="$reset" '
265+
/^\?\? / { sub(/^\?\? /, untracked "??" reset " ") }
266+
{ print }
267+
'
268+
}
269+
270+
# Build fzf-friendly entries from colored porcelain status lines.
271+
#
272+
# Input:
273+
# stdin - colored porcelain `git status --porcelain -z` rows, converted
274+
# to one line per entry
275+
# $1 rootdir - absolute repo root used to build the hidden payload
276+
#
277+
# Output:
278+
# Writes one line per entry with two `$_ffsep`-separated fields:
279+
# 1. the human-readable status line shown in fzf
280+
# 2. the absolute-path payload used by preview/edit/add actions
281+
_forgit_build_status_entries() {
282+
local cwd rootdir
283+
rootdir=$1
284+
cwd=$(pwd -P)
285+
286+
# Use a single Perl process so path normalization stays portable while the
287+
# full add-list transformation still runs as one batch pipeline stage.
288+
perl -MCwd=realpath -MFile::Spec -e '
289+
use strict;
290+
use warnings;
291+
292+
my ($rootdir, $cwd, $separator) = @ARGV;
293+
my $normalized_rootdir = realpath($rootdir);
294+
my $normalized_cwd = realpath($cwd);
295+
296+
while (my $line = <STDIN>) {
297+
chomp $line;
298+
next unless $line =~ /^(..[^[:space:]]*)( )(.*)$/;
299+
300+
my ($status, $repo_path) = ($1, $3);
301+
my $absolute_path = "$normalized_rootdir/$repo_path";
302+
my $display_path = File::Spec->abs2rel(realpath($absolute_path) // $absolute_path, $normalized_cwd);
303+
$display_path = "." if $display_path eq q{};
304+
305+
print "[$status] ${display_path}${separator}${normalized_rootdir}/${repo_path}\n";
306+
}
307+
' "$rootdir" "$cwd" "$_ffsep"
231308
}
232309

233310
_forgit_is_submodule() {
@@ -496,7 +573,8 @@ _forgit_show() {
496573
}
497574

498575
_forgit_add_preview() {
499-
file=$(echo "$1" | _forgit_get_single_file_from_add_line)
576+
local file
577+
file=$1
500578
# $file can be a directory when status.showUntrackedFiles is set to 'normal'
501579
# When this is the case and the directory is not a submodule show the content of the directory and return
502580
if [[ -d "$file" ]] && ! _forgit_is_submodule "$file"; then
@@ -516,17 +594,9 @@ _forgit_git_add() {
516594
git add "${_forgit_add_git_opts[@]}" "$@"
517595
}
518596

519-
_forgit_get_single_file_from_add_line() {
520-
# NOTE: paths listed by 'git status -su' mixed with quoted and unquoted style
521-
# remove indicators | remove original path for rename case | remove surrounding quotes
522-
sed 's/^.*] //' |
523-
sed 's/.* -> //' |
524-
sed -e 's/^\"//' -e 's/\"$//'
525-
}
526-
527597
_forgit_edit_add_file() {
528-
local input_line=$1
529-
filename=$(echo "$input_line" | _forgit_get_single_file_from_add_line)
598+
local filename
599+
filename=$1
530600
$EDITOR "$filename" >/dev/tty </dev/tty
531601
}
532602

@@ -539,15 +609,18 @@ _forgit_add() {
539609

540610
opts="
541611
$FORGIT_FZF_DEFAULT_OPTS
542-
-0 -m --nth 2..,..
543-
--preview=\"$FORGIT preview add_preview {}\"
544-
--bind=\"alt-e:execute($FORGIT edit_add_file {})+refresh-preview\"
612+
--delimiter=$_ffsep
613+
# Show only the formatted label in fzf, but return the hidden absolute
614+
# path payload so downstream actions never need to parse status lines.
615+
-0 -m --with-nth=1 --accept-nth=2
616+
--preview=\"$FORGIT preview add_preview {2}\"
617+
--bind=\"alt-e:execute($FORGIT edit_add_file {2})+refresh-preview\"
545618
$FORGIT_ADD_FZF_OPTS
546619
"
547620
files=()
548621
while IFS='' read -r file; do
549622
files+=("$file")
550-
done < <(_forgit_worktree_changes | FZF_DEFAULT_OPTS="$opts" fzf | _forgit_get_single_file_from_add_line)
623+
done < <(_forgit_worktree_changes | FZF_DEFAULT_OPTS="$opts" fzf)
551624
[[ "${#files[@]}" -gt 0 ]] && _forgit_git_add "$@" "${files[@]}" && git status -s && return
552625
echo 'Nothing to add.'
553626
}

tests/fzf.test.sh

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
#!/usr/bin/env bash
22

3+
function set_up_before_script() {
4+
source bin/git-forgit
5+
}
6+
37
function test_exit_when_fzf_is_not_installed() {
48
# Error code 127 = "command not found"
59
bashunit::mock fzf "return 127"
@@ -17,7 +21,7 @@ function test_exit_when_fzf_version_is_below_required_version() {
1721
output=$(bin/git-forgit)
1822

1923
assert_general_error
20-
assert_contains "fzf version 0.49.0 or higher is required" "$output"
24+
assert_contains "fzf version $REQUIRED_FZF_VERSION or higher is required" "$output"
2125
}
2226

2327
function fzf_versions_below_required_version() {
@@ -38,8 +42,8 @@ function test_pass_when_fzf_version_satisfies_required_version() {
3842
}
3943

4044
function fzf_versions_satisfying_required_version() {
41-
echo "0.49.0"
42-
echo "0.49.1"
45+
echo "0.60.0"
46+
echo "0.60.1"
4347
echo "0.80.0"
4448
echo "1.1.0"
4549
}

tests/working-tree-changes.test.sh

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#!/usr/bin/env bash
22

3+
FORGIT_REPO_ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)
4+
35
function set_up_before_script() {
46
source bin/git-forgit
57

@@ -65,3 +67,138 @@
6567

6668
assert_contains 'untracked_with_\backslash' "$output"
6769
}
70+
71+
function test_forgit_build_status_entries_uses_cwd_relative_display_paths_within_subdirectories() {
72+
local output
73+
74+
mkdir -p dir
75+
touch dir/file.txt
76+
cd dir || return 1
77+
78+
output=$(_forgit_worktree_changes)
79+
80+
assert_contains "file.txt" "$output"
81+
}
82+
83+
function test_forgit_build_status_entries_prefixes_parent_paths_for_entries_outside_cwd() {
84+
local output
85+
86+
mkdir -p dir other
87+
touch other/file.txt
88+
cd dir || return 1
89+
90+
output=$(_forgit_worktree_changes)
91+
92+
assert_contains "../other/file.txt" "$output"
93+
}
94+
95+
function test_forgit_build_status_entries_keeps_repo_relative_paths_at_repo_root() {
96+
local output
97+
98+
mkdir -p dir
99+
touch dir/file.txt
100+
101+
output=$(_forgit_worktree_changes)
102+
103+
assert_contains "dir/file.txt" "$output"
104+
}
105+
106+
function test_forgit_build_status_entries_uses_cwd_relative_display_paths_for_sibling_entries() {
107+
local output
108+
109+
mkdir -p dir1 dir2
110+
touch dir1/file.txt
111+
cd dir2 || return 1
112+
113+
output=$(_forgit_worktree_changes)
114+
115+
assert_contains "../dir1/file.txt" "$output"
116+
}
117+
118+
function test_forgit_build_status_entries_uses_cwd_relative_display_paths_from_logical_symlink_paths() {
119+
local output sandbox
120+
121+
sandbox=$(bashunit::temp_dir)
122+
mkdir -p "$sandbox/real/dir1" "$sandbox/real/dir2"
123+
(
124+
cd "$sandbox/real" || exit 1
125+
git init --quiet
126+
git config user.name "Test User"
127+
git config user.email "test@example.com"
128+
) || return 1
129+
ln -s "$sandbox/real" "$sandbox/link"
130+
cd "$sandbox/link/dir2" || return 1
131+
touch ../dir1/file.txt
132+
133+
output=$(_forgit_worktree_changes)
134+
135+
assert_contains "../dir1/file.txt" "$output"
136+
}
137+
138+
function test_forgit_worktree_changes_emits_absolute_payloads_for_subdir_entries() {
139+
local output rootdir
140+
141+
mkdir dir
142+
touch 'dir/with_\backslash'
143+
cd dir || return 1
144+
145+
output=$(_forgit_worktree_changes)
146+
rootdir=$(git rev-parse --show-toplevel)
147+
148+
assert_contains $'with_\\backslash'"$_ffsep""$rootdir/dir/with_\\backslash" "$output"
149+
}
150+
151+
function test_forgit_restore_untracked_color_colorizes_plain_untracked_lines() {
152+
local output
153+
154+
output=$(printf '?? plain.txt\n' | _forgit_restore_untracked_color '<u>' '<r>')
155+
156+
assert_same '<u>??<r> plain.txt' "$output"
157+
}
158+
159+
function test_forgit_restore_untracked_color_leaves_colored_lines_unchanged() {
160+
local colored output
161+
162+
colored=$'\033[33m??\033[m plain.txt'
163+
output=$(printf '%s\n' "$colored" | _forgit_restore_untracked_color '<u>' '<r>')
164+
165+
assert_same "$colored" "$output"
166+
}
167+
168+
function test_forgit_worktree_changes_preserves_special_characters_in_payload() {
169+
local output path rootdir
170+
171+
path=$'tab\t space \\ name.txt'
172+
touch "$path"
173+
rootdir=$(git rev-parse --show-toplevel)
174+
175+
output=$(_forgit_worktree_changes)
176+
177+
assert_contains "${path}${_ffsep}${rootdir}/${path}" "$output"
178+
}
179+
180+
function test_forgit_fzf_separator_does_not_use_literal_tabs() {
181+
local delimiter
182+
183+
delimiter=$_ffsep
184+
185+
assert_not_contains $'\t' "$delimiter"
186+
}
187+
188+
function test_forgit_worktree_changes_works_in_zsh() {
189+
local output
190+
191+
output=$(
192+
zsh -c '
193+
source "'"$FORGIT_REPO_ROOT"'/bin/git-forgit"
194+
cd "$(mktemp -d)" || exit 1
195+
git init --quiet
196+
touch "space name.txt" "back\\slash.txt" $'"'"'tab\tname.txt'"'"'
197+
_forgit_worktree_changes
198+
'
199+
)
200+
201+
assert_contains 'space name.txt' "$output"
202+
assert_contains 'back\slash.txt' "$output"
203+
assert_contains 'tab' "$output"
204+
}

0 commit comments

Comments
 (0)