* Bug: commit -p breaks with -m
@ 2013-08-15 4:43 Matan Nassau
2013-08-15 12:16 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: Matan Nassau @ 2013-08-15 4:43 UTC (permalink / raw)
To: git
With git 1.8.3.3,
$ seq 5 >data
$ git add data
$ git commit -mdata
$ sed -i '2 d' data
$ git commit -pmchange
At the prompt, type e to edit the hunk. The editor doesn't start, but git records a commit.
I found that builtin/commit.c sets the GIT_EDITOR env var to ":" when the user specifies the -m option. This was done in 406400ce4f69. Removing these two lines,
if (!use_editor)
setenv("GIT_EDITOR", ":", 1);
seems to fix the issue, but I'm not sure this won't break the prepare-commit-msg hook. I'd like to submit a patch: can I get a hint if this change would break commit hooks or anything else I'm not seeing?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Bug: commit -p breaks with -m
2013-08-15 4:43 Bug: commit -p breaks with -m Matan Nassau
@ 2013-08-15 12:16 ` Jeff King
2013-08-15 17:28 ` Junio C Hamano
0 siblings, 1 reply; 4+ messages in thread
From: Jeff King @ 2013-08-15 12:16 UTC (permalink / raw)
To: Matan Nassau; +Cc: git
On Thu, Aug 15, 2013 at 12:43:39AM -0400, Matan Nassau wrote:
> With git 1.8.3.3,
>
> $ seq 5 >data
> $ git add data
> $ git commit -mdata
> $ sed -i '2 d' data
> $ git commit -pmchange
>
> At the prompt, type e to edit the hunk. The editor doesn't start, but
> git records a commit.
>
> I found that builtin/commit.c sets the GIT_EDITOR env var to ":" when
> the user specifies the -m option. This was done in 406400ce4f69.
> Removing these two lines,
>
> if (!use_editor)
> setenv("GIT_EDITOR", ":", 1);
>
> seems to fix the issue, but I'm not sure this won't break the
> prepare-commit-msg hook. I'd like to submit a patch: can I get a hint
> if this change would break commit hooks or anything else I'm not
> seeing?
Yeah, that is definitely a bug. Just removing those lines would not be the
right fix, though, because the point of them is to let the
prepare-commit-msg hook know whether or not an editor is in use.
Instead, I think you would want to limit the scope of where we have set
GIT_EDITOR. I.e., drop those lines, and then add GIT_EDITOR=: to the
environment that we pass to the hook via the run_hook function.
Unfortunately, I think that will require some refactoring of the
run_hook interface, which does not allow arbitrary environment
parameters to be set.
-Peff
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Bug: commit -p breaks with -m
2013-08-15 12:16 ` Jeff King
@ 2013-08-15 17:28 ` Junio C Hamano
2013-08-15 17:31 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2013-08-15 17:28 UTC (permalink / raw)
To: Jeff King; +Cc: Matan Nassau, git
Jeff King <peff@peff.net> writes:
> Unfortunately, I think that will require some refactoring of the
> run_hook interface, which does not allow arbitrary environment
> parameters to be set.
Heh, I said that long time ago, e.g. $gmane/212284 ;-)
This might turn out to be a good starting point, even though I
didn't read it again before sending this message.
http://thread.gmane.org/gmane.comp.version-control.git/192669/focus=192806
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Bug: commit -p breaks with -m
2013-08-15 17:28 ` Junio C Hamano
@ 2013-08-15 17:31 ` Jeff King
0 siblings, 0 replies; 4+ messages in thread
From: Jeff King @ 2013-08-15 17:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Matan Nassau, git
On Thu, Aug 15, 2013 at 10:28:16AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Unfortunately, I think that will require some refactoring of the
> > run_hook interface, which does not allow arbitrary environment
> > parameters to be set.
>
> Heh, I said that long time ago, e.g. $gmane/212284 ;-)
>
> This might turn out to be a good starting point, even though I
> didn't read it again before sending this message.
>
> http://thread.gmane.org/gmane.comp.version-control.git/192669/focus=192806
Looking at both of those messages, I agree completely. I even started
to sketch out the refactoring when I sent the previous message, and it
looked a lot like the patch you sent.
-Peff
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-08-15 17:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-15 4:43 Bug: commit -p breaks with -m Matan Nassau
2013-08-15 12:16 ` Jeff King
2013-08-15 17:28 ` Junio C Hamano
2013-08-15 17:31 ` Jeff King
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.