Thread (1 message) 1 message, 1 author, 2021-01-17

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

From: Junio C Hamano <hidden>
Date: 2021-01-17 04:39:03

胡哲宁 [off-list ref] 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?
quoted
 #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?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help