"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.
* 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.
* 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);
