Michael J Gruber wrote:
> We don't do the systematic approach now. In some situations, some
> commands switch on the walker automatically (I think "show A..B") to
> make things more useful (to most users) but less systematic, even less
> predictable if you don't know these deviations/exceptions. I've
> suggested such a "usefulness exception" myself (don't prune commits by
> path for "show").
Ah. To be clearer about the present state:
- cmd_show reimplements much of get_revision, to work around the
revision walking machinery's lack of callbacks to print tags,
blobs, and so on.
- ^A means "--do-walk ^A", and A..B means "--do-walk ^A B". This
holds in rev-list, log, show, etc --- they all share the code that
does this.
- Similarly, -5 means "--do-walk -5".
- rev-parse shares a revision parser (get_sha1) with rev-list, but it
doesn't share an option parser (alas).
I personally kind of like the "don't prune commits by path with
--no-walk" idea. Not sure what happened to that.
> The strict, systematic approach produces some command/argument
> combinations which are not useful or rarely useful.
Well, --no-walk foo..bar would be nonsense regardless of whether "git
show" did the equivalent of "git rev-list --no-walk ...". And when
the user supplies nonsense, why should she expect us not to give
nonsense back? So how about something along these lines?
Needs documentation and tests.
-- >8 --
Subject: revisions: treat "log A..B --no-walk" as a synonym for "--no-walk A..B"
When git learned to treat "git show A..B" as "git show --do-walk
A..B", it was only an accident that the implied --do-walk had that
position.
Sure, it meant that one could write "git show A..B --no-walk" to get
the ancient behavior of showing "B" unless the two commits are the
same, but that is not a convenience --- on the contrary, it's a
confusing wart that has required effort to preserve ever since (see
v1.6.0-rc2~42, 2008-07-31, for example). So let's simplify the
semantics. After this patch:
Attempts to show A..B or show -5 _always_ walk. "git log A..B
--no-walk" simply doesn't make sense, so we assume the A..B came from
the end-user who wanted to override a script's suggestion not to walk.
The effect of --no-walk is the same everywhere, except that later
--do-walk overrides early --no-walk.
There is no longer a way to limit the number of commits listed to 5
without walking or to provide a negative revision without walking.
If someone wants that, we can introduce a new option instead of
tacking it onto the hack that supports "git show".
Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
---
revision.c | 15 +++++++++------
revision.h | 1 +
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/revision.c b/revision.c
index 0f38364..0996c6f 100644
--- a/revision.c
+++ b/revision.c
@@ -133,8 +133,8 @@ void mark_parents_uninteresting(struct commit *commit)
static void add_pending_object_with_mode(struct rev_info *revs, struct object *obj, const char *name, unsigned mode)
{
- if (revs->no_walk && (obj->flags & UNINTERESTING))
- revs->no_walk = 0;
+ if (obj->flags & UNINTERESTING)
+ revs->implied_walk = 1;
if (revs->reflog_info && obj->type == OBJ_COMMIT) {
struct strbuf buf = STRBUF_INIT;
int len = interpret_branch_name(name, &buf);
@@ -1186,7 +1186,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
if ((argcount = parse_long_opt("max-count", argv, &optarg))) {
revs->max_count = atoi(optarg);
- revs->no_walk = 0;
+ revs->implied_walk = 1;
return argcount;
} else if ((argcount = parse_long_opt("skip", argv, &optarg))) {
revs->skip_count = atoi(optarg);
@@ -1194,16 +1194,16 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
} else if ((*arg == '-') && isdigit(arg[1])) {
/* accept -<digit>, like traditional "head" */
revs->max_count = atoi(arg + 1);
- revs->no_walk = 0;
+ revs->implied_walk = 1;
} else if (!strcmp(arg, "-n")) {
if (argc <= 1)
return error("-n requires an argument");
revs->max_count = atoi(argv[1]);
- revs->no_walk = 0;
+ revs->implied_walk = 1;
return 2;
} else if (!prefixcmp(arg, "-n")) {
revs->max_count = atoi(arg + 2);
- revs->no_walk = 0;
+ revs->implied_walk = 1;
} else if ((argcount = parse_long_opt("max-age", argv, &optarg))) {
revs->max_age = atoi(optarg);
return argcount;
@@ -1691,6 +1691,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
add_pending_object_with_mode(revs, object, revs->def, mode);
}
+ if (revs->implied_walk)
+ revs->no_walk = 0;
+
/* Did the user ask for any diff output? Run the diff! */
if (revs->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT)
revs->diff = 1;
diff --git a/revision.h b/revision.h
index 9fd8f30..c18d2c1 100644
--- a/revision.h
+++ b/revision.h
@@ -42,6 +42,7 @@ struct rev_info {
unsigned int dense:1,
prune:1,
no_walk:1,
+ implied_walk:1,
show_all:1,
remove_empty_trees:1,
simplify_history:1,
--
1.7.5.rc3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html