On Mon, Mar 10, 2014 at 9:07 PM, Jeff King <peff@xxxxxxxx> wrote: > On Mon, Mar 10, 2014 at 07:49:34PM +0100, Benoit Pierre wrote: > >> Don't change git environment: move the GIT_EDITOR=":" override to the >> hook command subprocess, like it's already done for GIT_INDEX_FILE. >> [...] > > 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. Replying here instead of to Jun Hao message (I'm not subscribed to the mailing list, so I did not received it): Jun Hao wrote: > 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? According to the original commit, the change to GIT_EDITOR is only here for hooks: commit 406400ce4f69e79b544dd3539a71b85d99331820 Author: Paolo Bonzini <bonzini@xxxxxxx> Date: Tue Feb 5 11:01:45 2008 +0100 git-commit: set GIT_EDITOR=: if editor will not be launched This is a preparatory patch that provides a simple way for the future prepare-commit-msg hook to discover if the editor will be launched. Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> So there is really no reason to set it earlier, and not just in the hook subprocess environment. Regarding dry run: the bug is present, and my patch fix it too (but is missing a test for this). As to which patch is better: it's really not for me to decide! It's a question for the maintainer(s), Jun Hao patch is sure much smaller and simpler, mine is more involved and I believe cleaner in the long term: there is no risk of another part of the commit process to be impacted by the change of environment. Also note that my patch changes the merge builtin too: GIT_EDITOR will not be overriden if an editor will be launched (when used with --edit). -- A: Because it destroys the flow of conversation. Q: Why is top posting dumb? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html