threads / patch / 3899

patchgit-rev-list: fix --header

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

## tl;dr

5 messages between Apr 17, 2006 and Apr 18, 2006. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Johannes Schindelin· Apr 17, 2006, 12:51 UTC · lore
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
Show changes to rev-list.c +7 −1
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
Junio C Hamano· Apr 17, 2006, 19:36 UTC · re: Johannes Schindelin · lore

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

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.

Show changes to rev-list.c +3 −0
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;
Johannes Schindelin· Apr 17, 2006, 20:24 UTC · re: Junio C Hamano · lore

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

Hi,
On Mon, 17 Apr 2006, Junio C Hamano wrote:
Show 11 quoted lines
> 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

Junio C Hamano· Apr 17, 2006, 21:42 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 6 quoted lines
>> 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.

Johannes Schindelin· Apr 18, 2006, 14:50 UTC · re: Junio C Hamano · lore

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

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

← back to recent threads