# Bug: commit -p breaks with -m

4 messages from 2013-08-15 to 2013-08-15. Participants: Matan Nassau, Jeff King, Junio C Hamano.
Thread: https://gitlist.dev/t/34699

## Matan Nassau, 2013-08-15 04:43

Subject: Bug: commit -p breaks with -m
Message-ID: <6EC98F2F-98FC-439B-BD9C-B1D0DB1F5E10@gmail.com>
URL: https://gitlist.dev/e/6EC98F2F-98FC-439B-BD9C-B1D0DB1F5E10%40gmail.com

```
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?
```

## Jeff King, 2013-08-15 12:16

Subject: Re: Bug: commit -p breaks with -m
Message-ID: <20130815121635.GB22102@sigill.intra.peff.net>
URL: https://gitlist.dev/e/20130815121635.GB22102%40sigill.intra.peff.net
In-Reply-To: <6EC98F2F-98FC-439B-BD9C-B1D0DB1F5E10@gmail.com>

```
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

```

## Junio C Hamano, 2013-08-15 17:28

Subject: Re: Bug: commit -p breaks with -m
Message-ID: <7v1u5usw27.fsf@alter.siamese.dyndns.org>
URL: https://gitlist.dev/e/7v1u5usw27.fsf%40alter.siamese.dyndns.org
In-Reply-To: <20130815121635.GB22102@sigill.intra.peff.net>

```
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

```

## Jeff King, 2013-08-15 17:31

Subject: Re: Bug: commit -p breaks with -m
Message-ID: <20130815173158.GA27866@sigill.intra.peff.net>
URL: https://gitlist.dev/e/20130815173158.GA27866%40sigill.intra.peff.net
In-Reply-To: <7v1u5usw27.fsf@alter.siamese.dyndns.org>

```
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

```