Junio C Hamano <gitster@xxxxxxxxx> 于2021年1月21日周四 上午4:26写道：
>
> "ZheNing Hu via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
> > From: ZheNing Hu <adlternative@xxxxxxxxx>
> >
> > This situation may occur in the original code: lstat() failed
> > but we use `&st` to feed ie_modified() later.
> >
> > Therefore, we can directly execute show_ce without the judgment of
> > ie_modified() when lstat() has failed.
> >
> > Signed-off-by: ZheNing Hu <adlternative@xxxxxxxxx>
> > ---
>
> Thanks. A few comments:
>
> * The error_errno() line is not indented correctly; I'll fix it up
> while queuing, but it would conflict with 2/3 as you'll be moving
> that line around.
>
I might not have noticed,very Sorry.
> * When we say "error", we do not even know if the thing got removed
> or modified at all, so it is somewhat strange to report it as
> such (the path may be intact and the only issue may be that we
> cannot read the containing directory). It is equally strange not
> to say anything on the path, and between the two, there isn't
> clearly a more correct answer. What you implemented here does
> not change the traditional behaviour of reporting it as
> deleted/modified to "alert" the user, which I think is good.
>
Haha,thanks!
> * The logic for modified entry looks a bit duplicated. I wonder if
> the one at the end of this message reads better. Renaming err to
> stat_err is optional, but I think the name makes it clear why it
> is sensible that these two places use the variable as a sign that
> the path was deleted and/or modified.
>
> > err = lstat(fullname.buf, &st);
> > + if (err) {
> > + if (errno != ENOENT && errno != ENOTDIR)
> > + error_errno("cannot lstat '%s'", fullname.buf);
> > + if (show_deleted)
> > + show_ce(repo, dir, ce, fullname.buf, tag_removed);
> > + if (show_modified)
> > + show_ce(repo, dir, ce, fullname.buf, tag_modified);
> > + } else if (show_modified && ie_modified(repo->index, ce, &st, 0))
> > show_ce(repo, dir, ce, fullname.buf, tag_modified);
>
>
> stat_err = lstat(...);
> if (stat_err && (errno != ENOENT && errno != ENOTDIR))
> error_errno("cannot lstat '%s'", fullname.buf);
>
> if (show_deleted && stat_err)
> show_ce(..., tag_removed);
> if (show_modified &&
> (stat_err || ie_modified(..., &st, 0)))
> show_ce(..., tag_modified);
>
Yes,"stat_err" may better then "err".
And putting the conditions together will make the code more compact.
Thanks for your comment:)
