Re: [PATCH v3] ls-files.c: add --dedup option

[Junio C Hamano <gitster@pobox.com>]

胡哲宁 <adlternative@gmail.com> writes:

> Here I may disagree with your point of view:
>                  if (errno != E_NOENT)
>                          error_errno("cannot lstat '%s'", fullname.buf);
> With this sentence included, the patch will fail the test:
> t/t3010-ls-files-killed-modified.sh.
> the errno maybe ENOTDIR when you try to lstat a file`r` with `lstat("r/f",&st);`
> So I temporarily removed the judgment of errno.

I didn't mean to give you a solution that is ready to be
cut-and-pasted without thinking ;-) 

If NOTDIR needs to also be excluded, then you can exclude it just
like the above illustration excludes NOENT to solve the issue,
right?

>>  #2: consolidate two for loops into one.
>> ...
>>      This changes the semantics.  The original iterates the index
>>      twice, so you may see the same entry from --cached once and
>>      then again from --modified.  The updated one still will show
>>      the same entry twice but next to each other.
>>
> Well,This does change the semantics. I think people who used two
> for loops before may want to separate different outputs.
> Now, if you don’t use "--deduplicate", You may see six consecutive
> items under a combination of multiple options.

Yes, and that is intended and is a vast improvement from the current
behaviour, which shows 3 in the first loop, bunch of unrelated
entries from the rest of the first loop, yet another bunch of
unrelated entries from the early part of the second loop and then
finally shows 3 from the second loop.  With the "single loop"
update, at least, the entries are sorted by their path and it would
make it easier to see (if the user cares to trigger both --cached
and --modified, that is), no?