[PATCH v5 0/3] builtin/ls-files.c:add git ls-file --dedup option

["阿德烈 via GitGitGadget" <gitgitgadget@gmail.com>]

I am reading the source code of git ls-files and learned that git ls-files
may have duplicate files name when there are unmerged path in a branch merge
or when different options are used at the same time. Users may fell confuse
when they see these duplicate file names.

As Junio C Hamano said ,it have odd behaviour.

Therefore, we can provide an additional option to git ls-files to delete
those repeated information.

This fixes https://github.com/gitgitgadget/git/issues/198

Thanks!

ZheNing Hu (3):
  ls_files.c: bugfix for --deleted and --modified
  ls_files.c: consolidate two for loops into one
  ls-files.c: add --deduplicate option

 Documentation/git-ls-files.txt |  5 ++
 builtin/ls-files.c             | 83 ++++++++++++++++++++--------------
 t/t3012-ls-files-dedup.sh      | 66 +++++++++++++++++++++++++++
 3 files changed, 120 insertions(+), 34 deletions(-)
 create mode 100755 t/t3012-ls-files-dedup.sh


base-commit: 6d3ef5b467eccd2769f1aa1c555d317d3c8dc707
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-832%2Fadlternative%2Fls-files-dedup-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-832/adlternative/ls-files-dedup-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/832

