Re: [PATCH v5 0/3] clean up some code around editors
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
Show 128 quoted lines
>
> 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