Re: [PATCH 4/7] commit: fix patch hunk editing with "commit -p -m"

Jeff King [off-list ref] writes:

On Mon, Mar 10, 2014 at 07:49:34PM +0100, Benoit Pierre wrote:

quoted

Don't change git environment: move the GIT_EDITOR=":" override to the
hook command subprocess, like it's already done for GIT_INDEX_FILE.

Signed-off-by: Benoit Pierre <redacted>
---
 builtin/checkout.c                |  8 +++----
 builtin/clone.c                   |  4 ++--
 builtin/commit.c                  | 35 ++++++++++++++++++++++++-------
 builtin/gc.c                      |  2 +-
 builtin/merge.c                   |  6 +++---
 commit.h                          |  3 +++
 run-command.c                     | 44 ++++++++++++++++++++++++++++-----------
 run-command.h                     |  6 +++++-
 t/t7513-commit_-p_-m_hunk_edit.sh |  2 +-
 9 files changed, 79 insertions(+), 31 deletions(-)

This is a lot of change, and in some ways I think it is making things
better overall. However, the simplest fix for this is basically to move
the setting of GIT_EDITOR down to after we prepare the index.

Jun Hao (cc'd) has been preparing a series for this based on the
Bloomberg git hackday a few weeks ago, but it hasn't been sent to the
list yet.

Commits are here:

  https://github.com/bloomberg/git/compare/commit-patch-allow-hunk-editing

if you care to look. I'm not sure which solution is technically
superior, but it's worth considering both.

I regret not encouraging Jun to post to the list sooner, as we might
have avoided some duplicated effort. But that's a sunk cost, and we
should pick up whichever is the best for the project.

-Peff

I like the idea that the environment setting should be done in one
place. Just not sure run_hook is the right place tho. If user doesn't have
any hook setup, will this kick in? 
One more question, will this work for dry run? Or dry run doesn't matter
in this case?

Thanks - Jun