Re: [PATCH v6 1/3] ls_files.c: bugfix for --deleted and --modified
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 23, 2021, 17:55 UTC
- Message-ID
- <xmqqwnw34jxx.fsf@gitster.c.googlers.com>
- In-Reply-To
- <fbc38ce9075d3d28187351b0fb6b34a27ec431fd.1611397210.git.gitgitgadget@gmail.com>
"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 12 quoted lines
> From: ZheNing Hu <adlternative@gmail.com> > > 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@gmail.com> > --- > builtin/ls-files.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-)
Looks good. I think we are finished with this part, except for one nit.
Show 8 quoted lines
> + if (stat_err && show_deleted) > show_ce(repo, dir, ce, fullname.buf, tag_removed); > - if (show_modified && ie_modified(repo->index, ce, &st, 0)) > - show_ce(repo, dir, ce, fullname.buf, tag_modified); > + if (show_modified && > + (stat_err || ie_modified(repo->index, ce, &st, 0))) > + show_ce(repo, dir, ce, fullname.buf, tag_modified); > }
The last line is misindented by having one leading horizontal tab too many. show_ce() for modified files and show_ce() for deleted files are done independently under different conditions and stand as equals, so the beginning of them should align to show that.
Perhaps format the last three lines more like so:
if (show_modified && (stat_err || ie_modified(repo->index, ce, &st, 0))) show_ce(repo, dir, ce, fullname.buf, tag_modified);
Again this would cascade throughout the sreies, but let's see if there are other things we may want to change in the rest of the series first. Otherwise, instead of having you rebase, I probably have time to tweak the series on my end while queuing.
Thanks.