Re: [PATCH v5 0/3] clean up some code around editors

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Ben

On 13/08/2025 18:50, D. Ben Knoble wrote:
> Changes from v4:
> - revert use of test_env: fix the one instance that needs adjusted for
>    VAR=val cmd syntax by using test_must_fail env VAR=val cmd.

Thanks for re-rolling, the range-diff below looks good

Phillip

> 
> Changes from v3:
> - drop 4/4
> - use test_env (including a case our lint does not catch when the value
>    has spaces)
> 
> 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
> 
> These patches clean up some old code in the editor tests that does not use our
> modern idioms.
> 
> 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.
> 
> 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/
> v3: https://lore.kernel.org/git/20250811221706.67168-1-ben.knoble+github@gmail.com/
> v4: https://lore.kernel.org/git/20250812170256.71751-1-ben.knoble+github@gmail.com/
> Published-as: https://github.com/benknoble/tree/editor-cleanup
> 
> D. Ben Knoble (3):
>    t7005: use modern test style
>    t7005: stop abusing --exec-path
>    t7005: sanitize test environment for subsequent tests
> 
>   t/t7005-editor.sh | 147 +++++++++++++++++++---------------------------
>   1 file changed, 59 insertions(+), 88 deletions(-)
> 
> Diff-intervalle contre v4 :
> 1:  8ad2904a18 = 1:  8ad2904a18 t7005: use modern test style
> 2:  44a6fd8eb3 ! 2:  5a35889571 t7005: stop abusing --exec-path
>      @@ t/t7005-editor.sh
>        	esac
>        	test_expect_success "Using $i" '
>       -		git --exec-path=. commit --amend &&
>      -+		test_env PATH="$PWD:$PATH" git commit --amend &&
>      ++		PATH="$PWD:$PATH" git commit --amend &&
>        		test_commit_message HEAD expect
>        	'
>        done
>      @@ t/t7005-editor.sh
>        	esac
>        	test_expect_success "Using $i (override)" '
>       -		git --exec-path=. commit --amend &&
>      -+		test_env PATH="$PWD:$PATH" git commit --amend &&
>      ++		PATH="$PWD:$PATH" git commit --amend &&
>        		test_commit_message HEAD expect
>        	'
>        done
> 3:  135d4368d6 ! 3:  e6e31ab98c t7005: sanitize test environment for subsequent tests
>      @@ t/t7005-editor.sh
>       -export TERM
>        test_expect_success 'dumb should error out when falling back on vi' '
>       -	test_must_fail git commit --amend
>      -+	test_env TERM=dumb test_must_fail git commit --amend
>      ++	test_must_fail env TERM=dumb 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 &&
>      -+	test_env TERM=dumb EDITOR=./e-EDITOR.sh VISUAL=./e-VISUAL.sh \
>      ++	TERM=dumb EDITOR=./e-EDITOR.sh VISUAL=./e-VISUAL.sh \
>       +		git commit --amend &&
>        	test_commit_message HEAD -m "Edited by EDITOR"
>        '
>      @@ t/t7005-editor.sh
>       -		;;
>       -	esac
>        	test_expect_success "Using $i" '
>      --		test_env PATH="$PWD:$PATH" git commit --amend &&
>      +-		PATH="$PWD:$PATH" git commit --amend &&
>       -		test_commit_message HEAD expect
>       +		if test "$i" = core_editor
>       +		then
>      @@ t/t7005-editor.sh
>       +				export $i
>       +				;;
>       +			esac &&
>      -+			test_env PATH="$PWD:$PATH" TERM=vt100 git commit --amend
>      ++			PATH="$PWD:$PATH" TERM=vt100 git commit --amend
>       +		) &&
>       +		test_commit_message HEAD -m "Edited by $i"
>        	'
>      @@ t/t7005-editor.sh
>       -		;;
>       -	esac
>       -	test_expect_success "Using $i (override)" '
>      --		test_env PATH="$PWD:$PATH" git commit --amend &&
>      +-		PATH="$PWD:$PATH" git commit --amend &&
>       -		test_commit_message HEAD expect
>       -	'
>       -done
>      @@ t/t7005-editor.sh
>       +				export $i
>       +				;;
>       +			esac &&
>      -+			test_env PATH="$PWD:$PATH" git commit --amend &&
>      ++			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" &&
>      - 	chmod a+x "e space.sh" &&
>      --	GIT_EDITOR="./e\ space.sh" git commit --amend &&
>      -+	test_env GIT_EDITOR="./e\ space.sh" git commit --amend &&
>      +@@
>        	test_commit_message HEAD -m space
>        '
>        
> 
> base-commit: 112648dd6bdd8e4f485cd0ae11636807959d48be