Hi,
On Mon, 23 Jul 2007, Steven Grimm wrote:
> Johannes Schindelin wrote:
> > Ah, I missed that. But IIRC your patch does not wrap that logic behind test
> > ! -z "$templatefile", right? So this is my modified suggestion:
> >
> > test ! -z "$templatefile" && {
> > grep -vie '^Signed-off-by:' < "$GIT_DIR"/COMMIT_MSG" > "$GIT_DIR"/tmp1
> > grep -ve '^#' < "$templatefile" > "$GIT_DIR"/tmp1
> > trap 'rm "$GIT_DIR"/tmp[12]' 0
> > cmp "$GIT_DIR"/tmp[12] &&
> > die "Unchanged message; will not commit"
> > }
> >
>
> So you are suggesting I do this in addition to the existing git-commit
> stripping of Signed-off-by: lines? I can certainly do that, but I didn't want
> to make two passes over the commit message doing exactly the same stripping.
>
> Hmm, maybe I should outline my understanding of the current (unpatched)
> behavior and what I want it to do. Currently:
>
> * Strip off all comment lines (happens when COMMIT_MSG is created)
> * Strip off all Signed-off-by: lines
> * Trim whitespace
> * If the result has no content (`wc -l` == 0), abort.
>
> With the patch, my intent was:
>
> * Strip off all comment lines
> * Strip off all Signed-off-by: lines
> * Trim whitespace
> * If the result has no content (! -s file), abort.
> * If a template file was specified:
> * Strip off all comment and Signed-off-by: lines from the template
> * Trim whitespace from the template
> * If the resulting trimmed template is the same as the trimmed commit
> message, abort.
>
> So I guess before getting to the specifics of the code, I'll ask: does the
> above make sense as a design? I wanted to preserve the existing behavior in
> the absence of a template.
Yes, I think that makes sense.
> Since the existing code is already stripping Signed-off-by: lines to test for
> a zero-length commit message, I figured I should reuse that work. However,
> it's no big deal to do it twice if people feel that results in more readable
> code -- certainly no human will ever notice the time it takes to re-grep the
> commit message.
Okay, but I was really confused by the big if thing.
I'd make sure that COMMIT_BAREMSG is removed with a trap, and really try
to just enhance the
if cnt=`grep -v -i '^Signed-off-by' "$GIT_DIR"/COMMIT_MSG |
git stripspace |
wc -l` &&
test 0 -lt $cnt
so that it reads something like
if test -s "$GIT_DIR"/COMMIT_BAREMSG &&
test -z "$templatefile" ||
(grep -ve '^#' < "$templatefile" |
git stripspace |
git diff --quiet - "$GIT_DIR"/COMMIT_BAREMSG)
then
...
(Totally untested, of course.) Hmm?
Ciao,
Dscho
-
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
