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

[Junio C Hamano <gitster@xxxxxxxxx>]

"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>
> [jc: fixed misindented code]

I noticed that you reverted my fix in this version, when this is
compared with the one I sent last night.

Comparing the result of applying all three with what I sent last
night, this v7 looks worse (see below).  Let's discard this round
and declare victory with what is already on 'seen'.

Thanks.


---

comparison between what these three patches would produce (preimage)
and what is on 'seen' (postimage)is shown here.

diff --git w/builtin/ls-files.c c/builtin/ls-files.c
index fb9cf50d76..f6f9e483b2 100644
--- w/builtin/ls-files.c
+++ c/builtin/ls-files.c
@@ -313,7 +313,8 @@ static void show_files(struct repository *repo, struct dir_struct *dir)
 		if (show_killed)
 			show_killed_files(repo->index, dir);
 	}
-	if (! (show_cached || show_stage || show_deleted || show_modified))
+
+	if (!(show_cached || show_stage || show_deleted || show_modified))
 		return;
 	for (i = 0; i < repo->index->cache_nr; i++) {
 		const struct cache_entry *ce = repo->index->cache[i];
@@ -328,15 +329,16 @@ static void show_files(struct repository *repo, struct dir_struct *dir)
 		if (ce->ce_flags & CE_UPDATE)
 			continue;
 		if ((show_cached || show_stage) &&
-			(!show_unmerged || ce_stage(ce))) {
-				show_ce(repo, dir, ce, fullname.buf,
-					ce_stage(ce) ? tag_unmerged :
-					(ce_skip_worktree(ce) ? tag_skip_worktree :
-						tag_cached));
+		    (!show_unmerged || ce_stage(ce))) {
+			show_ce(repo, dir, ce, fullname.buf,
+				ce_stage(ce) ? tag_unmerged :
+				(ce_skip_worktree(ce) ? tag_skip_worktree :
+				 tag_cached));
 			if (skipping_duplicates)
 				goto skip_to_next_name;
 		}
-		if (!show_deleted && !show_modified)
+
+		if (!(show_deleted || show_modified))
 			continue;
 		if (ce_skip_worktree(ce))
 			continue;
@@ -349,12 +351,13 @@ static void show_files(struct repository *repo, struct dir_struct *dir)
 				goto skip_to_next_name;
 		}
 		if (show_modified &&
-			(stat_err || ie_modified(repo->index, ce, &st, 0))) {
-				show_ce(repo, dir, ce, fullname.buf, tag_modified);
+		    (stat_err || ie_modified(repo->index, ce, &st, 0))) {
+			show_ce(repo, dir, ce, fullname.buf, tag_modified);
 			if (skipping_duplicates)
 				goto skip_to_next_name;
 		}
 		continue;
+
 skip_to_next_name:
 		{
 			int j;
@@ -362,7 +365,7 @@ static void show_files(struct repository *repo, struct dir_struct *dir)
 			for (j = i + 1; j < repo->index->cache_nr; j++)
 				if (strcmp(ce->name, cache[j]->name))
 					break;
-			i = j - 1; /* compensate for outer for loop */
+			i = j - 1; /* compensate for the for loop */
 		}
 	}
 
@@ -590,7 +593,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 			N_("pretend that paths removed since <tree-ish> are still present")),
 		OPT__ABBREV(&abbrev),
 		OPT_BOOL(0, "debug", &debug_mode, N_("show debugging data")),
-		OPT_BOOL(0,"deduplicate",&skipping_duplicates,N_("suppress duplicate entries")),
+		OPT_BOOL(0, "deduplicate", &skipping_duplicates,
+			 N_("suppress duplicate entries")),
 		OPT_END()
 	};