git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder@ira.uka.de>
To: Jeff King <peff@peff.net>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Santi Béjar" <santi@agolina.net>,
	"Git Mailing List" <git@vger.kernel.org>
Subject: Re: git commit -v does not removes the patch
Date: Sat, 22 Nov 2008 16:55:41 +0100	[thread overview]
Message-ID: <20081122155541.GC6885@neumann> (raw)
In-Reply-To: <20081120152015.GA6283@coredump.intra.peff.net>

Hi Jeff,

On Thu, Nov 20, 2008 at 10:20:16AM -0500, Jeff King wrote:
> On Thu, Nov 20, 2008 at 02:09:28PM +0100, SZEDER Gábor wrote:

> Hmm. I am sad that this change has broken somebody's existing workflow.
> OTOH, I'm not convinced that workflow wasn't a little crazy to begin
> with, depending on this undocumented munging.

Well, the docs didn't say that the patch is only removed in verbose
mode (;  But I agree with you that this is (was) an undocumented
"feature" that I should not have relied upon.

> > So, what is the/is there a preferred way to always include the diff in
> > the commit message template and get it removed automatically?  Are
> > there any workarounds other than revert that commit locally?
> 
> How about:
> 
>     $ cat .git/hooks/prepare-commit-msg
>     #!/bin/sh
>     git diff --cached >>"$1"
> 
>     $ cat .git/hooks/commit-msg
>     #!/bin/sh
>     sed -i '/^diff --git/Q' "$1"
> 
> which is more or less the original behavior. The only downside I see is
> that "--no-verify" will turn off the commit-msg hook, but not the
> prepare-commit-msg hook.

Thanks for the tip.  Junio's suggestion about '# Everything under this
line is deleted.' could be implemented this way fairly easily (just an
additional echo in prepare-commit-msg, and other pattern in
commit-msg).  But 'git commit --no-verify' is indeed a problem.
Although I never use it explicitly, 'git rebase -i' does so.

However.

'git rebase -i' uses 'git commit --no-verify' since c5b09feb (Avoid
update hook during git-rebase --interactive, 2007-12-19) to avoid
squashing commits together when the pre-commit hook fails because of
whitespace errors.  But note that at that time the 'git commit'
calls looked like this (after that patch):

  $USE_OUTPUT git commit --no-verify -F "$MSG" $EDIT_COMMIT
  git commit --no-verify -F "$DOTEST"/message -e

Witness that it's not checked whether 'git commit' succeeded or
failed.
However, nowadays those lines look like this:

  $USE_OUTPUT git commit --no-verify $MSG_OPT "$MSG_FILE" $EDIT_COMMIT || failed=t
  git commit --no-verify -F "$DOTEST"/message -e || { ...

There is a check for failed 'git commit', so if the pre-commit hook
finds whitespace errors, then 'git commit' will error out, and,
consequently, 'git rebase -i' will error out, too, and commits won't
be squashed accidentaly.

So, I wonder whether those '--no-verify' options are still required in
'git rebase -i'.

Any thoughts?

Thanks,
Gábor

  reply	other threads:[~2008-11-22 15:57 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-10 15:20 git commit -v does not removes the patch Santi Béjar
2008-11-10 18:10 ` Jeff King
2008-11-10 22:34   ` Santi Béjar
2008-11-10 23:27     ` Junio C Hamano
2008-11-11  0:07       ` Jeff King
2008-11-11  7:56         ` Santi Béjar
2008-11-11 10:29           ` Jeff King
2008-11-11 11:20             ` Santi Béjar
2008-11-11 17:13             ` Junio C Hamano
2008-11-12  8:16               ` Jeff King
2008-11-12  8:17                 ` [PATCH 1/5] define empty tree sha1 as a macro Jeff King
2008-11-12  8:21                 ` [PATCH 2/5] wt-status: refactor initial commit printing Jeff King
2008-11-12  8:23                 ` [PATCH 3/5] status: show "-v" diff even for initial commit Jeff King
2008-11-12  8:24                 ` [PATCH 4/5] commit: loosen pattern for matching "-v" diff Jeff King
2008-11-12  8:25                 ` [PATCH 5/5] commit: only strip diff from message in verbose mode Jeff King
2008-11-12  8:29                   ` Jeff King
2008-11-13  2:15                 ` git commit -v does not removes the patch Junio C Hamano
2008-11-20 13:09                 ` SZEDER Gábor
2008-11-20 15:20                   ` Jeff King
2008-11-22 15:55                     ` SZEDER Gábor [this message]
2008-11-22 20:10                       ` Jeff King
2008-11-22 22:43                       ` Junio C Hamano

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=20081122155541.GC6885@neumann \
    --to=szeder@ira.uka.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=santi@agolina.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).