From: 阿德烈 via GitGitGadget <hidden> Date: 2021-01-06 08:54:02
From: ZheNing Hu <redacted>
1.When we use git ls-files with both -m -d,
we would find that repeated path,sometimes
it is confusing.
2.When we are performing a branch merge,
the default git ls-files will also output
multiple repeated file names.
Therefore, I added the --dedup option to git ls-files.
1. It can be achieved that only the deleted file name
is displayed when using -m, -d, and --dedup at the same time.
2. Add --dedup when merging branches to remove duplicate file
names. (unless -s, -u are used)
Signed-off-by: ZheNing Hu <redacted>
---
builtin/ls-files.c:add git ls-file --dedup option
I am reading the source code of git ls-files and learned that git ls
-files may have duplicate entries when conflict occurs in a branch merge
or when different options are used at the same time. Users may fell
confuse when they see these duplicate entries.
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!
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-832%2Fadlternative%2Fls-files-dedup-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-832/adlternative/ls-files-dedup-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/832
builtin/ls-files.c | 43 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 5 deletions(-)
@@ -35,6 +35,7 @@ static int line_terminator = '\n';staticintdebug_mode;staticintshow_eol;staticintrecurse_submodules;+staticintdelete_dup;staticconstchar*prefix;staticintmax_prefix_len;
@@ -301,6 +302,7 @@ static void show_files(struct repository *repo, struct dir_struct *dir){inti;structstrbuffullname=STRBUF_INIT;+conststructcache_entry*last_stage=NULL;/* For cached/deleted files we don't need to even do the readdir */if(show_others||show_killed){
@@ -578,6 +610,7 @@ 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,"dedup",&delete_dup,N_("delete duplicate entry in index")),OPT_END()};
From: Eric Sunshine <hidden> Date: 2021-01-07 06:11:19
On Wed, Jan 6, 2021 at 3:54 AM 阿德烈 via GitGitGadget
[off-list ref] wrote:
[...]
Therefore, I added the --dedup option to git ls-files.
1. It can be achieved that only the deleted file name
is displayed when using -m, -d, and --dedup at the same time.
2. Add --dedup when merging branches to remove duplicate file
names. (unless -s, -u are used)
I'm just pointing out a few minor style issues below; I'm not properly
reviewing the patch...
This change adds a new command-line option, so the documentation
(Documentation/git-ls-files.txt) should be updated and at least one
new test should be added (in one of the t/t30??-ls-files-*.sh scripts
probably).
From: 阿德烈 via GitGitGadget <hidden> Date: 2021-01-08 14:38:04
I am reading the source code of git ls-files and learned that git ls -files
may have duplicate entries when conflict occurs in a branch merge or when
different options are used at the same time. Users may fell confuse when
they see these duplicate entries.
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 (2):
builtin/ls-files.c:add git ls-file --dedup option
builtin:ls-files.c:add git ls-file --dedup option
Documentation/git-ls-files.txt | 4 +++
builtin/ls-files.c | 41 ++++++++++++++++++++--
t/t3012-ls-files-dedup.sh | 63 ++++++++++++++++++++++++++++++++++
3 files changed, 105 insertions(+), 3 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-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-832/adlternative/ls-files-dedup-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/832
Range-diff vs v1:
1: 0261e5d245e = 1: 0261e5d245e builtin/ls-files.c:add git ls-file --dedup option
-: ----------- > 2: a09a5098aa6 builtin:ls-files.c:add git ls-file --dedup option
--
gitgitgadget
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-01-08 14:38:04
From: ZheNing Hu <redacted>
1.When we use git ls-files with both -m -d,
we would find that repeated path,sometimes
it is confusing.
2.When we are performing a branch merge,
the default git ls-files will also output
multiple repeated file names.
Therefore, I added the --dedup option to git ls-files.
1. It can be achieved that only the deleted file name
is displayed when using -m, -d, and --dedup at the same time.
2. Add --dedup when merging branches to remove duplicate file
names. (unless -s, -u are used)
Signed-off-by: ZheNing Hu <redacted>
---
builtin/ls-files.c | 43 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 5 deletions(-)
@@ -35,6 +35,7 @@ static int line_terminator = '\n';staticintdebug_mode;staticintshow_eol;staticintrecurse_submodules;+staticintdelete_dup;staticconstchar*prefix;staticintmax_prefix_len;
@@ -301,6 +302,7 @@ static void show_files(struct repository *repo, struct dir_struct *dir){inti;structstrbuffullname=STRBUF_INIT;+conststructcache_entry*last_stage=NULL;/* For cached/deleted files we don't need to even do the readdir */if(show_others||show_killed){
@@ -578,6 +610,7 @@ 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,"dedup",&delete_dup,N_("delete duplicate entry in index")),OPT_END()};
@@ -81,6 +82,9 @@ OPTIONS \0 line termination on output and do not quote filenames. See OUTPUT below for more information.+--dedup::+ Suppress duplicates entries when conflicts happen or+ specify -d -m at the same time. -x <pattern>:: --exclude=<pattern>:: Skip untracked files matching pattern.
@@ -302,7 +302,7 @@ static void show_files(struct repository *repo, struct dir_struct *dir){inti;structstrbuffullname=STRBUF_INIT;-conststructcache_entry*last_stage=NULL;+conststructcache_entry*last_stage=NULL;/* For cached/deleted files we don't need to even do the readdir */if(show_others||show_killed){
@@ -610,7 +612,7 @@ 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,"dedup",&delete_dup,N_("delete duplicate entry in index")),+OPT_BOOL(0,"dedup",&delete_dup,N_("suppress duplicate entries")),OPT_END()};
From: Eric Sunshine <hidden> Date: 2021-01-14 06:40:08
On Fri, Jan 8, 2021 at 9:36 AM ZheNing Hu via GitGitGadget
[off-list ref] wrote:
builtin:ls-files.c:add git ls-file --dedup option
This subject concisely explains the purpose of the patch. That's good.
A more typical way to write it would be:
ls-files: add --dedup option
This commit standardizes the code format.
Fixing problems pointed out by reviewers is good. Normally, however,
when you submit a new version of your patch or patch series, you
should apply these fixes directly to the patch(es) which introduced
the problems in the first place rather than adding one or more
additional patches to fix problems introduced in earlier patches. To
do this, you typically would use `git rebase -i` or `git commit
--amend` to squash the fixes into the problematic patches. Thus, when
you re-submit the patches, they will appear to be "perfect".
For this particular two-patch series, patch [2/2] is doing two things:
(1) fixing style problems from patch [1/2], and (2) adding
documentation and tests which logically belong with the feature added
by patch [1/2]. Taking the above advice into account, a better
presentation when you re-submit this series would be to squash these
two patches into a single patch.
@@ -81,6 +82,9 @@ OPTIONS+--dedup::+ Suppress duplicates entries when conflicts happen or+ specify -d -m at the same time.
For consistency with typesetting elsewhere in this file, use backticks
around the command-line options. It also often is a good idea to spell
the options using long form since it is typically easier to search for
the long form of an option in documentation. So, perhaps the above can
be written like this:
Suppress duplicate entries when `--deleted` and `--modified` are
combined.
As mentioned above, these style fixes should be squashed into the
first patch, rather than being done in a separate patch, so that
reviewers see a nicely polished patch rather than a patch which
requires later fixing up.
@@ -0,0 +1,63 @@+test_expect_success 'master branch setup and write expect1 expect2 and commit' '
We usually give this test a simple title such as "setup" so that we
don't have to worry about the title becoming outdated as people make
changes to the test itself.
On this project, we use `touch` when the timestamp of the empty files
is important to the test. If the timestamp is not important, then we
just use `>`, like this:
>a.txt &&
>b.txt &&
>delete.txt &&
+ cat <<-EOF >expect1 &&
+ M a.txt
+ H b.txt
+ H delete.txt
+ H expect1
+ H expect2
+ EOF
+ cat <<-EOF >expect2 &&
+ C a.txt
+ R delete.txt
+ EOF
When no variables are being interpolated in the here-doc content, we
use -\EOF to let readers know that the here-doc body is literal. So:
cat >expect1 <<-\EOF &&
...
EOF
These two tests following the "setup" test also seem to be doing setup
tasks rather than testing the new --dedup functionality. If this is
the case, then it probably would make sense to combine all three tests
into a single "setup" test.
Do you foresee that people will add more tests to this file which will
use the files and branches set up by the "setup" test(s)? If not, if
those branches and files are only ever going to be used by this one
test, then it probably would be better to combine all the above code
into a single test.
You can see that the coding and documentation of GIT community are really very
standard, which may be one of the things I lack and need to improve ;)
Thanks for patiently correct my errors.
Eric Sunshine [off-list ref] 于2021年1月14日周四 下午2:39写道：
On Fri, Jan 8, 2021 at 9:36 AM ZheNing Hu via GitGitGadget
[off-list ref] wrote:
quoted
builtin:ls-files.c:add git ls-file --dedup option
This subject concisely explains the purpose of the patch. That's good.
A more typical way to write it would be:
ls-files: add --dedup option
OK.I will correct it more specification.
quoted
This commit standardizes the code format.
Fixing problems pointed out by reviewers is good. Normally, however,
when you submit a new version of your patch or patch series, you
should apply these fixes directly to the patch(es) which introduced
the problems in the first place rather than adding one or more
additional patches to fix problems introduced in earlier patches. To
do this, you typically would use `git rebase -i` or `git commit
--amend` to squash the fixes into the problematic patches. Thus, when
you re-submit the patches, they will appear to be "perfect".
For this particular two-patch series, patch [2/2] is doing two things:
(1) fixing style problems from patch [1/2], and (2) adding
documentation and tests which logically belong with the feature added
by patch [1/2]. Taking the above advice into account, a better
presentation when you re-submit this series would be to squash these
two patches into a single patch.
I thought before this was gitgitgadget would sent duplicate patch
over and over again. It seems like I really should go straight ahead
and squash my commits , so I know what I should do.
@@ -81,6 +82,9 @@ OPTIONS+--dedup::+ Suppress duplicates entries when conflicts happen or+ specify -d -m at the same time.
For consistency with typesetting elsewhere in this file, use backticks
around the command-line options. It also often is a good idea to spell
the options using long form since it is typically easier to search for
the long form of an option in documentation. So, perhaps the above can
be written like this:
Suppress duplicate entries when `--deleted` and `--modified` are
combined.
As mentioned above, these style fixes should be squashed into the
first patch, rather than being done in a separate patch, so that
reviewers see a nicely polished patch rather than a patch which
requires later fixing up.
@@ -0,0 +1,63 @@+test_expect_success 'master branch setup and write expect1 expect2 and commit' '
We usually give this test a simple title such as "setup" so that we
don't have to worry about the title becoming outdated as people make
changes to the test itself.
On this project, we use `touch` when the timestamp of the empty files
is important to the test. If the timestamp is not important, then we
just use `>`, like this:
>a.txt &&
>b.txt &&
>delete.txt &&
OK,maybe because I always use touch to generate files.
quoted
+ cat <<-EOF >expect1 &&
+ M a.txt
+ H b.txt
+ H delete.txt
+ H expect1
+ H expect2
+ EOF
+ cat <<-EOF >expect2 &&
+ C a.txt
+ R delete.txt
+ EOF
When no variables are being interpolated in the here-doc content, we
use -\EOF to let readers know that the here-doc body is literal. So:
cat >expect1 <<-\EOF &&
...
EOF
These two tests following the "setup" test also seem to be doing setup
tasks rather than testing the new --dedup functionality. If this is
the case, then it probably would make sense to combine all three tests
into a single "setup" test.
Do you foresee that people will add more tests to this file which will
use the files and branches set up by the "setup" test(s)? If not, if
those branches and files are only ever going to be used by this one
test, then it probably would be better to combine all the above code
into a single test.
From: 阿德烈 via GitGitGadget <hidden> Date: 2021-01-14 12:23:08
From: ZheNing Hu <redacted>
In order to provide users a better experience
when viewing information about files in the index
and the working tree, the `--dedup` option will suppress
some duplicate options under some conditions.
In a merge conflict, one item of "git ls-files" output may
appear multiple times. For example,now the file `a.c` has
a conflict,`a.c` will appear three times in the output of
"git ls-files".We can use "git ls-files --dedup" to output
`a.c` only one time.(unless `--stage` or `--unmerged` is
used to view all the detailed information in the index)
In addition, if you use both `--delete` and `--modify` in
the same time, The `--dedup` option can also suppress modified
entries output.
`--dedup` option relevant descriptions in
`Documentation/git-ls-files.txt`,
the test script in `t/t3012-ls-files-dedup.sh`
prove the correctness of the `--dedup` option.
this patch fixed:
https://github.com/gitgitgadget/git/issues/198
Thanks.
Signed-off-by: ZheNing Hu <redacted>
---
builtin/ls-files.c:add git ls-file --dedup option
I am reading the source code of git ls-files and learned that git ls
-files may have duplicate entries when conflict occurs in a branch merge
or when different options are used at the same time. Users may fell
confuse when they see these duplicate entries.
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!
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-832%2Fadlternative%2Fls-files-dedup-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-832/adlternative/ls-files-dedup-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/832
Range-diff vs v2:
1: 0261e5d245e < -: ----------- builtin/ls-files.c:add git ls-file --dedup option
2: a09a5098aa6 ! 1: 5ce52c8b7a4 builtin:ls-files.c:add git ls-file --dedup option
@@ Metadata
Author: ZheNing Hu [off-list ref]
## Commit message ##
- builtin:ls-files.c:add git ls-file --dedup option
+ ls-files.c: add --dedup option
- This commit standardizes the code format.
- For git ls-file --dedup option added
- relevant descriptions in Documentation/git-ls-files.txt
- and wrote t/t3012-ls-files-dedup.sh test script
- to prove the correctness of--dedup option.
+ In order to provide users a better experience
+ when viewing information about files in the index
+ and the working tree, the `--dedup` option will suppress
+ some duplicate options under some conditions.
- this patch fixed: https://github.com/gitgitgadget/git/issues/198
+ In a merge conflict, one item of "git ls-files" output may
+ appear multiple times. For example,now the file `a.c` has
+ a conflict,`a.c` will appear three times in the output of
+ "git ls-files".We can use "git ls-files --dedup" to output
+ `a.c` only one time.(unless `--stage` or `--unmerged` is
+ used to view all the detailed information in the index)
+
+ In addition, if you use both `--delete` and `--modify` in
+ the same time, The `--dedup` option can also suppress modified
+ entries output.
+
+ `--dedup` option relevant descriptions in
+ `Documentation/git-ls-files.txt`,
+ the test script in `t/t3012-ls-files-dedup.sh`
+ prove the correctness of the `--dedup` option.
+
+ this patch fixed:
+ https://github.com/gitgitgadget/git/issues/198
Thanks.
Signed-off-by: ZheNing Hu [off-list ref]
@@ Documentation/git-ls-files.txt: OPTIONS
See OUTPUT below for more information.
+--dedup::
-+ Suppress duplicates entries when conflicts happen or
-+ specify -d -m at the same time.
++ Suppress duplicate entries when conflict happen or `--deleted`
++ and `--modified` are combined.
++
-x <pattern>::
--exclude=<pattern>::
Skip untracked files matching pattern.
## builtin/ls-files.c ##
+@@ builtin/ls-files.c: static int line_terminator = '\n';
+ static int debug_mode;
+ static int show_eol;
+ static int recurse_submodules;
++static int delete_dup;
+
+ static const char *prefix;
+ static int max_prefix_len;
@@ builtin/ls-files.c: static void show_files(struct repository *repo, struct dir_struct *dir)
{
int i;
struct strbuf fullname = STRBUF_INIT;
-- const struct cache_entry *last_stage=NULL;
+ const struct cache_entry *last_stage = NULL;
/* For cached/deleted files we don't need to even do the readdir */
if (show_others || show_killed) {
@@ builtin/ls-files.c: static void show_files(struct repository *repo, struct dir_struct *dir)
- 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 && delete_dup){
-+
+
+ if (show_cached && delete_dup) {
- switch (ce_stage(ce)) {
- case 0:
- default:
-@@ builtin/ls-files.c: static void show_files(struct repository *repo, struct dir_struct *dir)
- if (last_stage &&
- !strcmp(last_stage->name, ce->name))
- continue;
-- last_stage=ce;
++ switch (ce_stage(ce)) {
++ case 0:
++ default:
++ break;
++ case 1:
++ case 2:
++ case 3:
++ if (last_stage &&
++ !strcmp(last_stage->name, ce->name))
++ continue;
+ last_stage = ce;
- }
- }
++ }
++ }
construct_fullname(&fullname, repo, ce);
+
+ if ((dir->flags & DIR_SHOW_IGNORED) &&
@@ builtin/ls-files.c: static void show_files(struct repository *repo, struct dir_struct *dir)
- const struct cache_entry *ce = repo->index->cache[i];
struct stat st;
int err;
-- if(delete_dup){
-+
+
+ if (delete_dup) {
- switch (ce_stage(ce)) {
- case 0:
- default:
-@@ builtin/ls-files.c: static void show_files(struct repository *repo, struct dir_struct *dir)
- if (last_stage &&
- !strcmp(last_stage->name, ce->name))
- continue;
-- last_stage=ce;
++ switch (ce_stage(ce)) {
++ case 0:
++ default:
++ break;
++ case 1:
++ case 2:
++ case 3:
++ if (last_stage &&
++ !strcmp(last_stage->name, ce->name))
++ continue;
+ last_stage = ce;
- }
- }
++ }
++ }
construct_fullname(&fullname, repo, ce);
+
+ if ((dir->flags & DIR_SHOW_IGNORED) &&
@@ builtin/ls-files.c: static void show_files(struct repository *repo, struct dir_struct *dir)
if (ce_skip_worktree(ce))
continue;
err = lstat(fullname.buf, &st);
-- if(delete_dup && show_deleted && show_modified && err)
+- if (show_deleted && err)
+ if (delete_dup && show_deleted && show_modified && err)
show_ce(repo, dir, ce, fullname.buf, tag_removed);
-- else{
-- if (show_deleted && err)/* you can't find it,so it's actually removed at all! */
+- if (show_modified && ie_modified(repo->index, ce, &st, 0))
+- show_ce(repo, dir, ce, fullname.buf, tag_modified);
+ else {
+ if (show_deleted && err)
- 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);
++ 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);
++ }
+ }
+ }
+
@@ builtin/ls-files.c: 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, "dedup", &delete_dup, N_("delete duplicate entry in index")),
+ OPT_BOOL(0, "dedup", &delete_dup, N_("suppress duplicate entries")),
OPT_END()
};
@@ t/t3012-ls-files-dedup.sh (new)
+
+. ./test-lib.sh
+
-+test_expect_success 'master branch setup and write expect1 expect2 and commit' '
-+ touch a.txt &&
-+ touch b.txt &&
-+ touch delete.txt &&
-+ cat <<-EOF >expect1 &&
++test_expect_success 'setup' '
++ > a.txt &&
++ > b.txt &&
++ > delete.txt &&
++ cat >expect1<<-\EOF &&
+ M a.txt
+ H b.txt
+ H delete.txt
+ H expect1
+ H expect2
+ EOF
-+ cat <<-EOF >expect2 &&
++ cat >expect2<<-EOF &&
+ C a.txt
+ R delete.txt
+ EOF
+ git add a.txt b.txt delete.txt expect1 expect2 &&
-+ git commit -m master:1
-+'
-+
-+test_expect_success 'main commit again' '
++ git commit -m master:1 &&
+ echo a>a.txt &&
+ echo b>b.txt &&
-+ echo delete>delete.txt &&
++ echo delete >delete.txt &&
+ git add a.txt b.txt delete.txt &&
-+ git commit -m master:2
-+'
-+
-+test_expect_success 'dev commit' '
++ git commit -m master:2 &&
+ git checkout HEAD~ &&
+ git switch -c dev &&
-+ echo change>a.txt &&
++ echo change >a.txt &&
+ git add a.txt &&
-+ git commit -m dev:1
-+'
-+
-+test_expect_success 'dev merge master' '
++ git commit -m dev:1 &&
+ test_must_fail git merge master &&
+ git ls-files -t --dedup >actual1 &&
+ test_cmp expect1 actual1 &&
Documentation/git-ls-files.txt | 5 ++++
builtin/ls-files.c | 41 ++++++++++++++++++++++++--
t/t3012-ls-files-dedup.sh | 54 ++++++++++++++++++++++++++++++++++
3 files changed, 97 insertions(+), 3 deletions(-)
create mode 100755 t/t3012-ls-files-dedup.sh
@@ -81,6 +82,10 @@ OPTIONS \0 line termination on output and do not quote filenames. See OUTPUT below for more information.+--dedup::+ Suppress duplicate entries when conflict happen or `--deleted`+ and `--modified` are combined.+ -x <pattern>:: --exclude=<pattern>:: Skip untracked files matching pattern.
@@ -35,6 +35,7 @@ static int line_terminator = '\n';staticintdebug_mode;staticintshow_eol;staticintrecurse_submodules;+staticintdelete_dup;staticconstchar*prefix;staticintmax_prefix_len;
@@ -301,6 +302,7 @@ static void show_files(struct repository *repo, struct dir_struct *dir){inti;structstrbuffullname=STRBUF_INIT;+conststructcache_entry*last_stage=NULL;/* For cached/deleted files we don't need to even do the readdir */if(show_others||show_killed){
@@ -578,6 +612,7 @@ 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,"dedup",&delete_dup,N_("suppress duplicate entries")),OPT_END()};
From: Eric Sunshine <hidden> Date: 2021-01-16 07:16:58
On Thu, Jan 14, 2021 at 7:22 AM 阿德烈 via GitGitGadget
[off-list ref] wrote:
In order to provide users a better experience
when viewing information about files in the index
and the working tree, the `--dedup` option will suppress
some duplicate options under some conditions.
[...]
I have a few very minor comments alongside Junio's review comments...
@@ -0,0 +1,54 @@+test_description='git ls-files --dedup test.++This test prepares the following in the cache:++ a.txt - a file(base)+ a.txt - a file(master)+ a.txt - a file(dev)+ b.txt - a file+ delete.txt - a file+ expect1 - a file+ expect2 - a file++'
This test script description is outdated now. Perhaps shorten it to:
test_description='ls-files dedup tests'
Or, it might be suitable to simply add the new test to the existing
t3004-ls-files-basic.sh instead.
If someone adds a new test after this test, then that new test will
run in the "dev" branch, which might be unexpected or undesirable. It
often is a good idea to ensure that tests do certain types of cleanup
to avoid breaking subsequent tests. Here, it would be a good idea to
ensure that the test switches back to the original branch when it
finishes (regardless of whether it finishes successfully or
unsuccessfully).
git switch -c dev &&
test_when_finished "git switch master" &&
Or you could use `git switch -` if you don't want to hard-code the
name "master" in the test (since there has been effort lately to
remove that name from tests.
We usually don't bother giving temporary files unique names like
"actual1" and "actual2" unless those files must exist at the same
time. This is because unique names like this may confuse readers into
wondering if there is some hidden interdependency between the files.
In this case, the files don't need to exist at the same time, so it
may be better simply to use the names "actual" and "expect", like
this:
...other stuff...
cat >expect <<-\EOF &&
...
EOF
git ls-files -t --dedup >actual &&
test_cmp expect actual &&
rm delete.txt &&
cat >expect <<-\EOF &&
...
EOF
git ls-files -d -m -t --dedup >actual &&
test_cmp expect actual
(It also has the benefit that the "expect" content is closer to the
place where it is actually used, which may make it a bit easier for a
person reading the test to understand what is supposed to be
produced.)
Eric,Thanks!
I have little confuse about I can use` test_when_finished "git switch master" `,
but I can't use` test_when_finished "git switch -" `,
why？
Eric Sunshine [off-list ref] 于2021年1月16日周六 下午3:13写道：
On Thu, Jan 14, 2021 at 7:22 AM 阿德烈 via GitGitGadget
[off-list ref] wrote:
quoted
In order to provide users a better experience
when viewing information about files in the index
and the working tree, the `--dedup` option will suppress
some duplicate options under some conditions.
[...]
I have a few very minor comments alongside Junio's review comments...
@@ -0,0 +1,54 @@+test_description='git ls-files --dedup test.++This test prepares the following in the cache:++ a.txt - a file(base)+ a.txt - a file(master)+ a.txt - a file(dev)+ b.txt - a file+ delete.txt - a file+ expect1 - a file+ expect2 - a file++'
This test script description is outdated now. Perhaps shorten it to:
test_description='ls-files dedup tests'
Or, it might be suitable to simply add the new test to the existing
t3004-ls-files-basic.sh instead.
If someone adds a new test after this test, then that new test will
run in the "dev" branch, which might be unexpected or undesirable. It
often is a good idea to ensure that tests do certain types of cleanup
to avoid breaking subsequent tests. Here, it would be a good idea to
ensure that the test switches back to the original branch when it
finishes (regardless of whether it finishes successfully or
unsuccessfully).
git switch -c dev &&
test_when_finished "git switch master" &&
Or you could use `git switch -` if you don't want to hard-code the
name "master" in the test (since there has been effort lately to
remove that name from tests.
We usually don't bother giving temporary files unique names like
"actual1" and "actual2" unless those files must exist at the same
time. This is because unique names like this may confuse readers into
wondering if there is some hidden interdependency between the files.
In this case, the files don't need to exist at the same time, so it
may be better simply to use the names "actual" and "expect", like
this:
...other stuff...
cat >expect <<-\EOF &&
...
EOF
git ls-files -t --dedup >actual &&
test_cmp expect actual &&
rm delete.txt &&
cat >expect <<-\EOF &&
...
EOF
git ls-files -d -m -t --dedup >actual &&
test_cmp expect actual
(It also has the benefit that the "expect" content is closer to the
place where it is actually used, which may make it a bit easier for a
person reading the test to understand what is supposed to be
produced.)
From: Eric Sunshine <hidden> Date: 2021-01-17 05:12:58
On Sat, Jan 16, 2021 at 10:48 PM 胡哲宁 [off-list ref] wrote:
Eric Sunshine [off-list ref] 于2021年1月16日周六 下午3:13写道：
quoted
quoted
+ git switch -c dev &&
If someone adds a new test after this test, then that new test will
run in the "dev" branch, which might be unexpected or undesirable. It
often is a good idea to ensure that tests do certain types of cleanup
to avoid breaking subsequent tests. Here, it would be a good idea to
ensure that the test switches back to the original branch when it
finishes (regardless of whether it finishes successfully or
unsuccessfully).
git switch -c dev &&
test_when_finished "git switch master" &&
Or you could use `git switch -` if you don't want to hard-code the
name "master" in the test (since there has been effort lately to
remove that name from tests.
I have little confuse about I can use` test_when_finished "git switch master" `,
but I can't use` test_when_finished "git switch -" `,
why？
You may use either one. I presented both as alternative approaches.
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-01-17 04:03:19
From: ZheNing Hu <redacted>
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.
Signed-off-by: ZheNing Hu <redacted>
---
builtin/ls-files.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-01-17 04:03:20
From: ZheNing Hu <redacted>
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.
Signed-off-by: ZheNing Hu <redacted>
---
builtin/ls-files.c | 68 +++++++++++++++++++---------------------------
1 file changed, 28 insertions(+), 40 deletions(-)
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-01-17 04:03:20
From: ZheNing Hu <redacted>
In order to provide users a better experience
when viewing information about files in the index
and the working tree, the `--deduplicate` option will suppress
some duplicate name under some conditions.
In a merge conflict, one file name of "git ls-files" output may
appear multiple times. For example,now there is an unmerged path
`a.c`,`a.c` will appear three times in the output of
"git ls-files".We can use "git ls-files --deduplicate" to output
`a.c` only one time.(unless `--stage` or `--unmerged` is
used to view all the detailed information in the index)
In addition, if you use both `--delete` and `--modify` at
the same time, The `--deduplicate` option
can also suppress file name output.
Additional instructions:
In order to display entries information,`deduplicate` suppresses
the output of duplicate file names, not the output of duplicate
entries information, so under the option of `-t`, `--stage`, `--unmerge`,
`--deduplicate` will have no effect.
Signed-off-by: ZheNing Hu <redacted>
---
Documentation/git-ls-files.txt | 5 +++
builtin/ls-files.c | 23 +++++++++++++-
t/t3012-ls-files-dedup.sh | 57 ++++++++++++++++++++++++++++++++++
3 files changed, 84 insertions(+), 1 deletion(-)
create mode 100755 t/t3012-ls-files-dedup.sh
@@ -81,6 +82,10 @@ OPTIONS \0 line termination on output and do not quote filenames. See OUTPUT below for more information.+--deduplicate::+ Suppress duplicate entries when there are unmerged paths in index+ or `--deleted` and `--modified` are combined.+ -x <pattern>:: --exclude=<pattern>:: Skip untracked files matching pattern.
@@ -35,6 +35,7 @@ static int line_terminator = '\n';staticintdebug_mode;staticintshow_eol;staticintrecurse_submodules;+staticintskipping_duplicates;staticconstchar*prefix;staticintmax_prefix_len;
@@ -301,6 +302,7 @@ static void show_files(struct repository *repo, struct dir_struct *dir){inti;structstrbuffullname=STRBUF_INIT;+conststructcache_entry*last_shown_ce;/* For cached/deleted files we don't need to even do the readdir */if(show_others||show_killed){
@@ -569,6 +587,7 @@ 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_END()};
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-01-19 06:32:17
From: ZheNing Hu <redacted>
In order to provide users a better experience
when viewing information about files in the index
and the working tree, the `--deduplicate` option will suppress
some duplicate name under some conditions.
In a merge conflict, one file name of "git ls-files" output may
appear multiple times. For example,now there is an unmerged path
`a.c`,`a.c` will appear three times in the output of
"git ls-files".We can use "git ls-files --deduplicate" to output
`a.c` only one time.(unless `--stage` or `--unmerged` is
used to view all the detailed information in the index)
In addition, if you use both `--delete` and `--modify` at
the same time, The `--deduplicate` option
can also suppress file name output.
Additional instructions:
In order to display entries information,`deduplicate` suppresses
the output of duplicate file names, not the output of duplicate
entries information, so under the option of `-t`, `--stage`, `--unmerge`,
`--deduplicate` will have no effect.
Signed-off-by: ZheNing Hu <redacted>
---
Documentation/git-ls-files.txt | 5 +++
builtin/ls-files.c | 32 ++++++++++++++---
t/t3012-ls-files-dedup.sh | 66 ++++++++++++++++++++++++++++++++++
3 files changed, 98 insertions(+), 5 deletions(-)
create mode 100755 t/t3012-ls-files-dedup.sh
@@ -81,6 +82,10 @@ OPTIONS \0 line termination on output and do not quote filenames. See OUTPUT below for more information.+--deduplicate::+ Suppress duplicate entries when there are unmerged paths in index+ or `--deleted` and `--modified` are combined.+ -x <pattern>:: --exclude=<pattern>:: Skip untracked files matching pattern.
@@ -35,6 +35,7 @@ static int line_terminator = '\n';staticintdebug_mode;staticintshow_eol;staticintrecurse_submodules;+staticintskipping_duplicates;staticconstchar*prefix;staticintmax_prefix_len;
@@ -301,6 +302,7 @@ static void show_files(struct repository *repo, struct dir_struct *dir){inti;structstrbuffullname=STRBUF_INIT;+conststructcache_entry*last_shown_ce;/* For cached/deleted files we don't need to even do the readdir */if(show_others||show_killed){
@@ -571,6 +590,7 @@ 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_END()};
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-01-19 06:32:38
From: ZheNing Hu <redacted>
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.
Signed-off-by: ZheNing Hu <redacted>
---
builtin/ls-files.c | 70 +++++++++++++++++++---------------------------
1 file changed, 29 insertions(+), 41 deletions(-)
From: Junio C Hamano <hidden> Date: 2021-01-20 20:37:23
"ZheNing Hu via GitGitGadget" [off-list ref] writes:
From: ZheNing Hu <redacted>
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.
Signed-off-by: ZheNing Hu <redacted>
---
builtin/ls-files.c | 70 +++++++++++++++++++---------------------------
1 file changed, 29 insertions(+), 41 deletions(-)
This one needs a bit more work, but I like the basic structure of
the rewritten loop.
We would want to reduce the indentation level of the show_ce() by
consolidating the nested if/if to
if ((show_cached || show_stage) &&
(!show_unmerged || ce_stage(ce)))
show_ce(...);
Everything below from this point should be skipped (especially, the
call to lstat()) unless show_modified and/or show_deleted was asked
by the caller, i.e. we want to insert
if (!(show_deleted || show_modified))
continue;
here, before we call ce_skip_worktree(), I think.
Junio C Hamano [off-list ref] 于2021年1月21日周四 上午4:27写道：
"ZheNing Hu via GitGitGadget" [off-list ref] writes:
quoted
From: ZheNing Hu <redacted>
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.
Signed-off-by: ZheNing Hu <redacted>
---
builtin/ls-files.c | 70 +++++++++++++++++++---------------------------
1 file changed, 29 insertions(+), 41 deletions(-)
This one needs a bit more work, but I like the basic structure of
the rewritten loop.
We would want to reduce the indentation level of the show_ce() by
consolidating the nested if/if to
if ((show_cached || show_stage) &&
(!show_unmerged || ce_stage(ce)))
show_ce(...);
The reason for this may be I gave
"if(show_cached || show_stage)" in 3/3
Added some logic.
Everything below from this point should be skipped (especially, the
call to lstat()) unless show_modified and/or show_deleted was asked
by the caller, i.e. we want to insert
if (!(show_deleted || show_modified))
continue;
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-01-19 06:32:53
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 | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
From: Junio C Hamano <hidden> Date: 2021-01-20 20:37:35
"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>
---
Thanks. A few comments:
* The error_errno() line is not indented correctly; I'll fix it up
while queuing, but it would conflict with 2/3 as you'll be moving
that line around.
* When we say "error", we do not even know if the thing got removed
or modified at all, so it is somewhat strange to report it as
such (the path may be intact and the only issue may be that we
cannot read the containing directory). It is equally strange not
to say anything on the path, and between the two, there isn't
clearly a more correct answer. What you implemented here does
not change the traditional behaviour of reporting it as
deleted/modified to "alert" the user, which I think is good.
* The logic for modified entry looks a bit duplicated. I wonder if
the one at the end of this message reads better. Renaming err to
stat_err is optional, but I think the name makes it clear why it
is sensible that these two places use the variable as a sign that
the path was deleted and/or modified.
Junio C Hamano [off-list ref] 于2021年1月21日周四 上午4:26写道：
"ZheNing Hu via GitGitGadget" [off-list ref] writes:
quoted
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>
---
Thanks. A few comments:
* The error_errno() line is not indented correctly; I'll fix it up
while queuing, but it would conflict with 2/3 as you'll be moving
that line around.
I might not have noticed,very Sorry.
* When we say "error", we do not even know if the thing got removed
or modified at all, so it is somewhat strange to report it as
such (the path may be intact and the only issue may be that we
cannot read the containing directory). It is equally strange not
to say anything on the path, and between the two, there isn't
clearly a more correct answer. What you implemented here does
not change the traditional behaviour of reporting it as
deleted/modified to "alert" the user, which I think is good.
Haha,thanks!
* The logic for modified entry looks a bit duplicated. I wonder if
the one at the end of this message reads better. Renaming err to
stat_err is optional, but I think the name makes it clear why it
is sensible that these two places use the variable as a sign that
the path was deleted and/or modified.
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-01-23 10:21:17
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(-)
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-01-23 10:21:17
From: ZheNing Hu <redacted>
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.
Signed-off-by: ZheNing Hu <redacted>
---
builtin/ls-files.c | 70 ++++++++++++++++++++--------------------------
1 file changed, 30 insertions(+), 40 deletions(-)
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-01-23 10:21:17
From: ZheNing Hu <redacted>
In order to provide users a better experience
when viewing information about files in the index
and the working tree, the `--deduplicate` option will suppress
some duplicate name under some conditions.
In a merge conflict, one file name of "git ls-files" output may
appear multiple times. For example,now there is an unmerged path
`a.c`,`a.c` will appear three times in the output of
"git ls-files".We can use "git ls-files --deduplicate" to output
`a.c` only one time.(unless `--stage` or `--unmerged` is
used to view all the detailed information in the index)
In addition, if you use both `--delete` and `--modify` at
the same time, The `--deduplicate` option
can also suppress file name output.
Additional instructions:
In order to display entries information,`deduplicate` suppresses
the output of duplicate file names, not the output of duplicate
entries information, so under the option of `-t`, `--stage`, `--unmerge`,
`--deduplicate` will have no effect.
Signed-off-by: ZheNing Hu <redacted>
---
Documentation/git-ls-files.txt | 5 +++
builtin/ls-files.c | 30 +++++++++++++---
t/t3012-ls-files-dedup.sh | 66 ++++++++++++++++++++++++++++++++++
3 files changed, 97 insertions(+), 4 deletions(-)
create mode 100755 t/t3012-ls-files-dedup.sh
@@ -81,6 +82,10 @@ OPTIONS \0 line termination on output and do not quote filenames. See OUTPUT below for more information.+--deduplicate::+ Suppress duplicate entries when there are unmerged paths in index+ or `--deleted` and `--modified` are combined.+ -x <pattern>:: --exclude=<pattern>:: Skip untracked files matching pattern.
@@ -340,11 +343,27 @@ static void show_files(struct repository *repo, struct dir_struct *dir)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)+if(stat_err&&show_deleted){show_ce(repo,dir,ce,fullname.buf,tag_removed);+if(skipping_duplicates)+gotoskip_to_next_name;+}if(show_modified&&-(stat_err||ie_modified(repo->index,ce,&st,0)))+(stat_err||ie_modified(repo->index,ce,&st,0))){show_ce(repo,dir,ce,fullname.buf,tag_modified);+if(skipping_duplicates)+gotoskip_to_next_name;+}+continue;+skip_to_next_name:+{+intj;+structcache_entry**cache=repo->index->cache;+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 */+}}strbuf_release(&fullname);
@@ -571,6 +590,7 @@ 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_END()};
From: Junio C Hamano <hidden> Date: 2021-01-23 19:54:36
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>
[jc: fixed misindented code]
Signed-off-by: Junio C Hamano <redacted>
---
builtin/ls-files.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
From: Junio C Hamano <hidden> Date: 2021-01-23 19:54:36
From: ZheNing Hu <redacted>
This will make it easier to show only one entry per filename in the
next step.
Signed-off-by: ZheNing Hu <redacted>
[jc: corrected the log message]
Signed-off-by: Junio C Hamano <redacted>
---
builtin/ls-files.c | 63 ++++++++++++++++++++--------------------------
1 file changed, 27 insertions(+), 36 deletions(-)
From: Junio C Hamano <hidden> Date: 2021-01-23 19:54:36
From: ZheNing Hu <redacted>
During a merge conflict, the name of a file may appear multiple
times in "git ls-files" output, once for each stage. If you use
both `--delete` and `--modify` at the same time, the output may
mention a deleted file twice.
When none of the '-t', '-u', or '-s' options is in use, these
duplicate entries do not add much value to the output.
Introduce a new '--deduplicate' option to suppress them.
Signed-off-by: ZheNing Hu <redacted>
[jc: extended doc and rewritten commit log]
Signed-off-by: Junio C Hamano <redacted>
---
Documentation/git-ls-files.txt | 8 +++++
builtin/ls-files.c | 31 ++++++++++++++--
t/t3012-ls-files-dedup.sh | 66 ++++++++++++++++++++++++++++++++++
3 files changed, 102 insertions(+), 3 deletions(-)
create mode 100755 t/t3012-ls-files-dedup.sh
@@ -80,6 +81,13 @@ OPTIONS \0 line termination on output and do not quote filenames. See OUTPUT below for more information.+--deduplicate::+ When only filenames are shown, suppress duplicates that may+ come from having multiple stages during a merge, or giving+ `--deleted` and `--modified` option at the same time.+ When any of the `-t`, `--unmerged`, or `--stage` option is+ in use, this option has no effect.+ -x <pattern>:: --exclude=<pattern>:: Skip untracked files matching pattern.
@@ -341,11 +345,28 @@ static void show_files(struct repository *repo, struct dir_struct *dir)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)+if(stat_err&&show_deleted){show_ce(repo,dir,ce,fullname.buf,tag_removed);+if(skipping_duplicates)+gotoskip_to_next_name;+}if(show_modified&&-(stat_err||ie_modified(repo->index,ce,&st,0)))+(stat_err||ie_modified(repo->index,ce,&st,0))){show_ce(repo,dir,ce,fullname.buf,tag_modified);+if(skipping_duplicates)+gotoskip_to_next_name;+}+continue;++skip_to_next_name:+{+intj;+structcache_entry**cache=repo->index->cache;+for(j=i+1;j<repo->index->cache_nr;j++)+if(strcmp(ce->name,cache[j]->name))+break;+i=j-1;/* compensate for the for loop */+}}strbuf_release(&fullname);
@@ -572,6 +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_END()};
From: 阿德烈 via GitGitGadget <hidden> Date: 2021-01-24 10:55:36
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 | 8 ++++
builtin/ls-files.c | 85 ++++++++++++++++++++--------------
t/t3012-ls-files-dedup.sh | 66 ++++++++++++++++++++++++++
3 files changed, 124 insertions(+), 35 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-v7
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-832/adlternative/ls-files-dedup-v7
Pull-Request: https://github.com/gitgitgadget/git/pull/832
Range-diff vs v6:
1: fbc38ce9075 ! 1: 8b02367a359 ls_files.c: bugfix for --deleted and --modified
@@ Commit message
ie_modified() when lstat() has failed.
Signed-off-by: ZheNing Hu [off-list ref]
+ [jc: fixed misindented code]
+ Signed-off-by: Junio C Hamano [off-list ref]
## builtin/ls-files.c ##
@@ builtin/ls-files.c: static void show_files(struct repository *repo, struct dir_struct *dir)
2: 3997d390883 ! 2: f9d5e44d2c0 ls_files.c: consolidate two for loops into one
@@ Metadata
## Commit message ##
ls_files.c: consolidate two for loops into one
- 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 will make it easier to show only one entry per filename in the
+ next step.
Signed-off-by: ZheNing Hu [off-list ref]
+ [jc: corrected the log message]
+ Signed-off-by: Junio C Hamano [off-list ref]
## builtin/ls-files.c ##
@@ builtin/ls-files.c: static void show_files(struct repository *repo, struct dir_struct *dir)
3: 07b603fd97c ! 3: 384f77a4c18 ls-files.c: add --deduplicate option
@@ Metadata
## Commit message ##
ls-files.c: add --deduplicate option
- In order to provide users a better experience
- when viewing information about files in the index
- and the working tree, the `--deduplicate` option will suppress
- some duplicate name under some conditions.
+ During a merge conflict, the name of a file may appear multiple
+ times in "git ls-files" output, once for each stage. If you use
+ both `--delete` and `--modify` at the same time, the output may
+ mention a deleted file twice.
- In a merge conflict, one file name of "git ls-files" output may
- appear multiple times. For example,now there is an unmerged path
- `a.c`,`a.c` will appear three times in the output of
- "git ls-files".We can use "git ls-files --deduplicate" to output
- `a.c` only one time.(unless `--stage` or `--unmerged` is
- used to view all the detailed information in the index)
+ When none of the '-t', '-u', or '-s' options is in use, these
+ duplicate entries do not add much value to the output.
- In addition, if you use both `--delete` and `--modify` at
- the same time, The `--deduplicate` option
- can also suppress file name output.
-
- Additional instructions:
- In order to display entries information,`deduplicate` suppresses
- the output of duplicate file names, not the output of duplicate
- entries information, so under the option of `-t`, `--stage`, `--unmerge`,
- `--deduplicate` will have no effect.
+ Introduce a new '--deduplicate' option to suppress them.
Signed-off-by: ZheNing Hu [off-list ref]
+ [jc: extended doc and rewritten commit log]
+ Signed-off-by: Junio C Hamano [off-list ref]
## Documentation/git-ls-files.txt ##
@@ Documentation/git-ls-files.txt: SYNOPSIS
@@ Documentation/git-ls-files.txt: OPTIONS
See OUTPUT below for more information.
+--deduplicate::
-+ Suppress duplicate entries when there are unmerged paths in index
-+ or `--deleted` and `--modified` are combined.
++ When only filenames are shown, suppress duplicates that may
++ come from having multiple stages during a merge, or giving
++ `--deleted` and `--modified` option at the same time.
++ When any of the `-t`, `--unmerged`, or `--stage` option is
++ in use, this option has no effect.
+
-x <pattern>::
--exclude=<pattern>::
--
gitgitgadget
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-01-24 10:55:36
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>
[jc: fixed misindented code]
Signed-off-by: Junio C Hamano <redacted>
---
builtin/ls-files.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-01-24 10:55:36
From: ZheNing Hu <redacted>
This will make it easier to show only one entry per filename in the
next step.
Signed-off-by: ZheNing Hu <redacted>
[jc: corrected the log message]
Signed-off-by: Junio C Hamano <redacted>
---
builtin/ls-files.c | 70 ++++++++++++++++++++--------------------------
1 file changed, 30 insertions(+), 40 deletions(-)
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-01-24 10:55:38
From: ZheNing Hu <redacted>
During a merge conflict, the name of a file may appear multiple
times in "git ls-files" output, once for each stage. If you use
both `--delete` and `--modify` at the same time, the output may
mention a deleted file twice.
When none of the '-t', '-u', or '-s' options is in use, these
duplicate entries do not add much value to the output.
Introduce a new '--deduplicate' option to suppress them.
Signed-off-by: ZheNing Hu <redacted>
[jc: extended doc and rewritten commit log]
Signed-off-by: Junio C Hamano <redacted>
---
Documentation/git-ls-files.txt | 8 +++++
builtin/ls-files.c | 30 +++++++++++++---
t/t3012-ls-files-dedup.sh | 66 ++++++++++++++++++++++++++++++++++
3 files changed, 100 insertions(+), 4 deletions(-)
create mode 100755 t/t3012-ls-files-dedup.sh
@@ -81,6 +82,13 @@ OPTIONS \0 line termination on output and do not quote filenames. See OUTPUT below for more information.+--deduplicate::+ When only filenames are shown, suppress duplicates that may+ come from having multiple stages during a merge, or giving+ `--deleted` and `--modified` option at the same time.+ When any of the `-t`, `--unmerged`, or `--stage` option is+ in use, this option has no effect.+ -x <pattern>:: --exclude=<pattern>:: Skip untracked files matching pattern.
@@ -340,11 +343,27 @@ static void show_files(struct repository *repo, struct dir_struct *dir)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)+if(stat_err&&show_deleted){show_ce(repo,dir,ce,fullname.buf,tag_removed);+if(skipping_duplicates)+gotoskip_to_next_name;+}if(show_modified&&-(stat_err||ie_modified(repo->index,ce,&st,0)))+(stat_err||ie_modified(repo->index,ce,&st,0))){show_ce(repo,dir,ce,fullname.buf,tag_modified);+if(skipping_duplicates)+gotoskip_to_next_name;+}+continue;+skip_to_next_name:+{+intj;+structcache_entry**cache=repo->index->cache;+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 */+}}strbuf_release(&fullname);
@@ -571,6 +590,7 @@ 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_END()};