Re: [PATCH] Allow pickaxe and diff-filter options to be used by git log.

From: Junio C Hamano <junkio@cox.net>
Date: May 19, 2006, 05:41 UTC
Message-ID: <7vbqtuk1uw.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <BAYC1-PASMTP096010F052E9BF78B5FD4AAEA70@CEZ.ICE>

Sean <seanlkml@sympatico.ca> writes:

> +	if (rev->always_show_header) {
> +		if (rev->diffopt.pickaxe || rev->diffopt.filter) {

I understand and agree to the change up to this part, but I do not necessarily agree with what follows:

> +			rev->always_show_header = 0;
> +			if (rev->diffopt.output_format == DIFF_FORMAT_RAW)
> +				rev->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;

To me, if the user explicitly says --diff-filter or -S, it seems more natural to interpret that the user wanted _some_ sort of diff. Now, there are people who say raw format is anti-human, which I consider is a valid view, but I think it is better than NO_OUTPUT in that case.

I wonder if doing something like this instead makes more sense perhaps?

-- >8 --

diff --git a/builtin-log.c b/builtin-log.c
index 69f2911..e68bfad 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -23,6 +23,35 @@ static int cmd_log_wc(int argc, const ch
 	if (argc > 1)
 		die("unrecognized argument: %s", argv[1]);
 
+	if (rev->always_show_header) {
+		/* Log command is primarily about the message for human
+		 * consumption, but if the user asks for any diff, it
+		 * is human unfriendly to give the raw diff.
+		 *
+		 * Show command is the same way, but there the default is
+		 * always give patch output, so this does not trigger.
+		 */
+		if (rev->diffopt.output_format == DIFF_FORMAT_RAW) {
+			if (rev->diffopt.pickaxe)
+				rev->diffopt.output_format = DIFF_FORMAT_PATCH;
+			else {
+				rev->diffopt.output_format = DIFF_FORMAT_DIFFSTAT;
+				rev->diffopt.summary = 1;
+			}
+		}
+
+		/* If the user is limiting the commits to the ones
+		 * that have particular classes of diff, we should not
+		 * show the log message for irrelevant ones.
+		 *
+		 * git show --diff-filter=R -M --all can be used to view
+		 * the branch tips that renames something.  I do not know
+		 * how useful that is, though.
+		 */
+		if (rev->diffopt.pickaxe || rev->diffopt.filter)
+			rev->always_show_header = 0;
+	}
+
 	prepare_revision_walk(rev);
 	setup_pager();
 	while ((commit = get_revision(rev)) != NULL) {