[PATCH] Allow pickaxe to be used via git log.

[PATCH] Allow pickaxe to be used via git log.

[Sean]

Handle the -S option when passed to git log such that only the appropriate
commits are displayed.  Unlike "whatchanged", by default no diff output
is produced.

---

This came out of recent comments on #git and will hopefully further reduce
the need for git-whatchanged.


ecdfaa21dbff93a6a387b02e1f1d3ebf05ee517d
 revision.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

ecdfaa21dbff93a6a387b02e1f1d3ebf05ee517d
diff --git a/revision.c b/revision.c
index 2294b16..2e18b2b 100644
--- a/revision.c
+++ b/revision.c
@@ -743,6 +743,13 @@ int setup_revisions(int argc, const char
 				continue;
 			}
 			opts = diff_opt_parse(&revs->diffopt, argv+i, argc-i);
+			if (revs->diffopt.pickaxe && revs->always_show_header) {
+				revs->always_show_header = 0;
+				revs->diff = 1;
+				revs->diffopt.recursive = 1;
+				if (revs->diffopt.output_format == DIFF_FORMAT_RAW)
+					revs->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;
+			}
 			if (opts > 0) {
 				revs->diff = 1;
 				i += opts - 1;
-- 
1.3.GIT

Re: [PATCH] Allow pickaxe to be used via git log.

[Junio C Hamano]

Sean <seanlkml@sympatico.ca> writes:

> Handle the -S option when passed to git log such that only the appropriate
> commits are displayed.  Unlike "whatchanged", by default no diff output
> is produced.

If that (specifically, "when passed to git log") is what you
want, do all of that in "git log" please.  I haven't checked,
but I suspect your changes would break git-rev-list and makes it
disobey the options the user gives it.

Re: [PATCH] Allow pickaxe to be used via git log.

[Sean]

On Thu, 18 May 2006 19:47:19 -0700
Junio C Hamano <junkio@cox.net> wrote:

> If that (specifically, "when passed to git log") is what you
> want, do all of that in "git log" please.  I haven't checked,
> but I suspect your changes would break git-rev-list and makes it
> disobey the options the user gives it.
> 

Well, it's more general than just git log, but that's primarily where one
would see it.  Essentially the patch is saying that specifying both a
"revs->diffopt.pickaxe" string and "revs->always_show_header" is illegal
and proceeds to use pickaxe instead of the other.

Is there any place that depends on the ability to specify both a pickaxe
string _and_ always_show_header?  If not, it's hard to see how this patch
could break anything.

Sean

Re: [PATCH] Allow pickaxe to be used via git log.

[Junio C Hamano]

Sean <seanlkml@sympatico.ca> writes:

> Is there any place that depends on the ability to specify both a pickaxe
> string _and_ always_show_header?  If not, it's hard to see how this patch
> could break anything.

User's scripts.  The point is the set of arguments rev-list as
the lowest level machinery should not be modified by an
arbitrary policy decision that happens to suit "git log" usage.

Re: [PATCH] Allow pickaxe to be used via git log.

[Sean]

On Thu, 18 May 2006 20:05:01 -0700
Junio C Hamano <junkio@cox.net> wrote:


> User's scripts.  The point is the set of arguments rev-list as
> the lowest level machinery should not be modified by an
> arbitrary policy decision that happens to suit "git log" usage.
> 

Okay, thanks.

This moves the policy up a level and into the log/show/whatchanged code
and makes the code match the commit message better.  However, it will still
"disobey" user options if the user tries to combine both pickaxe and
--always for these commands; but hopefully that's okay at this level.

Sean

diff --git a/builtin-log.c b/builtin-log.c
index d5bbc1c..d2c3df0 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -23,6 +23,11 @@ static int cmd_log_wc(int argc, const ch
 	rev->commit_format = CMIT_FMT_DEFAULT;
 	rev->verbose_header = 1;
 	argc = setup_revisions(argc, argv, rev, "HEAD");
+	if (rev->diffopt.pickaxe && rev->always_show_header) {
+		rev->always_show_header = 0;
+		if (rev->diffopt.output_format == DIFF_FORMAT_RAW)
+			rev->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;
+	}
 
 	if (argc > 1)
 		die("unrecognized argument: %s", argv[1]);

Re: [PATCH] Allow pickaxe to be used via git log.

[Junio C Hamano]

Sean <seanlkml@sympatico.ca> writes:

> Handle the -S option when passed to git log such that only the appropriate
> commits are displayed.  Unlike "whatchanged", by default no diff output
> is produced.

If your goal is to make whatchanged less necessary, I think you
would need to special case --diff-filter as well for "git log",
although nobody on #git channel seems to have noticed.  I often
run --diff-filter=A when I am trying to see when I added a
particular file, to avoid getting distracted by other types of
changes; log would be still shown if do not disable --always.

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

[Sean]

Handle the -S option when passed to git log such that only the
appropriate commits are displayed.  Also per Junio's comments, do
the same for "--diff-filter", so that it too can be used as an option
to git log.  By default no patch or diff information is displayed.

Signed-off-by: Sean Estabrooks <seanlkml@sympatico.ca>

---

> If your goal is to make whatchanged less necessary, I think you
> would need to special case --diff-filter as well for "git log",
> although nobody on #git channel seems to have noticed.  I often
> run --diff-filter=A when I am trying to see when I added a
> particular file, to avoid getting distracted by other types of
> changes; log would be still shown if do not disable --always.

Makes sense.  This patch should cover that case too.

Sean


a2221c07a94bc378ef40182fa6b260ac88804073
 builtin-log.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

a2221c07a94bc378ef40182fa6b260ac88804073
diff --git a/builtin-log.c b/builtin-log.c
index d5bbc1c..12a6d19 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -23,6 +23,13 @@ static int cmd_log_wc(int argc, const ch
 	rev->commit_format = CMIT_FMT_DEFAULT;
 	rev->verbose_header = 1;
 	argc = setup_revisions(argc, argv, rev, "HEAD");
+	if (rev->always_show_header) {
+		if (rev->diffopt.pickaxe || rev->diffopt.filter) {
+			rev->always_show_header = 0;
+			if (rev->diffopt.output_format == DIFF_FORMAT_RAW)
+				rev->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;
+		}
+	}
 
 	if (argc > 1)
 		die("unrecognized argument: %s", argv[1]);
-- 
1.3.GIT

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

[Junio C Hamano]

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) {

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

[Sean]

On Thu, 18 May 2006 22:41:43 -0700
Junio C Hamano <junkio@cox.net> wrote:

> 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?

Well, I was looking at the use of diff-filter and -S as a way
to prune uninteresting commits from the log rather than as an 
desire to see the patch information.

It's pretty natural to add -p or --stat along with the above 
options if that is what the user wants.  If you make those implied
by using --diff-filter or -S is there a way for the user to say,
no patch and no stat?

Sean

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

[Junio C Hamano]

Sean <seanlkml@sympatico.ca> writes:

> Well, I was looking at the use of diff-filter and -S as a way
> to prune uninteresting commits from the log rather than as an 
> desire to see the patch information.

Fair enough.

> It's pretty natural to add -p or --stat along with the above 
> options if that is what the user wants.  If you make those implied
> by using --diff-filter or -S is there a way for the user to say,
> no patch and no stat?

Well, I was sneaking in a hidden feature with that tweak.

Regardless of the diff-filter/-S issues, with that alternative
patch, you could do:

	$ git log -r

It lets you do a wonderful thing with surprisingly small number
of keystrokes ;-).