Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
>> + struct commit_list *check_reachable_list = reachable ?
>> + ref_cbdata->filter->reachable_from :
>> + ref_cbdata->filter->unreachable_from;
>> +
>> + if (!check_reachable_list)
>> + return;
>
> Rather than adding a boolean 'reachable' parameter to the function
> signature, you could instead directly pass in the `struct commit_list
> *` upon which to operate, which would allow you to drop the ternary
> operator here, and...
>
>> @@ -2322,8 +2337,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
>> - if (filter->merge_commit)
>> - do_merge_filter(&ref_cbdata);
>> + do_merge_filter(&ref_cbdata, 1);
>> + do_merge_filter(&ref_cbdata, 0);
>
> ... make the callers a bit less opaque by eliminating the
> less-than-meaningful 0-or-1, and making it obvious which list is being
> consulted:
>
> do_merge_filter(&ref_cbdata, ref_cbdata->filter->reachable_from);
> do_merge_filter(&ref_cbdata, ref_cbdata->filter->unreachable_from);
There is this code in do_merge_filter(), though.
if (is_merged == reachable)
array->items[array->nr++] = array->items[i];
else
free_array_item(item);
This is a body of the loop that runs over (surviving) tips of ref,
after painting the commits from the tips of refs (interesting) and
the [un]reachable_from commits (uninteresting). The temporary
variable is_merged signals if the tip after revision walk is painted
uninteresting, i.e. some of the [un]reachable_from commits reach the
tip.
If 'reachable' and 'is_merged' are the same, that means either
(1) the tip is reachable from some commit given as --merged
<commit>; or
(2) the tip is NOT reachable from any commit given as --no-merged
<commit>
which means it survives this round of filtering.
By losing the 'reachable' bit, you make this switch impossible.
I do not mind making the 0/1 a symbolic constant between
do_emreg_filter() and filter_refs() for enhanced readability,
though.
Thanks.
