On Sun, Sep 13, 2015 at 5:21 PM, Matthieu Moy
<Matthieu.Moy@xxxxxxxxxxxxxxx> wrote:
> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
>
>> @@ -667,26 +675,22 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
>> clear_commit_marks(item->commit, ALL_REV_FLAGS);
>> }
>> clear_commit_marks(filter, ALL_REV_FLAGS);
>> -
>> - if (verbose)
>> - ref_list.maxwidth = calc_maxwidth(&ref_list);
>> }
>> + if (verbose)
>> + maxwidth = calc_maxwidth(&ref_list, strlen(remote_prefix));
>
> I don't understand this hunk. To give a bit more context, the closing
> brace corresponds to:
>
> if (merge_filter != NO_FILTER) {
>
> Hence this patch gets the two lines out of this "if". Actually, I don't
> understand how it could work previously. Wasn't this "calc_maxwidth"
> needed regardless of merge_filter from the beginning?
Previously, we used to compute and store each item's width in append_ref()
and change the ref_list.maxwidth only if we find a new maxwidth (all
within append_ref()).
Although the maxwidth variable is only needed when we use the verbose
option this is computed otherwise also (without the usage of calc_maxwidth()).
When using the merge option with verbose the maxwidth would need to be
recalculated. Hence we compute the maxwidth again using
calc_maxwidth() within the
if block.
Currently we only compute and store maxwidth if required, after
obtaining required refs.
>
> In any case, that remark is not an objection on your patch, but I'd like
> to understand.
>
Happy to explain.
--
Regards,
Karthik Nayak
--
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