diff mbox series

[v5] ls-files: introduce "--format" option

Message ID pull.1262.v5.git.1657002760815.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series [v5] ls-files: introduce "--format" option | expand

Commit Message

ZheNing Hu July 5, 2022, 6:32 a.m. UTC
From: ZheNing Hu <adlternative@gmail.com>

Add a new option --format that output index enties
informations with custom format, taking inspiration
from the option with the same name in the `git ls-tree`
command.

--format cannot used with -s, -o, -k, -t, --resolve-undo,
--deduplicate and --eol.

Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
    ls-files: introduce "--format" options
    
    v4->v5:
    
     1. Let --format incompatible with -t.
     2. Fix %(eolinfo) and %(eolattr) docs suggested by Junio.
    
    Looking forward to Ævar's reviewing.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1262%2Fadlternative%2Fzh%2Fls-file-format-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1262/adlternative/zh/ls-file-format-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/1262

Range-diff vs v4:

 1:  6827e44e158 ! 1:  1ce69d6202a ls-files: introduce "--format" option
     @@ Commit message
          from the option with the same name in the `git ls-tree`
          command.
      
     -    --format cannot used with -s, -o, -k, --resolve-undo,
     +    --format cannot used with -s, -o, -k, -t, --resolve-undo,
          --deduplicate and --eol.
      
          Signed-off-by: ZheNing Hu <adlternative@gmail.com>
     @@ Documentation/git-ls-files.txt: followed by the  ("attr/<eolattr>").
      +	It also interpolates `%%` to `%`, and `%xx` where `xx` are hex digits
      +	interpolates to character with hex code `xx`; for example `%00`
      +	interpolates to `\0` (NUL), `%09` to `\t` (TAB) and %0a to `\n` (LF).
     -+	--format cannot be combined with `-s`, `-o`, `-k`, `--resolve-undo`
     ++	--format cannot be combined with `-s`, `-o`, `-k`, `-t`, `--resolve-undo`
      +	and `--eol`.
       \--::
       	Do not interpret any more arguments as options.
     @@ Documentation/git-ls-files.txt: quoted as explained for the configuration variab
      +stage::
      +	The stage of the file which is recorded in the index.
      +eolinfo:index::
     -+	The <eolinfo> of the file which is recorded in the index.
      +eolinfo:worktree::
     -+	The <eolinfo> of the file which is recorded in the working tree.
     ++	The <eolinfo> (see the description of the `--eol` option) of
     ++	the contents in the index or in the worktree for the path.
      +eolattr::
     -+	The <eolattr> of the file which is recorded in the index.
     ++	The <eolattr> (see the description of the `--eol` option)
     ++	that applies to the path.
      +path::
      +	The pathname of the file which is recorded in the index.
       
     @@ builtin/ls-files.c: int cmd_ls_files(int argc, const char **argv, const char *cm
       	}
      +
      +	if (format && (show_stage || show_others || show_killed ||
     -+		show_resolve_undo || skipping_duplicates || show_eol))
     -+			usage_msg_opt("--format cannot used with -s, -o, -k, "
     ++		show_resolve_undo || skipping_duplicates || show_eol || show_tag))
     ++			usage_msg_opt("--format cannot used with -s, -o, -k, -t"
      +				      "--resolve-undo, --deduplicate, --eol",
      +				      ls_files_usage, builtin_ls_files_options);
      +
     @@ t/t3013-ls-files-format.sh (new)
      +TEST_PASSES_SANITIZE_LEAK=true
      +. ./test-lib.sh
      +
     -+for flag in -s -o -k --resolve-undo --deduplicate --eol
     ++for flag in -s -o -k -t --resolve-undo --deduplicate --eol
      +do
      +	test_expect_success "usage: --format is incompatible with $flag" '
      +		test_expect_code 129 git ls-files --format="%(objectname)" $flag
     @@ t/t3013-ls-files-format.sh (new)
      +	100755
      +	100644
      +	EOF
     -+	git ls-files --format="%(objectmode)" -t >actual &&
     ++	git ls-files --format="%(objectmode)" >actual &&
      +	test_cmp expect actual
      +'
      +


 Documentation/git-ls-files.txt |  38 ++++++++++-
 builtin/ls-files.c             | 113 +++++++++++++++++++++++++++++++++
 t/t3013-ls-files-format.sh     | 108 +++++++++++++++++++++++++++++++
 3 files changed, 258 insertions(+), 1 deletion(-)
 create mode 100755 t/t3013-ls-files-format.sh


base-commit: ab336e8f1c8009c8b1aab8deb592148e69217085

Comments

Ævar Arnfjörð Bjarmason July 5, 2022, 8:39 a.m. UTC | #1
On Tue, Jul 05 2022, ZheNing Hu via GitGitGadget wrote:

> From: ZheNing Hu <adlternative@gmail.com>
>
> Add a new option --format that output index enties
> informations with custom format, taking inspiration
> from the option with the same name in the `git ls-tree`
> command.
>
> --format cannot used with -s, -o, -k, -t, --resolve-undo,
> --deduplicate and --eol.
>
> Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> ---
>     ls-files: introduce "--format" options
>     
>     v4->v5:
>     
>      1. Let --format incompatible with -t.
>      2. Fix %(eolinfo) and %(eolattr) docs suggested by Junio.
>     
>     Looking forward to Ævar's reviewing.

Thanks again, I took a look at this and it looks good to me as-is.

If you do want to further twiddle with it at this point I applied these
changes to it locally while poking around, changes:

 * Some trivial whitespace between variable decl.

 * Removed a "return;" at the end of a function

 * I found the new write_*() helpers to be uneccesary, what I did spot
   after seeing if they could be factored out is the existing
   write_eolinfo() function.

   I see you just copied some of the code from there, but
   e.g. initializing to "" and doing an unconditional strbuf_addstr()
   looks odd IMO compared to just doing it inline as below.

   I think if helpers are to be introduced here I'd think it would make
   more sense to split out the small bits of behavior from
   write_eolinfo() so you can call it picemeal and share the code, but
   since it's calling trivial external functions I think just calling
   those directly probably makes more sense...

 * Likewise for the test I wondered if you were adding a bug by not
   reporting when lstat() failed, but then found that this is the same
   thing we do on --eol.

   So for the tests I think it's better just to demonstrate that we can
   emit the exact same thing that --eol does with --format.

 * We've gone back & forth a bit on whether this would combine with
   --debug, while it's an internal-only feature it would be nice to have a
   test for it combined with --format, noting that the behavior might
   change...

 * There is one subtle behavior change here in that I deleted the "ce
   &&" part from write_index_eolinfo_to_buf() when moving the code
   over. I'm 99% sure this is the right thing to do, as other code in
   expand_show_index() unconditionally dereferences it.

   So perhaps we don't need that guard in write_eolinfo() either? In any
   case copy/pasting it over when we're already assuming a non-NULL "ce"
   in the same "if/elseif/else" chain looks a bit odd.

   Ah, I see it's because in show_dir_entry() we explicitly pass it as
   NULL, but that doesn't apply to our new codepath, so as long as we're
   not sharing that helper with write_eolinfo() it makes sense to not do
   that check.

   Even then the helper should probably assume "ce", and write_eolinfo()
   itself should do the "is ce NULL?" check which is specific to its
   use-case.

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 79ecdce2c9c..cc3cece3830 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -77,30 +77,6 @@ static void write_eolinfo(struct index_state *istate,
 	}
 }
 
