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

Re: [PATCH v6 1/3] ls_files.c: bugfix for --deleted and --modified

From: Junio C Hamano <hidden>
Date: 2021-01-23 17:56:09

"ZheNing Hu via GitGitGadget" [off-list ref] writes:
From: ZheNing Hu <redacted>

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 <redacted>
---
 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.
+			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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help