| Message ID | 81ae1280e8eb471c7a11dceb0aa7a8915948b2ce.1655300752.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ls-files: introduce "--format" and "--object-only" options | expand |
On Wed, Jun 15 2022, ZheNing Hu via GitGitGadget wrote: > From: ZheNing Hu <adlternative@gmail.com> > > --object-only is an alias for --format=%(objectname), > which output objectname of index entries, taking > inspiration from the option with the same name in > the `git ls-tree` command. > > --object-only cannot be used with --format, and -s, -o, > -k, --resolve-undo, --deduplicate, --debug. > > Signed-off-by: ZheNing Hu <adlternative@gmail.com> > --- > Documentation/git-ls-files.txt | 8 +++++++- > builtin/ls-files.c | 36 +++++++++++++++++++++++++++++++++- > t/t3013-ls-files-format.sh | 34 ++++++++++++++++++++++++++++++++ > 3 files changed, 76 insertions(+), 2 deletions(-) > > diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt > index b22860ec8c0..c3f46bb821b 100644 > --- a/Documentation/git-ls-files.txt > +++ b/Documentation/git-ls-files.txt > @@ -13,7 +13,7 @@ SYNOPSIS > [-c|--cached] [-d|--deleted] [-o|--others] [-i|--|ignored] > [-s|--stage] [-u|--unmerged] [-k|--|killed] [-m|--modified] > [--directory [--no-empty-directory]] [--eol] > - [--deduplicate] > + [--deduplicate] [--object-only] > [-x <pattern>|--exclude=<pattern>] > [-X <file>|--exclude-from=<file>] > [--exclude-per-directory=<file>] > @@ -199,6 +199,12 @@ followed by the ("attr/<eolattr>"). > interpolates to `\0` (NUL), `%09` to `\t` (TAB) and %0a to `\n` (LF). > --format cannot be combined with `-s`, `-o`, `-k`, `--resolve-undo`, > `--debug`. > + > +--object-only:: > + List only names of the objects, one per line. This is equivalent > + to specifying `--format='%(objectname)'`. Cannot be combined with > + `--format=<format>`. > + > \--:: > Do not interpret any more arguments as options. > > diff --git a/builtin/ls-files.c b/builtin/ls-files.c > index 9dd6c55eeb9..4ac8f34baac 100644 > --- a/builtin/ls-files.c > +++ b/builtin/ls-files.c > @@ -60,6 +60,27 @@ static const char *tag_modified = ""; > static const char *tag_skip_worktree = ""; > static const char *tag_resolve_undo = ""; > > +static enum ls_files_cmdmode { > + MODE_DEFAULT = 0, > + MODE_OBJECT_ONLY, > +} ls_files_cmdmode; > + > +struct ls_files_cmdmodee_to_fmt { > + enum ls_files_cmdmode mode; > + const char *const fmt; > +}; > + > +static struct ls_files_cmdmodee_to_fmt ls_files_cmdmode_format[] = { > + { > + .mode = MODE_DEFAULT, > + .fmt = NULL, > + }, > + { > + .mode = MODE_OBJECT_ONLY, > + .fmt = "%(objectname)", > + }, > +}; [...snip...] This code all looks OK from skimming it, and is substantially copied from builtin/ls-tree.c (which is good). But I wonder as in that case whether having such an alias is worth it at all, especially since in the case of ls-files (unlike ls-tree) we don't start out with various --just-the-X-field type options, this is the first one. So I *really* like that you took my suggestion of "why not a --format" from a previous round, but given the above for ls-files in particular is it really worth it to have this extra code just to type: --object-only Instead of: --format="%(objectname)" So, maybe, and I'm not set against it, but I think it's worth re-evaluating in this case. In particular because the part of ls-tree's code is missing here where we "format optimize", i.e. we take a form like: --format="%(objectname)" And dispatch it to the more optimized special function, instead of the generic strbuf_expand(), whereas in this case it's the other way around, the option is just an alias for --format.
Ævar Arnfjörð Bjarmason <avarab@gmail.com> 于2022年6月16日周四 04:25写道： > > > On Wed, Jun 15 2022, ZheNing Hu via GitGitGadget wrote: > > > From: ZheNing Hu <adlternative@gmail.com> > > > > --object-only is an alias for --format=%(objectname), > > which output objectname of index entries, taking > > inspiration from the option with the same name in > > the `git ls-tree` command. > > > > --object-only cannot be used with --format, and -s, -o, > > -k, --resolve-undo, --deduplicate, --debug. > > > > Signed-off-by: ZheNing Hu <adlternative@gmail.com> > > --- > > Documentation/git-ls-files.txt | 8 +++++++- > > builtin/ls-files.c | 36 +++++++++++++++++++++++++++++++++- > > t/t3013-ls-files-format.sh | 34 ++++++++++++++++++++++++++++++++ > > 3 files changed, 76 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt > > index b22860ec8c0..c3f46bb821b 100644 > > --- a/Documentation/git-ls-files.txt > > +++ b/Documentation/git-ls-files.txt > > @@ -13,7 +13,7 @@ SYNOPSIS > > [-c|--cached] [-d|--deleted] [-o|--others] [-i|--|ignored] > > [-s|--stage] [-u|--unmerged] [-k|--|killed] [-m|--modified] > > [--directory [--no-empty-directory]] [--eol] > > - [--deduplicate] > > + [--deduplicate] [--object-only] > > [-x <pattern>|--exclude=<pattern>] > > [-X <file>|--exclude-from=<file>] > > [--exclude-per-directory=<file>] > > @@ -199,6 +199,12 @@ followed by the ("attr/<eolattr>"). > > interpolates to `\0` (NUL), `%09` to `\t` (TAB) and %0a to `\n` (LF). > > --format cannot be combined with `-s`, `-o`, `-k`, `--resolve-undo`, > > `--debug`. > > + > > +--object-only:: > > + List only names of the objects, one per line. This is equivalent > > + to specifying `--format='%(objectname)'`. Cannot be combined with > > + `--format=<format>`. > > + > > \--:: > > Do not interpret any more arguments as options. > > > > diff --git a/builtin/ls-files.c b/builtin/ls-files.c > > index 9dd6c55eeb9..4ac8f34baac 100644 > > --- a/builtin/ls-files.c > > +++ b/builtin/ls-files.c > > @@ -60,6 +60,27 @@ static const char *tag_modified = ""; > > static const char *tag_skip_worktree = ""; > > static const char *tag_resolve_undo = ""; > > > > +static enum ls_files_cmdmode { > > + MODE_DEFAULT = 0, > > + MODE_OBJECT_ONLY, > > +} ls_files_cmdmode; > > + > > +struct ls_files_cmdmodee_to_fmt { > > + enum ls_files_cmdmode mode; > > + const char *const fmt; > > +}; > > + > > +static struct ls_files_cmdmodee_to_fmt ls_files_cmdmode_format[] = { > > + { > > + .mode = MODE_DEFAULT, > > + .fmt = NULL, > > + }, > > + { > > + .mode = MODE_OBJECT_ONLY, > > + .fmt = "%(objectname)", > > + }, > > +}; > [...snip...] > > This code all looks OK from skimming it, and is substantially copied > from builtin/ls-tree.c (which is good). > > But I wonder as in that case whether having such an alias is worth it at > all, especially since in the case of ls-files (unlike ls-tree) we don't > start out with various --just-the-X-field type options, this is the > first one. > > So I *really* like that you took my suggestion of "why not a --format" > from a previous round, but given the above for ls-files in particular is > it really worth it to have this extra code just to type: > > --object-only > > Instead of: > > --format="%(objectname)" > > So, maybe, and I'm not set against it, but I think it's worth > re-evaluating in this case. > > In particular because the part of ls-tree's code is missing here where > we "format optimize", i.e. we take a form like: > > --format="%(objectname)" > > And dispatch it to the more optimized special function, instead of the > generic strbuf_expand(), whereas in this case it's the other way around, > the option is just an alias for --format. Thanks for clarifying that --object-only uses the fast path instead of a simple alias of --format=%(objectname). Maybe I should do this too, or just drop this patch because git ls-file --format has included such a function :-) ZheNing Hu
diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt index b22860ec8c0..c3f46bb821b 100644 --- a/Documentation/git-ls-files.txt +++ b/Documentation/git-ls-files.txt @@ -13,7 +13,7 @@ SYNOPSIS [-c|--cached] [-d|--deleted] [-o|--others] [-i|--|ignored] [-s|--stage] [-u|--unmerged] [-k|--|killed] [-m|--modified] [--directory [--no-empty-directory]] [--eol] - [--deduplicate] + [--deduplicate] [--object-only] [-x <pattern>|--exclude=<pattern>] [-X <file>|--exclude-from=<file>] [--exclude-per-directory=<file>] @@ -199,6 +199,12 @@ followed by the ("attr/<eolattr>"). interpolates to `\0` (NUL), `%09` to `\t` (TAB) and %0a to `\n` (LF). --format cannot be combined with `-s`, `-o`, `-k`, `--resolve-undo`, `--debug`. + +--object-only:: + List only names of the objects, one per line. This is equivalent + to specifying `--format='%(objectname)'`. Cannot be combined with + `--format=<format>`. + \--:: Do not interpret any more arguments as options. diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 9dd6c55eeb9..4ac8f34baac 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -60,6 +60,27 @@ static const char *tag_modified = ""; static const char *tag_skip_worktree = ""; static const char *tag_resolve_undo = ""; +static enum ls_files_cmdmode { + MODE_DEFAULT = 0, + MODE_OBJECT_ONLY, +} ls_files_cmdmode; + +struct ls_files_cmdmodee_to_fmt { + enum ls_files_cmdmode mode; + const char *const fmt; +}; + +static struct ls_files_cmdmodee_to_fmt ls_files_cmdmode_format[] = { + { + .mode = MODE_DEFAULT, + .fmt = NULL, + }, + { + .mode = MODE_OBJECT_ONLY, + .fmt = "%(objectname)", + }, +}; + static void write_eolinfo_internal(struct strbuf *sb, struct index_state *istate, const struct cache_entry *ce, const char *path) { @@ -747,6 +768,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) DIR_SHOW_IGNORED), OPT_BOOL('s', "stage", &show_stage, N_("show staged contents' object name in the output")), + OPT_CMDMODE(0, "object-only", &ls_files_cmdmode, N_("list only objects"), + MODE_OBJECT_ONLY), OPT_BOOL('k', "killed", &show_killed, N_("show files on the filesystem that need to be removed")), OPT_BIT(0, "directory", &dir.flags, @@ -815,9 +838,20 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) add_pattern(exclude_list.items[i].string, "", 0, pl, --exclude_args); } + if (format && ls_files_cmdmode) + die(_("--format can't be combined with other format-altering options")); + + for (i = 0; !format && i < ARRAY_SIZE(ls_files_cmdmode_format); i++) { + if (ls_files_cmdmode == ls_files_cmdmode_format[i].mode) { + format = ls_files_cmdmode_format[i].fmt; + break; + } + } + if (format && (show_stage || show_others || show_killed || show_resolve_undo || skipping_duplicates || debug_mode)) - die(_("ls-files --format cannot used with -s, -o, -k, --resolve-undo, --deduplicate, --debug")); + die(_("ls-files --format or other format-altering options " + "cannot used with -s, -o, -k, --resolve-undo, --deduplicate, --debug")); if (show_tag || show_valid_bit || show_fsmonitor_bit) { tag_cached = "H "; diff --git a/t/t3013-ls-files-format.sh b/t/t3013-ls-files-format.sh index 61a2e68713a..1c982ea13e0 100755 --- a/t/t3013-ls-files-format.sh +++ b/t/t3013-ls-files-format.sh @@ -139,4 +139,38 @@ test_expect_success 'git ls-files --format with --debug must fail' ' test_must_fail git ls-files --format="%(objectname)" --debug ' +test_expect_success 'git ls-files --object-only equal to --format=%(objectname)' ' + git ls-files --format="%(objectname)" >expect && + git ls-files --object-only >actual && + test_cmp expect actual +' + +test_expect_success 'git ls-files --object-only with --format must fail' ' + test_must_fail git ls-files --format="%(path)" --object-only +' + +test_expect_success 'git ls-files --object-only with -s must fail' ' + test_must_fail git ls-files --object-only -s +' + +test_expect_success 'git ls-files --object-only with -o must fail' ' + test_must_fail git ls-files --object-only -o +' + +test_expect_success 'git ls-files --object-only with -k must fail' ' + test_must_fail git ls-files --object-only -k +' + +test_expect_success 'git ls-files --object-only with --resolve-undo must fail' ' + test_must_fail git ls-files --object-only --resolve-undo +' + +test_expect_success 'git ls-files --object-only with --deduplicate must fail' ' + test_must_fail git ls-files --object-only --deduplicate +' + +test_expect_success 'git ls-files --object-only with --debug must fail' ' + test_must_fail git ls-files --object-only --debug +' + test_done