Re: Fixes for option parsing

From: Linus Torvalds <torvalds@osdl.org>
Date: Apr 17, 2006, 03:03 UTC
Message-ID: <Pine.LNX.4.64.0604161952500.3701@g5.osdl.org>
In-Reply-To: <Pine.LNX.4.64.0604161938070.3701@g5.osdl.org>

On Sun, 16 Apr 2006, Linus Torvalds wrote:

> 
> Maybe. I'm not convinced, though. The reason? cmd_log_wc needs to generate 
> it regardless, for the "always" case.

Of course, you could just have the "diff" logic unconditionally do the call-back. That would be clean enough.

> Also, I think the "---" printing should be removed, and moved into the 
> "diffstat" flushing code. Right now it does the wrong thing entirely if 
> no diff exists, but we have always_show_header: it will print the "---" 
> for no good reason.

The alternative is to do something like this, but because the "diffstat" doesn't flush the header priperly, it doesn't add the "---" for merges (and it used to not show the log at all, even for "git log", before I fixed it).

Try it with

	git log --cc --patch-with-stat

to see what I mean.

I do agree that this would be much cleaner with a "print header" callback in the diffopt structure. This patch is the really hacky "continue to do things the ugly way" approach to fix some of the uglier output.

Only meant as a RFC and to illustrate what I think the output should look like (modulo the lack of "---" before a diffstat with no patch - for merges). Not meant to actually be applied, I think this can be done much more cleanly with the callback.

		Linus

---

diff --git a/git.c b/git.c
index fc4e429..dc577fa 100644
--- a/git.c
+++ b/git.c
@@ -284,7 +284,7 @@ static int cmd_log_wc(int argc, const ch
 	struct commit *commit;
 	char *buf = xmalloc(LOGSIZE);
 	const char *commit_prefix = "commit ";
-	int shown = 0;
+	int shown = 0, always_show_header;
 
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;
@@ -295,13 +295,19 @@ static int cmd_log_wc(int argc, const ch
 	if (rev->commit_format == CMIT_FMT_ONELINE)
 		commit_prefix = "";
 
+	/*
+	 * We handle always_show_header outselves, and leave the
+	 * "---" handling to log_tree_commit
+	 */
+	always_show_header = rev->always_show_header;
+	rev->always_show_header = 0;
+
 	prepare_revision_walk(rev);
 	setup_pager();
 	while ((commit = get_revision(rev)) != NULL) {
 		unsigned long ofs = 0;
 
-		if (shown && rev->diff &&
-		    rev->commit_format != CMIT_FMT_ONELINE)
+		if (shown && rev->commit_format != CMIT_FMT_ONELINE)
 			putchar('\n');
 
 		ofs = sprintf(buf, "%s", commit_prefix);
@@ -338,14 +344,16 @@ static int cmd_log_wc(int argc, const ch
 					   buf + ofs,
 					   LOGSIZE - ofs - 20,
 					   rev->abbrev);
+		if (always_show_header || !rev->diff) {
+			fputs(buf, stdout);
+			ofs = 0;
+		}
 
 		if (rev->diff) {
+			strcpy(buf + ofs, rev->diffopt.with_stat ? "---" : "\n");
 			rev->use_precomputed_header = buf;
-			strcpy(buf + ofs, "\n---\n");
 			log_tree_commit(rev, commit);
 		}
-		else
-			printf("%s\n", buf);
 		shown = 1;
 		free(commit->buffer);
 		commit->buffer = NULL;