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@xxxxxxxxx/ Published-as: https://github.com/benknoble/tree/editor-cleanup D. Ben Knoble (3): t7005: use modern test style 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 | 152 ++++++++++++++++++++++++---------------------- 4 files changed, 89 insertions(+), 82 deletions(-) Diff-intervalle : -: ---------- > 1: e7629202a1 t7005: use modern test style 1: a37db65107 ! 2: 0467e33ed0 t7005: sanitize test environment for subsequent tests @@ Commit message t7005: sanitize test environment for subsequent tests Some of the editor tests manipulate the environment or config in ways - that affect future tests (because they test a sequence of overrides), - but those modifications are visible to future tests and create a footgun - for them. + that affect future tests, but those modifications are visible to future + tests and create a footgun for them. - We can't make the environment-munging override tests undo their - modifications because they rely on editor variables overriding other - previously-set editor variables. - - Use test_config and undo environment modifications once finished. + Use test_config, subshells, 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@xxxxxxxxx> ## t/t7005-editor.sh ## @@ - ' - done + test_cmp expect actual + ' + +-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_expect_success 'dumb should prefer EDITOR to VISUAL' ' +- EDITOR=./e-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 && ++ git commit --amend && ++ echo "Edited by EDITOR" >expect && ++ git show -s --format=%s >actual ++ ) && + test_cmp expect actual + ' + +-TERM=vt100 +-export TERM +-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 +- case "$i" in +- core_editor) +- git config core.editor ./e-core_editor.sh +- ;; +- [A-Z]*) +- eval "$i=./e-$i.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 && ++ 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 ++ ) ++' + +-unset EDITOR VISUAL GIT_EDITOR +-git config --unset-all core.editor +-for i in $vi EDITOR VISUAL core_editor GIT_EDITOR +-do +- echo "Edited by $i" >expect +- case "$i" in +- core_editor) +- git config core.editor ./e-core_editor.sh +- ;; +- [A-Z]*) +- eval "$i=./e-$i.sh" +- export $i +- ;; +- 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 +- ' +-done ++test_expect_success 'Using editors with overrides' ' ++ ( ++ TERM=vt100 && ++ export TERM && ++ for i in $vi EDITOR VISUAL core_editor GIT_EDITOR ++ do ++ echo "Edited by $i" >expect && ++ 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 ++ ) ++' -+unset EDITOR VISUAL GIT_EDITOR -+git config --unset-all core.editor test_expect_success 'editor with a space' ' echo "echo space >\"\$1\"" >"e space.sh" && - chmod a+x "e space.sh" && @@ - + test_cmp expect actual ' -unset GIT_EDITOR test_expect_success 'core.editor with a space' ' -- - git config core.editor \"./e\ space.sh\" && + test_config core.editor \"./e\ space.sh\" && git commit --amend && - test space = "$(git show -s --pretty=format:%s)" -- - ' - - test_done + echo space >expect && + git show -s --pretty=tformat:%s >actual && 2: 5450c99f59 = 3: 4a9bb52470 editor: use standard strvec API to receive environment for external editors base-commit: 112648dd6bdd8e4f485cd0ae11636807959d48be -- 2.48.1
