Re: [PATCH 2/2] Fix ranges with git-show

Junio C Hamano [off-list ref] writes:

I do not think it is sad at all.  If we were told to walk, we should
walk just like "git log" does, and otherwise we just show them one
by one.  It might be even cleaner if we separate the command line
parsing and showing into separate phases, instead of the current
loop structure of parsing one object and deciding how to show.

How about doing it this way without applying your 2/2?

[...]

 	opt.tweak = show_rev_tweak_rev;
 	cmd_log_init(argc, argv, prefix, &rev, &opt);
 
+	if (!rev.no_walk)
+		return cmd_log_walk(&rev);
+
 	count = rev.pending.nr;
 	objects = rev.pending.objects;
 	for (i = 0; i < count && !ret; i++) {

Clever.  But it eliminates all possibility of *simultaneously* showing a
range and some other objects, of which

Among your new tests:

	git show ^side3 annotated

must change, as it is not asking to show individual objects (in
other words, I think the test expects a wrong thing), but everything
else should work as expected (I didn't check).

is just a symptom of.  Do you want to change it?  I'd be all for it, but
it changes the meaning somewhat.  So far, showing "anything plus ranges"
is broken only as far as the ranges are concerned.

If you do make this change, can we merge the log and show code?
Granted, show defaults to -p --cc --no-walk, and log does not, but can
we then unify the main logic?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch