Subject: Re: [PATCH v6 2/3] ls_files.c: consolidate two for loops into one

From: Junio C Hamano <[email-masked]>
Date: Sat, 23 Jan 2021 11:50:54 -0800

"ZheNing Hu via GitGitGadget" <[email-masked]> writes:

> From: ZheNing Hu <[email-masked]>
>
> Refactor the two for loops into one,skip showing the ce if it
> has the same name as the previously shown one, only when doing so
> won't lose information.

This message is all stale now.  This step does only refactoring,
without "skip showing" and others.

I've rebased the series locally and sending out a "v7" for your
review later.

Thanks.

>
> Signed-off-by: ZheNing Hu <[email-masked]>
> ---
>  builtin/ls-files.c | 70 ++++++++++++++++++++--------------------------
>  1 file changed, 30 insertions(+), 40 deletions(-)
>
> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> index 1e264bd1329..966c0ab0296 100644
> --- a/builtin/ls-files.c
> +++ b/builtin/ls-files.c
> @@ -312,49 +312,39 @@ 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) {
> -		for (i = 0; i < repo->index->cache_nr; i++) {
> -			const struct cache_entry *ce = repo->index->cache[i];
> -
> -			construct_fullname(&fullname, repo, ce);
> -
> -			if ((dir->flags & DIR_SHOW_IGNORED) &&
> -			    !ce_excluded(dir, repo->index, fullname.buf, ce))
> -				continue;
> -			if (show_unmerged && !ce_stage(ce))
> -				continue;
> -			if (ce->ce_flags & CE_UPDATE)
> -				continue;
> -			show_ce(repo, dir, ce, fullname.buf,
> -				ce_stage(ce) ? tag_unmerged :
> -				(ce_skip_worktree(ce) ? tag_skip_worktree :
> -				 tag_cached));
> -		}
> -	}
> -	if (show_deleted || show_modified) {
> -		for (i = 0; i < repo->index->cache_nr; i++) {
> -			const struct cache_entry *ce = repo->index->cache[i];
> -			struct stat st;
> -			int stat_err;
> +	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];
> +		struct stat st;
> +		int stat_err;
>  
> -			construct_fullname(&fullname, repo, ce);
> +		construct_fullname(&fullname, repo, ce);
>  
> -			if ((dir->flags & DIR_SHOW_IGNORED) &&
> -			    !ce_excluded(dir, repo->index, fullname.buf, ce))
> -				continue;
> -			if (ce->ce_flags & CE_UPDATE)
> -				continue;
> -			if (ce_skip_worktree(ce))
> -				continue;
> -			stat_err = lstat(fullname.buf, &st);
> -			if (stat_err && (errno != ENOENT && errno != ENOTDIR))
> -				error_errno("cannot lstat '%s'", fullname.buf);
> -			if (stat_err && show_deleted)
> -				show_ce(repo, dir, ce, fullname.buf, tag_removed);
> -			if (show_modified &&
> -				(stat_err || ie_modified(repo->index, ce, &st, 0)))
> -					show_ce(repo, dir, ce, fullname.buf, tag_modified);
> +		if ((dir->flags & DIR_SHOW_IGNORED) &&
> +			!ce_excluded(dir, repo->index, fullname.buf, ce))
> +			continue;
> +		if (ce->ce_flags & CE_UPDATE)
> +			continue;
> +		if (show_cached || show_stage) {
> +			if (!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 (!show_deleted && !show_modified)
> +			continue;
> +		if (ce_skip_worktree(ce))
> +			continue;
> +		stat_err = lstat(fullname.buf, &st);
> +		if (stat_err && (errno != ENOENT && errno != ENOTDIR))
> +			error_errno("cannot lstat '%s'", fullname.buf);
> +		if (stat_err && show_deleted)
> +			show_ce(repo, dir, ce, fullname.buf, tag_removed);
> +		if (show_modified &&
> +			(stat_err || ie_modified(repo->index, ce, &st, 0)))
> +				show_ce(repo, dir, ce, fullname.buf, tag_modified);
>  	}
>  
>  	strbuf_release(&fullname);