git-fmt-merge-message problem..

git-fmt-merge-message problem..

[Linus Torvalds]

Not a huge one, but it's ugly.

When I did a "git pull repo", the merge messages _used_ to look like this:

    Merge master.kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6
    
    * master.kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6:
      [NEIGH]: Fix IP-over-ATM and ARP interaction.
      [TG3]: ethtool always report port is TP.

or (if I specified a branch):

    Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ro
    
    * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/roland/infini
      IB/mthca: FMR ioremap fix
      IPoIB: Free child interfaces properly
      IB/mthca: Fix race in reference counting
      IB/srp: Fix tracking of pending requests during error handling
      IB: Fix display of 4-bit port counters in sysfs

but now they look like this:

    Merge commit master.kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6 of HEAD
    
    * HEAD:
      [NET]: fix __sk_stream_mem_reclaim
      [Bluetooth] Fix deadlock in the L2CAP layer
      [Bluetooth] Let BT_HIDP depend on INPUT
      [Bluetooth] Avoid NULL pointer dereference with tty->driver
      [Bluetooth] Remaining transitions to use kzalloc()
      [WAN]: converting generic HDLC to use netif_dormant*()
      [IPV4]: Fix error handling for fib_insert_node call
      [NETROM] lockdep: fix false positive
      ....

which makes no sense AT ALL. It's doesn't even parse. "Merge commit <repo> 
of <branch>"? Whaa? That's just insane.

Also, the " * HEAD" is just ugly. It was better before.

Dscho? Can you please make git fmt-merge-message print out something 
sensible again? It doesn't have to match the old behaviour 100%, but the 
current one is just ugly and senseless.

			Linus

Re: git-fmt-merge-message problem..

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> which makes no sense AT ALL. It's doesn't even parse. "Merge commit <repo> 
> of <branch>"? Whaa? That's just insane.
>
> Also, the " * HEAD" is just ugly. It was better before.

Yes, this is a simple mistranslation from Perl to C.  A patch
will follow shortly.

Re: git-fmt-merge-message problem..

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> Linus Torvalds <torvalds@osdl.org> writes:
>
>> which makes no sense AT ALL. It's doesn't even parse. "Merge commit <repo> 
>> of <branch>"? Whaa? That's just insane.
>>
>> Also, the " * HEAD" is just ugly. It was better before.
>
> Yes, this is a simple mistranslation from Perl to C.  A patch
> will follow shortly.

-- >8 --
[PATCH] fmt-merge-msg fix

The new C version mistranslated the original Perl version in the
case to pull from the HEAD.  This made it to say nonsense like
this:

	Merge commit ...url... of HEAD

        * HEAD:
        ...

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-fmt-merge-msg.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index 6527482..fe0ef44 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -76,6 +76,7 @@ static int handle_line(char *line)
 	unsigned char *sha1;
 	char *src, *origin;
 	struct src_data *src_data;
+	int pulling_head = 0;
 
 	if (len < 43 || line[40] != '\t')
 		return 1;
@@ -101,8 +102,11 @@ static int handle_line(char *line)
 	if (src) {
 		*src = 0;
 		src += 4;
-	} else
-		src = "HEAD";
+		pulling_head = 0;
+	} else {
+		src = line;
+		pulling_head = 1;
+	}
 
 	i = find_in_list(&srcs, src);
 	if (i < 0) {
@@ -112,7 +116,10 @@ static int handle_line(char *line)
 	}
 	src_data = srcs.payload[i];
 
-	if (!strncmp(line, "branch ", 7)) {
+	if (pulling_head) {
+		origin = strdup(src);
+		src_data->head_status |= 1;
+	} else if (!strncmp(line, "branch ", 7)) {
 		origin = strdup(line + 7);
 		append_to_list(&src_data->branch, origin, NULL);
 		src_data->head_status |= 2;
@@ -124,9 +131,6 @@ static int handle_line(char *line)
 		origin = strdup(line + 14);
 		append_to_list(&src_data->r_branch, origin, NULL);
 		src_data->head_status |= 2;
-	} else if (!strcmp(line, "HEAD")) {
-		origin = strdup(src);
-		src_data->head_status |= 1;
 	} else {
 		origin = strdup(src);
 		append_to_list(&src_data->generic, strdup(line), NULL);

Re: git-fmt-merge-message problem..

[Linus Torvalds]

On Wed, 12 Jul 2006, Junio C Hamano wrote:
> 
> Yes, this is a simple mistranslation from Perl to C.  A patch
> will follow shortly.

On a similar vein, how about this?

I always end up editing the revert sentence by hand to make grammatical 
sense.

I think we always talk about "commit xyz".

We never talk about "xyz commit", except when we end up talking about a 
commit as a branch head (notably, I would say "the HEAD commit", or 
possibly "the top-of-master commit", but here $commit is a SHA1 name, not 
anything else).

Hmm?

		Linus

---

diff --git a/git-revert.sh b/git-revert.sh
index de8b5f0..2bf35d1 100755
--- a/git-revert.sh
+++ b/git-revert.sh
@@ -84,7 +84,7 @@ revert)
 		s/^[^ ]* /Revert "/
 		s/$/"/'
 	echo
-	echo "This reverts $commit commit."
+	echo "This reverts commit $commit."
 	test "$rev" = "$commit" ||
 	echo "(original 'git revert' arguments: $@)"
 	base=$commit next=$prev

Re: git-fmt-merge-message problem..

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> On Wed, 12 Jul 2006, Junio C Hamano wrote:
>> 
>> Yes, this is a simple mistranslation from Perl to C.  A patch
>> will follow shortly.
>
> On a similar vein, how about this?
>
> I always end up editing the revert sentence by hand to make grammatical 
> sense.
>
> I think we always talk about "commit xyz".
>
> We never talk about "xyz commit", except when we end up talking about a 
> commit as a branch head (notably, I would say "the HEAD commit", or 
> possibly "the top-of-master commit", but here $commit is a SHA1 name, not 
> anything else).
>
> Hmm?

Makes sense.  Sign-off ;-)?

I wonder if we would want to abbreviate this, though.

Re: git-fmt-merge-message problem..

[Johannes Schindelin]

Hi,

On Wed, 12 Jul 2006, Linus Torvalds wrote:

> Dscho? Can you please make git fmt-merge-message print out something 
> sensible again?

Sorry for the breakage. I saw Junio already patched and pushed.

Ciao,
Dscho