Junio C Hamano <gitster@xxxxxxxxx> writes:
> Micronit. When splitting "for (init; fini; cont)" into multiple
> lines, it is often easier to read to make that into three lines:
>
> for (parent_number = 1, parents = commit->parents;
> parents;
> parents = parents->next, parent_number++) {
>
>> + if (exclude_parent && parent_number != exclude_parent)
>> + continue;
>> +
>> + show_rev(include_parents ? NORMAL : REVERSED,
>> + parents->item->object.oid.hash, arg);
>> + }
>
> It is very clear to see what is going on. Good job.
>
>> *dotdot = '^';
>> + if (exclude_parent >= parent_number)
>> + return 0;
>
> This is not quite nice. You've already called show_rev() number of
> times, and it is too late to signal an error here. I think you
> would need to count the number of parents much earlier when
> exclude_parent option is in effect and error out before making any
> call to show_rev().
> ...
> Likewise. It is way too late to say "Nah, this wasn't a valid rev^-
> notation after all" to the caller after calling add_rev_cmdline()
> and add_pending_object() in the above loop.
Taking these two together, perhaps squashing this in may be
sufficient.
Please do not use --no-prefix when sending a patch to this list, by
the way.
builtin/rev-parse.c | 18 +++++++++++++++---
revision.c | 17 ++++++++++++++++-
2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 2c3da19..9474c37 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -333,8 +333,22 @@ static int try_parent_shorthands(const char *arg)
if (include_rev)
show_rev(NORMAL, sha1, arg);
commit = lookup_commit_reference(sha1);
+
+ if (exclude_parent) {
+ /* do we have enough parents? */
+ for (parent_number = 0, parents = commit->parents;
+ parents;
+ parents = parents->next)
+ parent_number++;
+ if (parent_number < exclude_parent) {
+ *dotdot = '^';
+ return 0;
+ }
+ }
+
for (parent_number = 1, parents = commit->parents;
- parents; parents = parents->next, parent_number++) {
+ parents;
+ parents = parents->next, parent_number++) {
if (exclude_parent && parent_number != exclude_parent)
continue;
@@ -343,8 +357,6 @@ static int try_parent_shorthands(const char *arg)
}
*dotdot = '^';
- if (exclude_parent >= parent_number)
- return 0;
return 1;
}
diff --git a/revision.c b/revision.c
index 511e1ed..09da7f4 100644
--- a/revision.c
+++ b/revision.c
@@ -1318,8 +1318,23 @@ static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,
if (it->type != OBJ_COMMIT)
return 0;
commit = (struct commit *)it;
+
+ if (exclude_parent) {
+ struct commit_list *parents;
+ int parent_number;
+
+ /* do we have enough parents? */
+ for (parent_number = 0, parents = commit->parents;
+ parents;
+ parents = parents->next)
+ parent_number++;
+ if (parent_number < exclude_parent)
+ return 0;
+ }
+
for (parent_number = 1, parents = commit->parents;
- parents; parents = parents->next, parent_number++) {
+ parents;
+ parents = parents->next, parent_number++) {
if (exclude_parent && parent_number != exclude_parent)
continue;