[PATCH] git-rev-list: fix --header

[PATCH] git-rev-list: fix --header

[Johannes Schindelin]

gitk expects raw verbose headers limited by \0. Meet these expectations.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	This fixes the "clock format" bug I mentioned in another thread: 
	git-rev-list printed non-raw headers (lacking "author", 
	"committer", and the other lines), and thus, the author date was
	empty.

 rev-list.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

18f897509cd95b7e55c63e75fa40655c532507f6
diff --git a/rev-list.c b/rev-list.c
index 9d8db25..54e92f7 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -293,6 +293,7 @@ int main(int argc, const char **argv)
 {
 	struct commit_list *list;
 	int i;
+	int header = 0;
 
 	init_revisions(&revs);
 	revs.abbrev = 0;
@@ -303,7 +304,8 @@ int main(int argc, const char **argv)
 		const char *arg = argv[i];
 
 		if (!strcmp(arg, "--header")) {
-			revs.verbose_header = 1;
+			header = 1;
+			revs.commit_format = CMIT_FMT_UNSPECIFIED;
 			continue;
 		}
 		if (!strcmp(arg, "--timestamp")) {
@@ -324,6 +326,10 @@ int main(int argc, const char **argv)
 			revs.header_prefix = "";
 		else
 			revs.header_prefix = "commit ";
+	} else if (header) {
+		revs.verbose_header = 1;
+		revs.commit_format = CMIT_FMT_RAW;
+		hdr_termination = 0;
 	}
 
 	list = revs.commits;
-- 
1.2.0.ga8a6

Re: [PATCH] git-rev-list: fix --header

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> gitk expects raw verbose headers limited by \0. Meet these expectations.
>
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

Thanks.

Wouldn't this be simpler and do the same thing, I wonder?  The
point being that "--pretty --header" and "--header --pretty"
traditionally did not make --header to override --pretty.

diff --git a/rev-list.c b/rev-list.c
index 000f27a..d3c0dd9 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -326,6 +326,9 @@ int main(int argc, const char **argv)
 		else
 			revs.header_prefix = "commit ";
 	}
+	else if (revs.verbose_header)
+		/* Only --header was specified */
+		revs.commit_format = CMIT_FMT_RAW;
 
 	list = revs.commits;
 

Re: [PATCH] git-rev-list: fix --header

[Johannes Schindelin]

Hi,

On Mon, 17 Apr 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > gitk expects raw verbose headers limited by \0. Meet these expectations.
> >
> > Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> 
> Thanks.
> 
> Wouldn't this be simpler and do the same thing, I wonder?  The
> point being that "--pretty --header" and "--header --pretty"
> traditionally did not make --header to override --pretty.

I thought, why not fix that bug, too? After all, it is counterintuitive 
what "--header --pretty" does, and it was easy to fix.

Ciao,
Dscho

Re: [PATCH] git-rev-list: fix --header

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Wouldn't this be simpler and do the same thing, I wonder?  The
>> point being that "--pretty --header" and "--header --pretty"
>> traditionally did not make --header to override --pretty.
>
> I thought, why not fix that bug, too? After all, it is counterintuitive 
> what "--header --pretty" does, and it was easy to fix.

I checked with 0.99.9m and both "--pretty --header" and
"--header --pretty" gives preference to --pretty.  I think your
patch changes it to favor whichever comes later.

I thought it could be considered a bug to accept --header and
--pretty at the same time without complaining, but if you want
to forbid it, you could error out.  However, that might break
existing Porcelains, and that's why I suggested to keep the
traditional "--pretty wins over --header" behaviour.

If gitk were the only Porcelain we care about that uses
--header, it would make more sense to change the rule to
"--header trumps --pretty" and "gitk --pretty" would magically
start working.

Still undecided.  As you say it is an easy change, so I'd rather
leave the behaviour as before for now.

Re: [PATCH] git-rev-list: fix --header

[Johannes Schindelin]

Hi,

On Mon, 17 Apr 2006, Junio C Hamano wrote:

> Still undecided.  As you say it is an easy change, so I'd rather
> leave the behaviour as before for now.

Okay, I'd say go with the shorter (your) solution.

Ciao,
Dscho