-static void write_index_eolinfo_to_buf(struct strbuf *sb, struct index_state *istate,
-				       const struct cache_entry *ce)
-{
-	const char *i_txt = "";
-	if (ce && S_ISREG(ce->ce_mode))
-		i_txt = get_cached_convert_stats_ascii(istate, ce->name);
-	strbuf_addstr(sb, i_txt);
-}
-
-static void write_worktree_eolinfo_to_buf(struct strbuf *sb, const char *path)
-{
-	struct stat st;
-	const char *w_txt = "";
-	if (!lstat(path, &st) && S_ISREG(st.st_mode))
-		w_txt = get_wt_convert_stats_ascii(path);
-	strbuf_addstr(sb, w_txt);
-}
-
-static void write_eolattr_to_buf(struct strbuf *sb, struct index_state *istate,
-				 const char *path)
-{
-	strbuf_addstr(sb, get_convert_attr_ascii(istate, path));
-}
-
 static void write_name(const char *name)
 {
 	/*
@@ -114,6 +90,7 @@ static void write_name(const char *name)
 static void write_name_to_buf(struct strbuf *sb, const char *name)
 {
 	const char *rel = relative_path(name, prefix_len ? prefix : NULL, sb);
+
 	if (line_terminator)
 		quote_c_style(rel, sb, NULL, 0);
 	else
@@ -270,6 +247,8 @@ static size_t expand_show_index(struct strbuf *sb, const char *start,
 	const char *end;
 	const char *p;
 	size_t len = strbuf_expand_literal_cb(sb, start, NULL);
+	struct stat st;
+
 	if (len)
 		return len;
 	if (*start != '(')
@@ -288,12 +267,16 @@ static size_t expand_show_index(struct strbuf *sb, const char *start,
 		strbuf_add_unique_abbrev(sb, &data->ce->oid, abbrev);
 	else if (skip_prefix(start, "(stage)", &p))
 		strbuf_addf(sb, "%d", ce_stage(data->ce));
-	else if (skip_prefix(start, "(eolinfo:index)", &p))
-		write_index_eolinfo_to_buf(sb, data->istate, data->ce);
-	else if (skip_prefix(start, "(eolinfo:worktree)", &p))
-		write_worktree_eolinfo_to_buf(sb, data->pathname);
+	else if (skip_prefix(start, "(eolinfo:index)", &p) &&
+		 S_ISREG(data->ce->ce_mode))
+		strbuf_addstr(sb, get_cached_convert_stats_ascii(data->istate,
+								 data->ce->name));
+	else if (skip_prefix(start, "(eolinfo:worktree)", &p) &&
+		 !lstat(data->pathname, &st) && S_ISREG(st.st_mode))
+		strbuf_addstr(sb, get_wt_convert_stats_ascii(data->pathname));
 	else if (skip_prefix(start, "(eolattr)", &p))
-		write_eolattr_to_buf(sb, data->istate, data->pathname);
+		strbuf_addstr(sb, get_convert_attr_ascii(data->istate,
+							 data->pathname));
 	else if (skip_prefix(start, "(path)", &p))
 		write_name_to_buf(sb, data->pathname);
 	else
@@ -310,13 +293,12 @@ static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,
 		.istate = repo->index,
 		.ce = ce,
 	};
-
 	struct strbuf sb = STRBUF_INIT;
+
 	strbuf_expand(&sb, format, expand_show_index, &data);
 	strbuf_addch(&sb, line_terminator);
 	fwrite(sb.buf, sb.len, 1, stdout);
 	strbuf_release(&sb);
-	return;
 }
 
 static void show_ce(struct repository *repo, struct dir_struct *dir,
diff --git a/t/t3013-ls-files-format.sh b/t/t3013-ls-files-format.sh
index 60c415aafd6..baf03f9096e 100755
--- a/t/t3013-ls-files-format.sh
+++ b/t/t3013-ls-files-format.sh
@@ -40,27 +40,13 @@ test_expect_success 'git ls-files --format objectname' '
 	test_cmp expect actual
 '
 
-test_expect_success 'git ls-files --format eolinfo:index' '
-	cat >expect <<-\EOF &&
-	lf
-	lf
-	EOF
-	git ls-files --format="%(eolinfo:index)" >actual &&
-	test_cmp expect actual
-'
-
-test_expect_success 'git ls-files --format eolinfo:worktree' '
-	cat >expect <<-\EOF &&
-	lf
-	lf
-	EOF
-	git ls-files --format="%(eolinfo:worktree)" >actual &&
-	test_cmp expect actual
-'
-
-test_expect_success 'git ls-files --format eolattr' '
-	printf "\n\n" >expect &&
-	git ls-files --format="%(eolattr)" >actual &&
+HT='	'
+WS='    '
+test_expect_success 'git ls-files --format v.s. --eol' '
+	git ls-files --eol >expect 2>err &&
+	test_must_be_empty err &&
+	git ls-files --format="i/%(eolinfo:index)${WS}w/%(eolinfo:worktree)${WS}attr/${WS}${WS}${WS}${WS} ${HT}%(path)" >actual 2>err &&
+	test_must_be_empty err &&
 	test_cmp expect actual
 '
Torsten Bögershausen July 5, 2022, 7:28 p.m. UTC | #2
On Tue, Jul 05, 2022 at 06:32:40AM +0000, ZheNing Hu via GitGitGadget wrote:
> From: ZheNing Hu <adlternative@gmail.com>
>
> Add a new option --format that output index enties
> informations with custom format, taking inspiration
> from the option with the same name in the `git ls-tree`
> command.
[]
> +FIELD NAMES

Nice

> +-----------
> +Various values from structured fields can be used to interpolate
> +into the resulting output. For each outputting line, the following
> +names can be used:
> +
> +objectmode::
> +	The mode of the file which is recorded in the index.
> +objectname::
> +	The name of the file which is recorded in the index.
> +stage::
> +	The stage of the file which is recorded in the index.
> +eolinfo:index::
> +eolinfo:worktree::
> +	The <eolinfo> (see the description of the `--eol` option) of
> +	the contents in the index or in the worktree for the path.
> +eolattr::
> +	The <eolattr> (see the description of the `--eol` option)
> +	that applies to the path.

This may be a matter of taste, looking at the eol-stuff:
Should the ':' be dropped and we have 3 fieldnames like this:

eolindex
eolworktree
eolattr

> +test_expect_success 'git ls-files --format eolinfo:index' '
> +	cat >expect <<-\EOF &&
> +	lf
> +	lf
> +	EOF
> +	git ls-files --format="%(eolinfo:index)" >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git ls-files --format eolinfo:worktree' '
> +	cat >expect <<-\EOF &&
> +	lf
> +	lf
> +	EOF
> +	git ls-files --format="%(eolinfo:worktree)" >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git ls-files --format eolattr' '
> +	printf "\n\n" >expect &&
> +	git ls-files --format="%(eolattr)" >actual &&
> +	test_cmp expect actual
> +'
> +

What exactly should this testcases test ?
Does it make sense to set up a combination of index, worktree, attr,
which are happening in real live ?

There are some tests in t0025, t0027 and t0028 that do more
realistic tests of different combinations.
ZheNing Hu July 11, 2022, 3:14 p.m. UTC | #3
Ævar Arnfjörð Bjarmason <avarab@gmail.com> 于2022年7月5日周二 16:50写道:
>
>
> On Tue, Jul 05 2022, ZheNing Hu via GitGitGadget wrote:
>
> > From: ZheNing Hu <adlternative@gmail.com>
> >
> > Add a new option --format that output index enties
> > informations with custom format, taking inspiration
> > from the option with the same name in the `git ls-tree`
> > command.
> >
> > --format cannot used with -s, -o, -k, -t, --resolve-undo,
> > --deduplicate and --eol.
> >
> > Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> > ---
> >     ls-files: introduce "--format" options
> >
> >     v4->v5:
> >
> >      1. Let --format incompatible with -t.
> >      2. Fix %(eolinfo) and %(eolattr) docs suggested by Junio.
> >
> >     Looking forward to Ævar's reviewing.
>
> Thanks again, I took a look at this and it looks good to me as-is.
>
> If you do want to further twiddle with it at this point I applied these
> changes to it locally while poking around, changes:
>
>  * Some trivial whitespace between variable decl.
>
>  * Removed a "return;" at the end of a function
>
>  * I found the new write_*() helpers to be uneccesary, what I did spot
>    after seeing if they could be factored out is the existing
>    write_eolinfo() function.
>
>    I see you just copied some of the code from there, but
>    e.g. initializing to "" and doing an unconditional strbuf_addstr()
>    looks odd IMO compared to just doing it inline as below.
>

Indeed, it may be a little inelegant...

>    I think if helpers are to be introduced here I'd think it would make
>    more sense to split out the small bits of behavior from
>    write_eolinfo() so you can call it picemeal and share the code, but
>    since it's calling trivial external functions I think just calling
>    those directly probably makes more sense...
>
>  * Likewise for the test I wondered if you were adding a bug by not
>    reporting when lstat() failed, but then found that this is the same
>    thing we do on --eol.
>

Yes, write_eolinfo() ignore lstat() error too, so this would not be a problem.

>    So for the tests I think it's better just to demonstrate that we can
>    emit the exact same thing that --eol does with --format.
>
>  * We've gone back & forth a bit on whether this would combine with
>    --debug, while it's an internal-only feature it would be nice to have a
>    test for it combined with --format, noting that the behavior might
>    change...
>

Oh, if we really want --format, --debug used with --eol, -t, some user
may curious about why --format can not used with  --eol, -t (without --debug),
and I think this will make it interface more complicated. So now I pefer to keep
origin design.

>  * There is one subtle behavior change here in that I deleted the "ce
>    &&" part from write_index_eolinfo_to_buf() when moving the code
>    over. I'm 99% sure this is the right thing to do, as other code in
>    expand_show_index() unconditionally dereferences it.
>
>    So perhaps we don't need that guard in write_eolinfo() either? In any
>    case copy/pasting it over when we're already assuming a non-NULL "ce"
>    in the same "if/elseif/else" chain looks a bit odd.
>
>    Ah, I see it's because in show_dir_entry() we explicitly pass it as
>    NULL, but that doesn't apply to our new codepath, so as long as we're
>    not sharing that helper with write_eolinfo() it makes sense to not do
>    that check.
>

Agree.

>    Even then the helper should probably assume "ce", and write_eolinfo()
>    itself should do the "is ce NULL?" check which is specific to its
>    use-case.
>
> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> index 79ecdce2c9c..cc3cece3830 100644
> --- a/builtin/ls-files.c
> +++ b/builtin/ls-files.c
> @@ -77,30 +77,6 @@ static void write_eolinfo(struct index_state *istate,
>         }
>  }
>
> -static void write_index_eolinfo_to_buf(struct strbuf *sb, struct index_state *istate,
> -                                      const struct cache_entry *ce)
> -{
> -       const char *i_txt = "";
> -       if (ce && S_ISREG(ce->ce_mode))
> -               i_txt = get_cached_convert_stats_ascii(istate, ce->name);
> -       strbuf_addstr(sb, i_txt);
> -}
> -
> -static void write_worktree_eolinfo_to_buf(struct strbuf *sb, const char *path)
> -{
> -       struct stat st;
> -       const char *w_txt = "";
> -       if (!lstat(path, &st) && S_ISREG(st.st_mode))
> -               w_txt = get_wt_convert_stats_ascii(path);
> -       strbuf_addstr(sb, w_txt);
> -}
> -
> -static void write_eolattr_to_buf(struct strbuf *sb, struct index_state *istate,
> -                                const char *path)
> -{
> -       strbuf_addstr(sb, get_convert_attr_ascii(istate, path));
> -}
> -
>  static void write_name(const char *name)
>  {
>         /*
> @@ -114,6 +90,7 @@ static void write_name(const char *name)
>  static void write_name_to_buf(struct strbuf *sb, const char *name)
>  {
>         const char *rel = relative_path(name, prefix_len ? prefix : NULL, sb);
> +
>         if (line_terminator)
>                 quote_c_style(rel, sb, NULL, 0);
>         else
> @@ -270,6 +247,8 @@ static size_t expand_show_index(struct strbuf *sb, const char *start,
>         const char *end;
>         const char *p;
>         size_t len = strbuf_expand_literal_cb(sb, start, NULL);
> +       struct stat st;
> +
>         if (len)
>                 return len;
>         if (*start != '(')
> @@ -288,12 +267,16 @@ static size_t expand_show_index(struct strbuf *sb, const char *start,
>                 strbuf_add_unique_abbrev(sb, &data->ce->oid, abbrev);
>         else if (skip_prefix(start, "(stage)", &p))
>                 strbuf_addf(sb, "%d", ce_stage(data->ce));
> -       else if (skip_prefix(start, "(eolinfo:index)", &p))
> -               write_index_eolinfo_to_buf(sb, data->istate, data->ce);
> -       else if (skip_prefix(start, "(eolinfo:worktree)", &p))
> -               write_worktree_eolinfo_to_buf(sb, data->pathname);
> +       else if (skip_prefix(start, "(eolinfo:index)", &p) &&
> +                S_ISREG(data->ce->ce_mode))
> +               strbuf_addstr(sb, get_cached_convert_stats_ascii(data->istate,
> +                                                                data->ce->name));
> +       else if (skip_prefix(start, "(eolinfo:worktree)", &p) &&
> +                !lstat(data->pathname, &st) && S_ISREG(st.st_mode))
> +               strbuf_addstr(sb, get_wt_convert_stats_ascii(data->pathname));
>         else if (skip_prefix(start, "(eolattr)", &p))
> -               write_eolattr_to_buf(sb, data->istate, data->pathname);
> +               strbuf_addstr(sb, get_convert_attr_ascii(data->istate,
> +                                                        data->pathname));
>         else if (skip_prefix(start, "(path)", &p))
>                 write_name_to_buf(sb, data->pathname);
>         else
> @@ -310,13 +293,12 @@ static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,
>                 .istate = repo->index,
>                 .ce = ce,
>         };
> -
>         struct strbuf sb = STRBUF_INIT;
> +
>         strbuf_expand(&sb, format, expand_show_index, &data);
>         strbuf_addch(&sb, line_terminator);
>         fwrite(sb.buf, sb.len, 1, stdout);
>         strbuf_release(&sb);
> -       return;
>  }
>
>  static void show_ce(struct repository *repo, struct dir_struct *dir,
> diff --git a/t/t3013-ls-files-format.sh b/t/t3013-ls-files-format.sh
> index 60c415aafd6..baf03f9096e 100755
> --- a/t/t3013-ls-files-format.sh
> +++ b/t/t3013-ls-files-format.sh
> @@ -40,27 +40,13 @@ test_expect_success 'git ls-files --format objectname' '
>         test_cmp expect actual
>  '
>
> -test_expect_success 'git ls-files --format eolinfo:index' '
> -       cat >expect <<-\EOF &&
> -       lf
> -       lf
> -       EOF
> -       git ls-files --format="%(eolinfo:index)" >actual &&
> -       test_cmp expect actual
> -'
> -
> -test_expect_success 'git ls-files --format eolinfo:worktree' '
> -       cat >expect <<-\EOF &&
> -       lf
> -       lf
> -       EOF
> -       git ls-files --format="%(eolinfo:worktree)" >actual &&
> -       test_cmp expect actual
> -'
> -
> -test_expect_success 'git ls-files --format eolattr' '
> -       printf "\n\n" >expect &&
> -       git ls-files --format="%(eolattr)" >actual &&
> +HT='   '
> +WS='    '
> +test_expect_success 'git ls-files --format v.s. --eol' '
> +       git ls-files --eol >expect 2>err &&
> +       test_must_be_empty err &&
> +       git ls-files --format="i/%(eolinfo:index)${WS}w/%(eolinfo:worktree)${WS}attr/${WS}${WS}${WS}${WS} ${HT}%(path)" >actual 2>err &&
> +       test_must_be_empty err &&
>         test_cmp expect actual
>  '
>

Thanks for review and help :-)

ZheNing Hu
ZheNing Hu July 11, 2022, 3:27 p.m. UTC | #4
Torsten Bögershausen <tboegi@web.de> 于2022年7月6日周三 03:28写道:
>
> On Tue, Jul 05, 2022 at 06:32:40AM +0000, ZheNing Hu via GitGitGadget wrote:
> > From: ZheNing Hu <adlternative@gmail.com>
> >
> > Add a new option --format that output index enties
> > informations with custom format, taking inspiration
> > from the option with the same name in the `git ls-tree`
> > command.
> []
> > +FIELD NAMES
>
> Nice
>
> > +-----------
> > +Various values from structured fields can be used to interpolate
> > +into the resulting output. For each outputting line, the following
> > +names can be used:
> > +
> > +objectmode::
> > +     The mode of the file which is recorded in the index.
> > +objectname::
> > +     The name of the file which is recorded in the index.
> > +stage::
> > +     The stage of the file which is recorded in the index.
> > +eolinfo:index::
> > +eolinfo:worktree::
> > +     The <eolinfo> (see the description of the `--eol` option) of
> > +     the contents in the index or in the worktree for the path.
> > +eolattr::
> > +     The <eolattr> (see the description of the `--eol` option)
> > +     that applies to the path.
>
> This may be a matter of taste, looking at the eol-stuff:
> Should the ':' be dropped and we have 3 fieldnames like this:
>
> eolindex
> eolworktree
> eolattr
>

Let's see the document of --eol in git-ls-files.txt:

--eol::
     Show <eolinfo> and <eolattr> of files.
     <eolinfo> is the file content identification used by Git when
     the "text" attribute is "auto" (or not set and core.autocrlf is not false).
     <eolinfo> is either "-text", "none", "lf", "crlf", "mixed" or "".

There mentioned eolinfo and eolattr many times, so let's keep it.

> > +test_expect_success 'git ls-files --format eolinfo:index' '
> > +     cat >expect <<-\EOF &&
> > +     lf
> > +     lf
> > +     EOF
> > +     git ls-files --format="%(eolinfo:index)" >actual &&
> > +     test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'git ls-files --format eolinfo:worktree' '
> > +     cat >expect <<-\EOF &&
> > +     lf
> > +     lf
> > +     EOF
> > +     git ls-files --format="%(eolinfo:worktree)" >actual &&
> > +     test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'git ls-files --format eolattr' '
> > +     printf "\n\n" >expect &&
> > +     git ls-files --format="%(eolattr)" >actual &&
> > +     test_cmp expect actual
> > +'
> > +
>
> What exactly should this testcases test ?
> Does it make sense to set up a combination of index, worktree, attr,
> which are happening in real live ?
>
> There are some tests in t0025, t0027 and t0028 that do more
> realistic tests of different combinations.
>
>

Origin test is not good, But now I decide use Avar's patch version:

-test_expect_success 'git ls-files --format eolattr' '
-       printf "\n\n" >expect &&
-       git ls-files --format="%(eolattr)" >actual &&
+HT='   '
+WS='    '
+test_expect_success 'git ls-files --format v.s. --eol' '
+       git ls-files --eol >expect 2>err &&
+       test_must_be_empty err &&
+       git ls-files
--format="i/%(eolinfo:index)${WS}w/%(eolinfo:worktree)${WS}attr/${WS}${WS}${WS}${WS}
${HT}%(path)" >actual 2>err &&
+       test_must_be_empty err &&
        test_cmp expect actual

it can compare the output of git ls-files --format with git ls-files --eol.

Thanks for review!

ZheNing Hu
diff mbox series

Patch

diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index 0dabf3f0ddc..97d4cebba9f 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -20,7 +20,7 @@  SYNOPSIS
 		[--exclude-standard]
 		[--error-unmatch] [--with-tree=<tree-ish>]
 		[--full-name] [--recurse-submodules]
-		[--abbrev[=<n>]] [--] [<file>...]
+		[--abbrev[=<n>]] [--format=<format>] [--] [<file>...]
 
 DESCRIPTION
 -----------
@@ -192,6 +192,13 @@  followed by the  ("attr/<eolattr>").
 	to the contained files. Sparse directories will be shown with a
 	trailing slash, such as "x/" for a sparse directory "x".
 
+--format=<format>::
+	A string that interpolates `%(fieldname)` from the result being shown.
+	It also interpolates `%%` to `%`, and `%xx` where `xx` are hex digits
+	interpolates to character with hex code `xx`; for example `%00`
+	interpolates to `\0` (NUL), `%09` to `\t` (TAB) and %0a to `\n` (LF).
+	--format cannot be combined with `-s`, `-o`, `-k`, `-t`, `--resolve-undo`
+	and `--eol`.
 \--::
 	Do not interpret any more arguments as options.
 
@@ -223,6 +230,35 @@  quoted as explained for the configuration variable `core.quotePath`
 (see linkgit:git-config[1]).  Using `-z` the filename is output
 verbatim and the line is terminated by a NUL byte.
 
+It is possible to print in a custom format by using the `--format`
+option, which is able to interpolate different fields using
+a `%(fieldname)` notation. For example, if you only care about the
+"objectname" and "path" fields, you can execute with a specific
+"--format" like
+
+	git ls-files --format='%(objectname) %(path)'
+
+FIELD NAMES
+-----------
+Various values from structured fields can be used to interpolate
+into the resulting output. For each outputting line, the following
+names can be used:
+
+objectmode::
+	The mode of the file which is recorded in the index.
+objectname::
+	The name of the file which is recorded in the index.
+stage::
+	The stage of the file which is recorded in the index.
+eolinfo:index::
+eolinfo:worktree::
+	The <eolinfo> (see the description of the `--eol` option) of
+	the contents in the index or in the worktree for the path.
+eolattr::
+	The <eolattr> (see the description of the `--eol` option)
+	that applies to the path.
+path::
+	The pathname of the file which is recorded in the index.
 
 EXCLUDE PATTERNS
 ----------------
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index e791b65e7e9..79ecdce2c9c 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -11,6 +11,7 @@ 
 #include "quote.h"
 #include "dir.h"
 #include "builtin.h"
+#include "strbuf.h"
 #include "tree.h"
 #include "cache-tree.h"
 #include "parse-options.h"
@@ -48,6 +49,7 @@  static char *ps_matched;
 static const char *with_tree;
 static int exc_given;
 static int exclude_args;
+static const char *format;
 
 static const char *tag_cached = "";
 static const char *tag_unmerged = "";
@@ -75,6 +77,30 @@  static void write_eolinfo(struct index_state *istate,
 	}
 }
 
+static void write_index_eolinfo_to_buf(struct strbuf *sb, struct index_state *istate,
+				       const struct cache_entry *ce)
+{
+	const char *i_txt = "";
+	if (ce && S_ISREG(ce->ce_mode))
+		i_txt = get_cached_convert_stats_ascii(istate, ce->name);
+	strbuf_addstr(sb, i_txt);
+}
+
+static void write_worktree_eolinfo_to_buf(struct strbuf *sb, const char *path)
+{
+	struct stat st;
+	const char *w_txt = "";
+	if (!lstat(path, &st) && S_ISREG(st.st_mode))
+		w_txt = get_wt_convert_stats_ascii(path);
+	strbuf_addstr(sb, w_txt);
+}
+
+static void write_eolattr_to_buf(struct strbuf *sb, struct index_state *istate,
+				 const char *path)
+{
+	strbuf_addstr(sb, get_convert_attr_ascii(istate, path));
+}
+
 static void write_name(const char *name)
 {
 	/*
@@ -85,6 +111,15 @@  static void write_name(const char *name)
 				   stdout, line_terminator);
 }
 
+static void write_name_to_buf(struct strbuf *sb, const char *name)
+{
+	const char *rel = relative_path(name, prefix_len ? prefix : NULL, sb);
+	if (line_terminator)
+		quote_c_style(rel, sb, NULL, 0);
+	else
+		strbuf_add(sb, rel, strlen(rel));
+}
+
 static const char *get_tag(const struct cache_entry *ce, const char *tag)
 {
 	static char alttag[4];
@@ -222,6 +257,68 @@  static void show_submodule(struct repository *superproject,
 	repo_clear(&subrepo);
 }
 
+struct show_index_data {
+	const char *pathname;
+	struct index_state *istate;
+	const struct cache_entry *ce;
+};
+
+static size_t expand_show_index(struct strbuf *sb, const char *start,
+			       void *context)
+{
+	struct show_index_data *data = context;
+	const char *end;
+	const char *p;
+	size_t len = strbuf_expand_literal_cb(sb, start, NULL);
+	if (len)
+		return len;
+	if (*start != '(')
+		die(_("bad ls-files format: element '%s' "
+		      "does not start with '('"), start);
+
+	end = strchr(start + 1, ')');
+	if (!end)
+		die(_("bad ls-files format: element '%s'"
+		      "does not end in ')'"), start);
+
+	len = end - start + 1;
+	if (skip_prefix(start, "(objectmode)", &p))
+		strbuf_addf(sb, "%06o", data->ce->ce_mode);
+	else if (skip_prefix(start, "(objectname)", &p))
+		strbuf_add_unique_abbrev(sb, &data->ce->oid, abbrev);
+	else if (skip_prefix(start, "(stage)", &p))
+		strbuf_addf(sb, "%d", ce_stage(data->ce));
+	else if (skip_prefix(start, "(eolinfo:index)", &p))
+		write_index_eolinfo_to_buf(sb, data->istate, data->ce);
+	else if (skip_prefix(start, "(eolinfo:worktree)", &p))
+		write_worktree_eolinfo_to_buf(sb, data->pathname);
+	else if (skip_prefix(start, "(eolattr)", &p))
+		write_eolattr_to_buf(sb, data->istate, data->pathname);
+	else if (skip_prefix(start, "(path)", &p))
+		write_name_to_buf(sb, data->pathname);
+	else
+		die(_("bad ls-files format: %%%.*s"), (int)len, start);
+
+	return len;
+}
+
+static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,
+			const char *format, const char *fullname) {
+
+	struct show_index_data data = {
+		.pathname = fullname,
+		.istate = repo->index,
+		.ce = ce,
+	};
+
+	struct strbuf sb = STRBUF_INIT;
+	strbuf_expand(&sb, format, expand_show_index, &data);
+	strbuf_addch(&sb, line_terminator);
+	fwrite(sb.buf, sb.len, 1, stdout);
+	strbuf_release(&sb);
+	return;
+}
+
 static void show_ce(struct repository *repo, struct dir_struct *dir,
 		    const struct cache_entry *ce, const char *fullname,
 		    const char *tag)
@@ -236,6 +333,12 @@  static void show_ce(struct repository *repo, struct dir_struct *dir,
 				  max_prefix_len, ps_matched,
 				  S_ISDIR(ce->ce_mode) ||
 				  S_ISGITLINK(ce->ce_mode))) {
+		if (format) {
+			show_ce_fmt(repo, ce, format, fullname);
+			print_debug(ce);
+			return;
+		}
+
 		tag = get_tag(ce, tag);
 
 		if (!show_stage) {
@@ -675,6 +778,9 @@  int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 			 N_("suppress duplicate entries")),
 		OPT_BOOL(0, "sparse", &show_sparse_dirs,
 			 N_("show sparse directories in the presence of a sparse index")),
+		OPT_STRING_F(0, "format", &format, N_("format"),
+			     N_("format to use for the output"),
+			     PARSE_OPT_NONEG),
 		OPT_END()
 	};
 	int ret = 0;
@@ -699,6 +805,13 @@  int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 	for (i = 0; i < exclude_list.nr; i++) {
 		add_pattern(exclude_list.items[i].string, "", 0, pl, --exclude_args);
 	}
+
+	if (format && (show_stage || show_others || show_killed ||
+		show_resolve_undo || skipping_duplicates || show_eol || show_tag))
+			usage_msg_opt("--format cannot used with -s, -o, -k, -t"
+				      "--resolve-undo, --deduplicate, --eol",
+				      ls_files_usage, builtin_ls_files_options);
+
 	if (show_tag || show_valid_bit || show_fsmonitor_bit) {
 		tag_cached = "H ";
 		tag_unmerged = "M ";
diff --git a/t/t3013-ls-files-format.sh b/t/t3013-ls-files-format.sh
new file mode 100755
index 00000000000..60c415aafd6
--- /dev/null
+++ b/t/t3013-ls-files-format.sh
@@ -0,0 +1,108 @@ 
+#!/bin/sh
+
+test_description='git ls-files --format test'
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+for flag in -s -o -k -t --resolve-undo --deduplicate --eol
+do
+	test_expect_success "usage: --format is incompatible with $flag" '
+		test_expect_code 129 git ls-files --format="%(objectname)" $flag
+	'
+done
+
+test_expect_success 'setup' '
+	echo o1 >o1 &&
+	echo o2 >o2 &&
+	git add o1 o2 &&
+	git add --chmod +x o1 &&
+	git commit -m base
+'
+
+test_expect_success 'git ls-files --format objectmode' '
+	cat >expect <<-\EOF &&
+	100755
+	100644
+	EOF
+	git ls-files --format="%(objectmode)" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git ls-files --format objectname' '
+	oid1=$(git hash-object o1) &&
+	oid2=$(git hash-object o2) &&
+	cat >expect <<-EOF &&
+	$oid1
+	$oid2
+	EOF
+	git ls-files --format="%(objectname)" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git ls-files --format eolinfo:index' '
+	cat >expect <<-\EOF &&
+	lf
+	lf
+	EOF
+	git ls-files --format="%(eolinfo:index)" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git ls-files --format eolinfo:worktree' '
+	cat >expect <<-\EOF &&
+	lf
+	lf
+	EOF
+	git ls-files --format="%(eolinfo:worktree)" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git ls-files --format eolattr' '
+	printf "\n\n" >expect &&
+	git ls-files --format="%(eolattr)" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git ls-files --format path' '
+	cat >expect <<-\EOF &&
+	o1
+	o2
+	EOF
+	git ls-files --format="%(path)" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git ls-files --format with -m' '
+	echo change >o1 &&
+	cat >expect <<-\EOF &&
+	o1
+	EOF
+	git ls-files --format="%(path)" -m >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git ls-files --format with -d' '
+	echo o3 >o3 &&
+	git add o3 &&
+	rm o3 &&
+	cat >expect <<-\EOF &&
+	o3
+	EOF
+	git ls-files --format="%(path)" -d >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git ls-files --format imitate --stage' '
+	git ls-files --stage >expect &&
+	git ls-files --format="%(objectmode) %(objectname) %(stage)%x09%(path)" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git ls-files --format with --debug' '
+	git ls-files --debug >expect &&
+	git ls-files --format="%(path)" --debug >actual &&
+	test_cmp expect actual
+'
+
+test_done