All of lore.kernel.org
 help / color / mirror / Atom feed
From: "D. Ben Knoble" <ben.knoble+github@gmail.com>
To: git@vger.kernel.org
Cc: "D. Ben Knoble" <ben.knoble+github@gmail.com>,
	Patrick Steinhardt <ps@pks.im>,
	Junio C Hamano <gitster@pobox.com>,
	Phillip Wood <phillip.wood123@gmail.com>
Subject: [PATCH 0/3] clean up some code around editors
Date: Sun, 10 Aug 2025 12:03:16 -0400	[thread overview]
Message-ID: <20250810160323.49372-1-ben.knoble+github@gmail.com> (raw)
In-Reply-To: <20250520193506.95199-1-ben.knoble+github@gmail.com>

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/
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@gmail.com>
     
      ## 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


  parent reply	other threads:[~2025-08-10 16:03 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-20 19:34 [PATCH 0/4] Drop git-exec-path from non-Git child programs D. Ben Knoble
2025-05-20 19:34 ` [PATCH 1/4] t7005: sanitize test environment for subsequent tests D. Ben Knoble
2025-05-21  7:56   ` Patrick Steinhardt
2025-05-21 13:23     ` D. Ben Knoble
2025-05-20 19:34 ` [PATCH 2/4] editor: use standard strvec API to receive environment for external editors D. Ben Knoble
2025-05-21  7:56   ` Patrick Steinhardt
2025-05-21 13:26     ` D. Ben Knoble
2025-05-21 13:34       ` Patrick Steinhardt
2025-05-21 15:20       ` Junio C Hamano
2025-05-21 15:10     ` Junio C Hamano
2025-05-20 19:34 ` [PATCH 3/4] run-command: prep_childenv on all platforms D. Ben Knoble
2025-05-21  7:56   ` Patrick Steinhardt
2025-05-21 13:07     ` Phillip Wood
2025-05-21 13:33       ` D. Ben Knoble
2025-05-20 19:34 ` [PATCH 4/4] drop git_exec_path() from non-Git commands' PATH D. Ben Knoble
2025-05-20 20:33   ` Junio C Hamano
2025-05-21 13:44     ` D. Ben Knoble
2025-05-21  8:27   ` Patrick Steinhardt
2025-05-21 13:07     ` Phillip Wood
2025-05-21 13:17       ` Patrick Steinhardt
2025-05-21 15:27         ` Phillip Wood
2025-05-26  6:34           ` Patrick Steinhardt
2025-05-21 15:50         ` Justin Tobler
2025-05-26  6:31           ` Patrick Steinhardt
2025-05-21 13:48     ` D. Ben Knoble
2025-05-21 14:47     ` Junio C Hamano
2025-05-22 13:21 ` [PATCH 0/4] Drop git-exec-path from non-Git child programs Phillip Wood
2025-08-05  1:41   ` D. Ben Knoble
2025-08-05  2:40 ` [PATCH 0/2] clean up some code around editors D. Ben Knoble
2025-08-06 10:07   ` Phillip Wood
2025-08-05  2:40 ` [PATCH 1/2] t7005: sanitize test environment for subsequent tests D. Ben Knoble
2025-08-05  2:40 ` [PATCH 2/2] editor: use standard strvec API to receive environment for external editors D. Ben Knoble
2025-08-10 16:03 ` D. Ben Knoble [this message]
2025-08-10 16:06   ` [PATCH 0/3] clean up some code around editors D. Ben Knoble
2025-08-10 16:34     ` Ben Knoble
2025-08-11 22:16   ` [PATCH v3 0/4] " D. Ben Knoble
2025-08-11 22:59     ` Ben Knoble
2025-08-12  0:16       ` Eric Sunshine
2025-08-12 16:42         ` D. Ben Knoble
2025-08-12 17:35         ` Junio C Hamano
2025-08-12 18:13           ` Eric Sunshine
2025-08-12 18:22             ` Junio C Hamano
2025-08-12 18:30               ` Eric Sunshine
2025-08-12 17:02     ` [PATCH v4 0/3] " D. Ben Knoble
2025-08-13 10:14       ` Phillip Wood
2025-08-13 15:41         ` Junio C Hamano
2025-08-13 17:36           ` D. Ben Knoble
2025-08-13 17:50       ` [PATCH v5 " D. Ben Knoble
2025-08-15 16:00         ` Phillip Wood
2025-08-13 17:50       ` [PATCH v5 1/3] t7005: use modern test style D. Ben Knoble
2025-08-13 17:50       ` [PATCH v5 2/3] t7005: stop abusing --exec-path D. Ben Knoble
2025-08-13 17:50       ` [PATCH v5 3/3] t7005: sanitize test environment for subsequent tests D. Ben Knoble
2025-08-12 17:02     ` [PATCH v4 1/3] t7005: use modern test style D. Ben Knoble
2025-08-12 17:02     ` [PATCH v4 2/3] t7005: stop abusing --exec-path D. Ben Knoble
2025-08-12 17:02     ` [PATCH v4 3/3] t7005: sanitize test environment for subsequent tests D. Ben Knoble
2025-08-11 22:16   ` [PATCH v3 1/4] t7005: use modern test style D. Ben Knoble
2025-08-11 22:16   ` [PATCH v3 2/4] t7005: stop abusing --exec-path D. Ben Knoble
2025-08-11 22:16   ` [PATCH v3 3/4] t7005: sanitize test environment for subsequent tests D. Ben Knoble
2025-08-11 22:34     ` Eric Sunshine
2025-08-12 16:40       ` D. Ben Knoble
2025-08-12 17:13         ` Eric Sunshine
2025-08-11 22:16   ` [PATCH v3 4/4] editor: use standard strvec API to receive environment for external editors D. Ben Knoble
2025-08-11 22:46     ` Eric Sunshine
2025-08-11 23:58       ` Junio C Hamano
2025-08-10 16:03 ` [PATCH 1/3] t7005: use modern test style D. Ben Knoble
2025-08-10 19:42   ` Phillip Wood
2025-08-11 10:04   ` Phillip Wood
2025-08-10 16:03 ` [PATCH 2/3] t7005: sanitize test environment for subsequent tests D. Ben Knoble
2025-08-10 19:44   ` Phillip Wood
2025-08-10 19:53     ` Ben Knoble
2025-08-10 20:58       ` Eric Sunshine
2025-08-11  9:59     ` Phillip Wood
2025-08-10 16:03 ` [PATCH 3/3] editor: use standard strvec API to receive environment for external editors D. Ben Knoble

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250810160323.49372-1-ben.knoble+github@gmail.com \
    --to=ben.knoble+github@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood123@gmail.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.