[PATCH v3 0/4] clean up some code around editors

["D. Ben Knoble" <ben.knoble+github@gmail.com>]

Changes from v2:
- shuffle setup code and use more helpers in 1/4
- insert 2/4 to stop abusing --exec-path
- improve environment-cleansing idioms in {2 => 3}/4

Thanks especially to Phillip's encyclopaedic knowledge of test helpers ;)

Changes from v1:
- add a prep patch with style fixes to t7005
- rework the environment munging to use subshells, per Phillip Wood's
  suggestion

This reroll of the previous exec-path series is simplified to contain
only the first 2 cleanup patches, which were largely acked by the list.
Drop the controversial and broken PATH munging.

Also, this version is (still) based on a later master 112648dd6b (Merge
branch 'master' of https://github.com/j6t/git-gui, 2025-08-04) than the
original from May.

These patches clean up some old code in the editor tests and subsystem
that does not use our modern idioms.

v1: https://lore.kernel.org/git/20250520193506.95199-1-ben.knoble+github@gmail.com/
v2: https://lore.kernel.org/git/20250810160323.49372-1-ben.knoble+github@gmail.com/
Published-as: https://github.com/benknoble/tree/editor-cleanup

D. Ben Knoble (4):
  t7005: use modern test style
  t7005: stop abusing --exec-path
  t7005: sanitize test environment for subsequent tests
  editor: use standard strvec API to receive environment for external
    editors

 builtin/commit.c  |   2 +-
 editor.c          |  10 ++--
 editor.h          |   7 ++-
 t/t7005-editor.sh | 147 +++++++++++++++++++---------------------------
 4 files changed, 69 insertions(+), 97 deletions(-)

Diff-intervalle contre v2 :
1:  e7629202a1 ! 1:  8ad2904a18 t7005: use modern test style
    @@ Metadata
      ## Commit message ##
         t7005: use modern test style
     
    -    Tests in t7005 mask Git error codes and do not use our nice helpers for
    -    comparing results. Improve that, and drop a few old-style blank lines
    -    while at it.
    +    Tests in t7005 mask Git error codes and do not use our nice test
    +    helpers. Improve that, move some code into the setup test, and drop a
    +    few old-style blank lines while at it.
     
    +    Best-viewed-with: --ignore-all-space
         Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
     
      ## t/t7005-editor.sh ##
    @@ t/t7005-editor.sh
     -
      '
      
    - if ! expr "$vi" : '[a-z]*$' >/dev/null
    -@@
    - fi
    - 
    - test_expect_success setup '
    +-if ! expr "$vi" : '[a-z]*$' >/dev/null
    +-then
    +-	vi=
    +-fi
     -
    +-for i in GIT_EDITOR core_editor EDITOR VISUAL $vi
    +-do
    +-	cat >e-$i.sh <<-EOF
    +-	#!$SHELL_PATH
    +-	echo "Edited by $i" >"\$1"
    +-	EOF
    +-	chmod +x e-$i.sh
    +-done
    +-
    +-if ! test -z "$vi"
    +-then
    +-	mv e-$vi.sh $vi
    +-fi
    +-
    + test_expect_success setup '
    ++	if ! expr "$vi" : "[a-z]*$" >/dev/null
    ++	then
    ++		vi=
    ++	fi &&
    ++
    ++	for i in GIT_EDITOR core_editor EDITOR VISUAL $vi
    ++	do
    ++		write_script e-$i.sh <<-EOF || return 1
    ++			echo "Edited by $i" >"\$1"
    ++		EOF
    ++	done &&
    ++
    ++	if ! test -z "$vi"
    ++	then
    ++		mv e-$vi.sh $vi
    ++	fi &&
    + 
      	msg="Hand-edited" &&
      	test_commit "$msg" &&
    - 	echo "$msg" >expect &&
    +-	echo "$msg" >expect &&
     -	git show -s --format=%s > actual &&
    -+	git show -s --format=%s >actual &&
    - 	test_cmp expect actual
    +-	test_cmp expect actual
     -
    ++	test_commit_message HEAD -m "$msg"
      '
      
      TERM=dumb
      export TERM
      test_expect_success 'dumb should error out when falling back on vi' '
     -
    - 	if git commit --amend
    - 	then
    - 		echo "Oops?"
    -@@
    +-	if git commit --amend
    +-	then
    +-		echo "Oops?"
    +-		false
    +-	else
    +-		: happy
    +-	fi
    ++	test_must_fail git commit --amend
      '
      
      test_expect_success 'dumb should prefer EDITOR to VISUAL' '
    @@ t/t7005-editor.sh
      	git commit --amend &&
     -	test "$(git show -s --format=%s)" = "Edited by EDITOR"
     -
    -+	echo "Edited by EDITOR" >expect &&
    -+	git show -s --format=%s >actual &&
    -+	test_cmp expect actual
    ++	test_commit_message HEAD -m "Edited by EDITOR"
      '
      
      TERM=vt100
    @@ t/t7005-editor.sh
      		git --exec-path=. commit --amend &&
     -		git show -s --pretty=oneline |
     -		sed -e "s/^[0-9a-f]* //" >actual &&
    -+		git show -s --pretty=oneline >show &&
    -+		<show sed -e "s/^[0-9a-f]* //" >actual &&
    - 		test_cmp expect actual
    +-		test_cmp expect actual
    ++		test_commit_message HEAD expect
      	'
      done
    + 
     @@
      	esac
      	test_expect_success "Using $i (override)" '
      		git --exec-path=. commit --amend &&
     -		git show -s --pretty=oneline |
     -		sed -e "s/^[0-9a-f]* //" >actual &&
    -+		git show -s --pretty=oneline >show &&
    -+		<show sed -e "s/^[0-9a-f]* //" >actual &&
    - 		test_cmp expect actual
    +-		test_cmp expect actual
    ++		test_commit_message HEAD expect
      	'
      done
    + 
     @@
      	echo "echo space >\"\$1\"" >"e space.sh" &&
      	chmod a+x "e space.sh" &&
      	GIT_EDITOR="./e\ space.sh" git commit --amend &&
     -	test space = "$(git show -s --pretty=format:%s)"
     -
    -+	echo space >expect &&
    -+	git show -s --pretty=tformat:%s >actual &&
    -+	test_cmp expect actual
    ++	test_commit_message HEAD -m space
      '
      
      unset GIT_EDITOR
    @@ t/t7005-editor.sh
      	git commit --amend &&
     -	test space = "$(git show -s --pretty=format:%s)"
     -
    -+	echo space >expect &&
    -+	git show -s --pretty=tformat:%s >actual &&
    -+	test_cmp expect actual
    ++	test_commit_message HEAD -m space
      '
      
      test_done
-:  ---------- > 2:  9451e4f0f6 t7005: stop abusing --exec-path
2:  0467e33ed0 ! 3:  61cb116780 t7005: sanitize test environment for subsequent tests
    @@ Commit message
         that affect future tests, but those modifications are visible to future
         tests and create a footgun for them.
     
    -    Use test_config, subshells, and test helpers to automatically undo
    -    environment and config modifications once finished.
    +    Use test_config, subshells, single-command environment overrides, and
    +    test helpers to automatically undo environment and config modifications
    +    once finished.
     
         Best-viewed-with: --ignore-all-space
         Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
     
      ## t/t7005-editor.sh ##
     @@
    - 	test_cmp expect actual
    + 	test_commit_message HEAD -m "$msg"
      '
      
     -TERM=dumb
     -export TERM
      test_expect_success 'dumb should error out when falling back on vi' '
    --	if git commit --amend
    --	then
    --		echo "Oops?"
    --		false
    --	else
    --		: happy
    --	fi
    -+	(
    -+		TERM=dumb &&
    -+		export TERM &&
    -+		if git commit --amend
    -+		then
    -+			echo "Oops?"
    -+			false
    -+		else
    -+			: happy
    -+		fi
    -+	)
    +-	test_must_fail git commit --amend
    ++	TERM=dumb test_must_fail git commit --amend
      '
      
      test_expect_success 'dumb should prefer EDITOR to VISUAL' '
    @@ t/t7005-editor.sh
     -	VISUAL=./e-VISUAL.sh &&
     -	export EDITOR VISUAL &&
     -	git commit --amend &&
    --	echo "Edited by EDITOR" >expect &&
    --	git show -s --format=%s >actual &&
    -+	(
    -+		TERM=dumb &&
    -+		export TERM &&
    -+		EDITOR=./e-EDITOR.sh &&
    -+		VISUAL=./e-VISUAL.sh &&
    -+		export EDITOR VISUAL &&
    ++	TERM=dumb EDITOR=./e-EDITOR.sh VISUAL=./e-VISUAL.sh \
     +		git commit --amend &&
    -+		echo "Edited by EDITOR" >expect &&
    -+		git show -s --format=%s >actual
    -+	) &&
    - 	test_cmp expect actual
    + 	test_commit_message HEAD -m "Edited by EDITOR"
      '
      
     -TERM=vt100
     -export TERM
    --for i in $vi EDITOR VISUAL core_editor GIT_EDITOR
    --do
    + for i in $vi EDITOR VISUAL core_editor GIT_EDITOR
    + do
     -	echo "Edited by $i" >expect
     -	unset EDITOR VISUAL GIT_EDITOR
     -	git config --unset-all core.editor
    @@ t/t7005-editor.sh
     -		export $i
     -		;;
     -	esac
    --	test_expect_success "Using $i" '
    --		git --exec-path=. commit --amend &&
    --		git show -s --pretty=oneline >show &&
    --		<show sed -e "s/^[0-9a-f]* //" >actual &&
    --		test_cmp expect actual
    --	'
    --done
    -+test_expect_success 'Using individual editors' '
    -+	test_when_finished "test_unconfig --unset-all core.editor" &&
    -+	(
    -+		TERM=vt100 &&
    -+		export TERM &&
    -+		for i in $vi EDITOR VISUAL core_editor GIT_EDITOR
    -+		do
    -+			sane_unset EDITOR VISUAL GIT_EDITOR &&
    -+			test_might_fail git config --unset-all core.editor &&
    -+			echo "Edited by $i" >expect &&
    + 	test_expect_success "Using $i" '
    +-		PATH="$PWD:$PATH" git commit --amend &&
    +-		test_commit_message HEAD expect
    ++		if test "$i" = core_editor
    ++		then
    ++			test_config core.editor ./e-core_editor.sh
    ++		fi &&
    ++		(
     +			case "$i" in
    -+			core_editor)
    -+				git config core.editor ./e-core_editor.sh
    -+				;;
     +			[A-Z]*)
     +				eval "$i=./e-$i.sh" &&
     +				export $i
     +				;;
     +			esac &&
    -+			git --exec-path=. commit --amend &&
    -+			git show -s --pretty=oneline >show &&
    -+			<show sed -e "s/^[0-9a-f]* //" >actual &&
    -+			test_cmp expect actual
    -+		done
    -+	)
    -+'
    ++			PATH="$PWD:$PATH" TERM=vt100 git commit --amend
    ++		) &&
    ++		test_commit_message HEAD -m "Edited by $i"
    + 	'
    + done
      
     -unset EDITOR VISUAL GIT_EDITOR
     -git config --unset-all core.editor
    @@ t/t7005-editor.sh
     -		;;
     -	esac
     -	test_expect_success "Using $i (override)" '
    --		git --exec-path=. commit --amend &&
    --		git show -s --pretty=oneline >show &&
    --		<show sed -e "s/^[0-9a-f]* //" >actual &&
    --		test_cmp expect actual
    +-		PATH="$PWD:$PATH" git commit --amend &&
    +-		test_commit_message HEAD expect
     -	'
     -done
     +test_expect_success 'Using editors with overrides' '
    @@ t/t7005-editor.sh
     +				export $i
     +				;;
     +			esac &&
    -+			git --exec-path=. commit --amend &&
    -+			git show -s --pretty=oneline >show &&
    -+			<show sed -e "s/^[0-9a-f]* //" >actual &&
    -+			test_cmp expect actual
    ++			PATH="$PWD:$PATH" git commit --amend &&
    ++			test_commit_message HEAD expect || exit 1
     +		done
     +	)
     +'
    @@ t/t7005-editor.sh
      test_expect_success 'editor with a space' '
      	echo "echo space >\"\$1\"" >"e space.sh" &&
     @@
    - 	test_cmp expect actual
    + 	test_commit_message HEAD -m space
      '
      
     -unset GIT_EDITOR
    @@ t/t7005-editor.sh
     -	git config core.editor \"./e\ space.sh\" &&
     +	test_config core.editor \"./e\ space.sh\" &&
      	git commit --amend &&
    - 	echo space >expect &&
    - 	git show -s --pretty=tformat:%s >actual &&
    + 	test_commit_message HEAD -m space
    + '
3:  4a9bb52470 = 4:  ea269f2442 editor: use standard strvec API to receive environment for external editors

base-commit: 112648dd6bdd8e4f485cd0ae11636807959d48be
-- 
2.48.1