Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
> On Mon, Apr 6, 2015 at 9:52 AM, Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote:
>> When both --cached and one of -amdAMD is used together we may have two
>> entries of the same path, e.g. " foo" and "MM foo". In this case it's
>> pretty clear that "foo" must be tracked, no need to display " foo".
>> The new function does that.
>>
>> Helped-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
>> ---
>> diff --git a/builtin/list-files.c b/builtin/list-files.c
>> index 14ffd62..31c2336 100644
>> --- a/builtin/list-files.c
>> +++ b/builtin/list-files.c
>> @@ -93,7 +93,10 @@ static int compare_item(const void *a_, const void *b_)
>> {
>> const struct item *a = a_;
>> const struct item *b = b_;
>> - return strcmp(a->path, b->path);
>> + int ret = strcmp(a->path, b->path);
>> + if (ret)
>> + return ret;
>> + return strncmp(a->tag, b->tag, 2);
>> }
>>
>> static void free_item(struct item *item)
>> @@ -132,7 +135,12 @@ static void remove_duplicates(struct item_list *list)
>> for (src = dst = 1; src < list->nr; src++) {
>> if (!compare_item(list->items + dst - 1, list->items + src))
>> free_item(list->items + src);
>> - else
>> + else if ((list->items[dst - 1].tag[0] == ' ' &&
>> + list->items[dst - 1].tag[1] == ' ' &&
>> + !strcmp(list->items[src].path, list->items[dst - 1].path))) {
>> + free_item(list->items + dst - 1);
>> + list->items[dst - 1] = list->items[src];
>
> I was wondering if you could drop this backward-patching case by
> having tag==" " items sort after tag="xx" items and just fold out the
> tag==" " items normally in the preceding 'if', however, when I
> started coding it, I found that the resulting code wasn't any more
> pleasant.
You may read from wt-status and then index (or the other way around)
into the set of files to be listed---instead of deduping at the end,
you should logically do "add in from index only for entries we did
not see in wt-status output". So some deduping is necessary.
But I actually think that the worst aspect of the series this part
of the code shows is the same issue I raised in the previous review
round in the approach, which has not been addressed at all as far as
I can see in this round.
Why does tag[] record the semantic meaning in textual form? Isn't
this a sympotom that shows the code is turning the information into
textual form way too early, which makes the code very unpleasant to
read and hard to maintain and modify.
--
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