Range-diff vs v4:

 1:  f4d9af8a312 ! 1:  ec9464f6094 ls_files.c: bugfix for --deleted and --modified
     @@ Commit message
          This situation may occur in the original code: lstat() failed
          but we use `&st` to feed ie_modified() later.
      
     -    It's buggy!
     -
          Therefore, we can directly execute show_ce without the judgment of
          ie_modified() when lstat() has failed.
      
     @@ builtin/ls-files.c: static void show_files(struct repository *repo, struct dir_s
      -				show_ce(repo, dir, ce, fullname.buf, tag_removed);
      -			if (show_modified && ie_modified(repo->index, ce, &st, 0))
      +			if (err) {
     -+					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))
     ++				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);
       		}
       	}
 2:  50efd9b45b1 ! 2:  802ff802be8 ls_files.c: consolidate two for loops into one
     @@ builtin/ls-files.c: static void show_files(struct repository *repo, struct dir_s
      -	if (show_cached || show_stage) {
      -		for (i = 0; i < repo->index->cache_nr; i++) {
      -			const struct cache_entry *ce = repo->index->cache[i];
     --
     ++	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 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;
     @@ builtin/ls-files.c: static void show_files(struct repository *repo, struct dir_s
      -				ce_stage(ce) ? tag_unmerged :
      -				(ce_skip_worktree(ce) ? tag_skip_worktree :
      -				 tag_cached));
     --		}
     ++		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) {
      -		for (i = 0; i < repo->index->cache_nr; i++) {
      -			const struct cache_entry *ce = repo->index->cache[i];
      -			struct stat st;
      -			int 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 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;
     @@ builtin/ls-files.c: static void show_files(struct repository *repo, struct dir_s
      -				continue;
      -			err = lstat(fullname.buf, &st);
      -			if (err) {
     --					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);
     -+		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 (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))
      +		if (ce_skip_worktree(ce))
      +			continue;
      +		err = lstat(fullname.buf, &st);
      +		if (err) {
     -+				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))
     ++			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);
       	}
       
 3:  0c7830d07db ! 3:  e9c53186706 ls-files: add --deduplicate option
     @@ Metadata
      Author: ZheNing Hu <adlternative@gmail.com>
      
       ## Commit message ##
     -    ls-files: add --deduplicate option
     +    ls-files.c: add --deduplicate option
      
          In order to provide users a better experience
          when viewing information about files in the index
     @@ builtin/ls-files.c: static void show_files(struct repository *repo, struct dir_s
       		if (ce->ce_flags & CE_UPDATE)
       			continue;
       		if (show_cached || show_stage) {
     -+			if (show_cached && skipping_duplicates && last_shown_ce &&
     ++			if (skipping_duplicates && last_shown_ce &&
      +				!strcmp(last_shown_ce->name,ce->name))
      +					continue;
       			if (!show_unmerged || ce_stage(ce))
     @@ builtin/ls-files.c: static void show_files(struct repository *repo, struct dir_s
       					ce_stage(ce) ? tag_unmerged :
       					(ce_skip_worktree(ce) ? tag_skip_worktree :
       						tag_cached));
     -+			if(show_cached && skipping_duplicates)
     ++			if (show_cached && skipping_duplicates)
      +				last_shown_ce = ce;
       		}
       		if (ce_skip_worktree(ce))
       			continue;
     -+		if (skipping_duplicates && last_shown_ce && !strcmp(last_shown_ce->name,ce->name))
     -+			continue;
     ++		if (skipping_duplicates && last_shown_ce &&
     ++			!strcmp(last_shown_ce->name,ce->name))
     ++				continue;
       		err = lstat(fullname.buf, &st);
       		if (err) {
     +-			if (errno != ENOENT && errno != ENOTDIR)
     +-				error_errno("cannot lstat '%s'", fullname.buf);
     +-			if (show_deleted)
      +			if (skipping_duplicates && show_deleted && show_modified)
     -+				show_ce(repo, dir, ce, fullname.buf, tag_removed);
     + 				show_ce(repo, dir, ce, fullname.buf, tag_removed);
     +-			if (show_modified)
     +-				show_ce(repo, dir, ce, fullname.buf, tag_modified);
      +			else {
     - 				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))
     ++				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))
     + 		} else if (show_modified && ie_modified(repo->index, ce, &st, 0))
       			show_ce(repo, dir, ce, fullname.buf, tag_modified);
      +		last_shown_ce = ce;
       	}
     @@ builtin/ls-files.c: int cmd_ls_files(int argc, const char **argv, const char *cm
       	};
       
      @@ builtin/ls-files.c: int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
     - 		tag_skip_worktree = "S ";
     - 		tag_resolve_undo = "U ";
     - 	}
     -+	if (show_tag && skipping_duplicates)
     + 		 * you also show the stage information.
     + 		 */
     + 		show_stage = 1;
     ++	if (show_tag || show_stage)
      +		skipping_duplicates = 0;
     - 	if (show_modified || show_others || show_deleted || (dir.flags & DIR_SHOW_IGNORED) || show_killed)
     - 		require_work_tree = 1;
     - 	if (show_unmerged)
     + 	if (dir.exclude_per_dir)
     + 		exc_given = 1;
     + 
      
       ## t/t3012-ls-files-dedup.sh (new) ##
      @@
     @@ t/t3012-ls-files-dedup.sh (new)
      +	>b.txt &&
      +	>delete.txt &&
      +	git add a.txt b.txt delete.txt &&
     -+	git commit -m master:1 &&
     ++	git commit -m base &&
      +	echo a >a.txt &&
      +	echo b >b.txt &&
      +	echo delete >delete.txt &&
      +	git add a.txt b.txt delete.txt &&
     -+	git commit -m master:2 &&
     -+	git checkout HEAD~ &&
     -+	git switch -c dev &&
     -+	test_when_finished "git switch master" &&
     ++	git commit -m tip &&
     ++	git tag tip &&
     ++	git reset --hard HEAD^ &&
      +	echo change >a.txt &&
     -+	git add a.txt &&
     -+	git commit -m dev:1 &&
     -+	test_must_fail git merge master &&
     ++	git commit -a -m side &&
     ++	git tag side
     ++'
     ++
     ++test_expect_success 'git ls-files --deduplicate to show unique unmerged path' '
     ++	test_must_fail git merge tip &&
      +	git ls-files --deduplicate >actual &&
      +	cat >expect <<-\EOF &&
      +	a.txt
     @@ t/t3012-ls-files-dedup.sh (new)
      +	delete.txt
      +	EOF
      +	test_cmp expect actual &&
     ++	git merge --abort
     ++'
     ++
     ++test_expect_success 'git ls-files -d -m --deduplicate with different display options' '
     ++	git reset --hard side &&
     ++	test_must_fail git merge tip &&
      +	rm delete.txt &&
      +	git ls-files -d -m --deduplicate >actual &&
      +	cat >expect <<-\EOF &&
     @@ t/t3012-ls-files-dedup.sh (new)
      +	delete.txt
      +	EOF
      +	test_cmp expect actual &&
     -+	git ls-files -d -m -t  --deduplicate >actual &&
     ++	git ls-files -d -m -t --deduplicate >actual &&
      +	cat >expect <<-\EOF &&
      +	C a.txt
      +	C a.txt
     @@ t/t3012-ls-files-dedup.sh (new)
      +	C delete.txt
      +	EOF
      +	test_cmp expect actual &&
     -+	git ls-files -d -m -c  --deduplicate >actual &&
     ++	git ls-files -d -m -c --deduplicate >actual &&
      +	cat >expect <<-\EOF &&
      +	a.txt
      +	b.txt
     @@ t/t3012-ls-files-dedup.sh (new)
      +	test_cmp expect actual &&
      +	git merge --abort
      +'
     ++
      +test_done

-- 
gitgitgadget