[PATCH] git-commit: filter out log message lines only when editor was run.

[PATCH] git-commit: filter out log message lines only when editor was run.

[Yann Dirson]

The current behaviour strips out lines starting with a # even when fed
through stdin or -m.  This is particularly bad when importing history from
another SCM (tailor 0.9.23 uses git-commit).  In the best cases all lines
are stripped and the commit fails with a confusing "empty log message"
error, but in many cases the commit is done, with loss of information.

Note that it is quite peculiar to just have "#" handled as a leading
comment char here.  One commonly meet CVS: or CG: or STG: as prefixes, and
using GIT: would be more robust as well as consistent with other commit
tools.  However, that would break any tool relying on the # (if any).

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 git-commit.sh |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/git-commit.sh b/git-commit.sh
index 6dd04fd..aa3b1ea 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -691,13 +691,18 @@ t)
 	fi
 esac
 
-sed -e '
-    /^diff --git a\/.*/{
-	s///
-	q
-    }
-    /^#/d
-' "$GIT_DIR"/COMMIT_EDITMSG |
+if test -z "$no_edit"
+then
+    sed -e '
+        /^diff --git a\/.*/{
+	    s///
+	    q
+	}
+	/^#/d
+    ' "$GIT_DIR"/COMMIT_EDITMSG
+else
+    cat "$GIT_DIR"/COMMIT_EDITMSG
+fi |
 git-stripspace >"$GIT_DIR"/COMMIT_MSG
 
 if cnt=`grep -v -i '^Signed-off-by' "$GIT_DIR"/COMMIT_MSG |

Re: [PATCH] git-commit: filter out log message lines only when editor was run.

[Junio C Hamano]

Yann Dirson <ydirson@altern.org> writes:

> The current behaviour strips out lines starting with a # even when fed
> through stdin or -m.  This is particularly bad when importing history from
> another SCM (tailor 0.9.23 uses git-commit).  In the best cases all lines
> are stripped and the commit fails with a confusing "empty log message"
> error, but in many cases the commit is done, with loss of information.

I agree with this in principle but we would need to make sure
that our own scripts do not expect that the message is cleaned
up when feeding a commit log message via stdin, -m or -F, and if
they do fix them before applying this patch.
 

Re: [PATCH] git-commit: filter out log message lines only when editor was run.

[Yann Dirson]

Junio wrote:
> I agree with this in principle but we would need to make sure
> that our own scripts do not expect that the message is cleaned
> up when feeding a commit log message via stdin, -m or -F, and if
> they do fix them before applying this patch.

The only tools in git.git I could identify as using git-commit rather
than commit-tree are:

git-revert.sh: OK
git-rebase.sh: only uses -C

cogito, stgit, and pg also use commit-tree.  Only qgit seems to be
using git-commit, and probably makes use of this (mis)feature.

I guess the easiest way to get the 1.4.0 behaviour is to change
git-commit calls to "git-commit -e" with EDITOR=true in the
environment.  I'm not sure it would be worth adding a new flag to
switch on/off log stripping.

Best regards,
-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>

Re: [PATCH] git-commit: filter out log message lines only when editor was run.

[Marco Costalba]

On 6/24/06, Yann Dirson <ydirson@altern.org> wrote:
> Junio wrote:
> > I agree with this in principle but we would need to make sure
> > that our own scripts do not expect that the message is cleaned
> > up when feeding a commit log message via stdin, -m or -F, and if
> > they do fix them before applying this patch.
>
> The only tools in git.git I could identify as using git-commit rather
> than commit-tree are:
>
> git-revert.sh: OK
> git-rebase.sh: only uses -C
>
> cogito, stgit, and pg also use commit-tree.  Only qgit seems to be
> using git-commit, and probably makes use of this (mis)feature.
>

Yes, qgit uses git-commit, but issue is different.

qgit uses the output of git status as a 'default text' used in the
commit dialog. When commit button is pressed, after some message
editing by the user,  the comment lines are stripped _before_ to save
the content of commit message in a temporary file to be used by
git-commit with -F option.

The comment stripping is done by this code:

	msg = textEditMsg->text();
	msg.remove(QRegExp("\\n\\s*#[^\\n]*")); // strip comments
	msg.replace(QRegExp("[ \\t\\r\\f\\v]+\\n"), "\n"); // strip line trailing cruft
	msg = msg.stripWhiteSpace();

as you see it rely on the  '#' character.

So conclusions are:

1) It's ok to let git-comit do not strip comment lines, it's already
done by qgit.

2) Please don't change git status output to use another symbol to mark
comment lines because it will break line stripping code.

        Marco

Re: [PATCH] git-commit: filter out log message lines only when editor was run.

[Yann Dirson]

On Sun, Jun 25, 2006 at 09:08:02AM +0200, Marco Costalba wrote:
> 1) It's ok to let git-comit do not strip comment lines, it's already
> done by qgit.

That's great news :)

> 2) Please don't change git status output to use another symbol to mark
> comment lines because it will break line stripping code.

If it ever gets changed, it should anyway IMHO be made a parameter.  A
strategy could be to make it customizable first, advertize this loudly
so tools adapt themselves to force the prefix to what suits them best,
and then eventually change the default in a major release.

Best regards,
-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>