Junio,
I appreciate you taking the time to look over this patch. I have some
comments below before re-submitting an updated version.
On 08/24/2017 04:07 PM, Junio C Hamano wrote:
> Sonny Michaud <michaud.sonny@xxxxxxxxx> writes:
>
>> diff --git a/wt-status.c b/wt-status.c
>> index 77c27c511..651bb01f0 100644
>> --- a/wt-status.c
>> +++ b/wt-status.c
>> @@ -1827,6 +1827,15 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)
>> fputc(s->null_termination ? '\0' : '\n', s->fp);
>> }
>>
>> +static void wt_shortstatus_print_stash_summary(struct wt_status *s)
>> +{
>> + int stash_count = 0;
>> +
>> + for_each_reflog_ent("refs/stash", stash_count_refs, &stash_count);
> A singleton instance of this in wt_longstatus_print_stash_summary()
> thing was OK, but let's not duplicate and spread the badness. Have
> a simple there-liner helper function "static int stash_count(void);"
> that does the above and returns the stash_count, and use it from
> both places.
That is reasonable.
>> + if (stash_count > 0)
>> + color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "## Stash entries: %d", stash_count);
> That's a funny way to indent (dedent?) a body of an if() statement.
My bad, I can clean that up!
>
> Don't scripts that read this output (I notice that this is also
> called by wt_porcelain_print() function) expect that entries that
> are led by "##" are about the current branch and its tracking
> information?
>
> This patch would break these script by adding this new line using
> the same "##" leader.
>
I did not consider porcelain output; it looks like the headers are
stripped from it, though, so this might not be an issue. Do you think
this is a worthwhile path to continue on?
Thanks,
Sonny
