ls-files: align format atoms with git ls-tree #1533
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/submit |
|
Submitted as pull.1533.git.1683969100.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| @@ -270,6 +270,8 @@ interpolated. The following "fieldname" are understood: | |||
|
|
|||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: ZheNing Hu <adlternative@gmail.com>
>
> Sometimes users may want to align the feature of
> `git ls-files --format` with that of `git ls-tree --format`,
> but the %(objecttype) atom is missing in the format option
> of git ls-files compared to git ls-tree.
"Sometimes users may want to" sounds a bit awkward; even if no user
notices that the two very similar commands supports different subset
of the vocabulary without good reason, wouldn't we want to align the
feature set of these two commands?
> Therefore, the %(objecttype) atom is added to the format option
> of git ls-files, which can be used to obtain the object type
> of the file which is recorded in the index.
And from that point of view, this conclusion has a bit more to think
about. Is the %(objecttype) singled out here only because somebody
happened to have complained on the list, or did somebody went into
the list of supported atoms between two commands and considered what
is missing from one but is supported by the other, and concluded that
only adding this one atom to ls-files would make the two consistent?
I would not complain if it were the former, but it must be explained
here in the proposed log message. That would encourage others to do
a follow-on work to complete the comparison to fill the gaps on the
both sides. If it were the former, saying so explicitly in the
proposed log message will save others---otherwise they may try to do
the comparison themselves only to find that this was the last one
remaining discrepancy.
> Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> ---
> Documentation/git-ls-files.txt | 2 ++
> builtin/ls-files.c | 2 ++
> t/t3013-ls-files-format.sh | 7 +++++++
> 3 files changed, 11 insertions(+)
>
> diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
> index 1abdd3c21c5..4356c094cec 100644
> --- a/Documentation/git-ls-files.txt
> +++ b/Documentation/git-ls-files.txt
> @@ -270,6 +270,8 @@ interpolated. The following "fieldname" are understood:
>
> objectmode::
> The mode of the file which is recorded in the index.
> +objecttype::
> + The object type of the file which is recorded in the index.
> objectname::
> The name of the file which is recorded in the index.
> stage::
> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> index 625f48f0d61..6ff764cda18 100644
> --- a/builtin/ls-files.c
> +++ b/builtin/ls-files.c
> @@ -272,6 +272,8 @@ static size_t expand_show_index(struct strbuf *sb, const char *start,
> 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, "(objecttype)", &p))
> + strbuf_addstr(sb, type_name(object_type(data->ce->ce_mode)));
> else if (skip_prefix(start, "(stage)", &p))
> strbuf_addf(sb, "%d", ce_stage(data->ce));
> else if (skip_prefix(start, "(eolinfo:index)", &p))
> diff --git a/t/t3013-ls-files-format.sh b/t/t3013-ls-files-format.sh
> index ef6fb53f7f1..3a1da3d6697 100755
> --- a/t/t3013-ls-files-format.sh
> +++ b/t/t3013-ls-files-format.sh
> @@ -38,6 +38,13 @@ test_expect_success 'git ls-files --format objectname v.s. -s' '
> test_cmp expect actual
> '
>
> +test_expect_success 'git ls-files --format objecttype' '
> + git ls-files --format="%(objectname)" o1.txt o4.txt o6.txt >objectname &&
> + git cat-file --batch-check="%(objecttype)" >expect <objectname &&
> + git ls-files --format="%(objecttype)" o1.txt o4.txt o6.txt >actual &&
> + test_cmp expect actual
> +'
> +
> test_expect_success 'git ls-files --format v.s. --eol' '
> git ls-files --eol >tmp &&
> sed -e "s/ / /g" -e "s/ */ /g" tmp >expect 2>err &&There was a problem hiding this comment.
On the Git mailing list, ZheNing Hu wrote (reply to this):
Junio C Hamano <gitster@pobox.com> 于2023年5月15日周一 13:00写道：
>
> "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: ZheNing Hu <adlternative@gmail.com>
> >
> > Sometimes users may want to align the feature of
> > `git ls-files --format` with that of `git ls-tree --format`,
> > but the %(objecttype) atom is missing in the format option
> > of git ls-files compared to git ls-tree.
>
> "Sometimes users may want to" sounds a bit awkward; even if no user
> notices that the two very similar commands supports different subset
> of the vocabulary without good reason, wouldn't we want to align the
> feature set of these two commands?
>
> > Therefore, the %(objecttype) atom is added to the format option
> > of git ls-files, which can be used to obtain the object type
> > of the file which is recorded in the index.
>
> And from that point of view, this conclusion has a bit more to think
> about. Is the %(objecttype) singled out here only because somebody
> happened to have complained on the list, or did somebody went into
> the list of supported atoms between two commands and considered what
> is missing from one but is supported by the other, and concluded that
> only adding this one atom to ls-files would make the two consistent?
>
> I would not complain if it were the former, but it must be explained
> here in the proposed log message. That would encourage others to do
> a follow-on work to complete the comparison to fill the gaps on the
> both sides. If it were the former, saying so explicitly in the
> proposed log message will save others---otherwise they may try to do
> the comparison themselves only to find that this was the last one
> remaining discrepancy.
>
I think the original requirement is that users wanted to obtain a similar
output format to the default output format of git ls-tree directly through
git ls-files --format="%(objectmode) %(objecttype) %(objectname)%x09%(path)",
but found that the corresponding functionality was missing.
However, from a deeper perspective, the results displayed by git ls-files
for the index and git ls-tree -r for the tree are very similar. Making
git ls-files compatible with the atoms of git ls-tree can provide a
unified view here,
and can also be used for some conversion between the index and tree, such as
git ls-files --format | git mktree.
| @@ -270,8 +270,14 @@ interpolated. The following "fieldname" are understood: | |||
|
|
|||
| objectmode:: | |||
| The mode of the file which is recorded in the index. | |||
| objecttype:: | |||
| The object type of the file which is recorded in the index. | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: ZheNing Hu <adlternative@gmail.com>
>
> Sometimes users may want to align the feature of ...
Exactly the same comment applies here to this patch.
If these two are the only ones missing and after these patches
ls-files and ls-tree become equivalent in the support of atoms,
then explaining that in the log message and do these in a single
patch would make the most sense.
Thanks.
|
This patch series was integrated into seen via git@e836513. |
|
This branch is now known as |
|
This patch series was integrated into seen via git@e500b47. |
|
This patch series was integrated into seen via git@891b7db. |
|
This patch series was integrated into seen via git@4129a0f. |
|
There was a status update in the "New Topics" section about the branch Some atoms that can be used in "--format=3D<format>" for "git ls-tree" were not supported by "git ls-files", even though they were relevant in the context of the latter. source: <pull.1533.git.1683969100.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@f4e0eb8. |
|
This patch series was integrated into seen via git@383d94b. |
|
This patch series was integrated into seen via git@bf1bd67. |
|
This patch series was integrated into seen via git@f66a267. |
|
This patch series was integrated into seen via git@fc73701. |
|
There was a status update in the "Cooking" section about the branch Some atoms that can be used in "--format=<format>" for "git ls-tree" were not supported by "git ls-files", even though they were relevant in the context of the latter. source: <pull.1533.git.1683969100.gitgitgadget@gmail.com> |
adlternative
force-pushed
the
zh/ls-files-formats-expand
branch
3 times, most recently
from
1327661
to
a99618c
Compare
"git ls-files --format" can be used to format the output of multiple file entries in the index, while "git ls-tree --format" can be used to format the contents of a tree object. However, the current set of %(objecttype), "(objectsize)", and "%(objectsize:padded)" atoms supported by "git ls-files --format" is a subset of what is available in "git ls-tree --format". Users sometimes need to establish a unified view between the index and tree, which can help with comparison or conversion between the two. Therefore, this patch adds the missing atoms to "git ls-files --format". "%(objecttype)" can be used to retrieve the object type corresponding to a file in the index, "(objectsize)" can be used to retrieve the object size corresponding to a file in the index, and "%(objectsize:padded)" is the same as "(objectsize)", except with padded format. Signed-off-by: ZheNing Hu <adlternative@gmail.com>
|
/submit |
|
Submitted as pull.1533.v2.git.1684832418299.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
This patch series was integrated into seen via git@3082bd0. |
|
There was a status update in the "Cooking" section about the branch Some atoms that can be used in "--format=<format>" for "git ls-tree" were not supported by "git ls-files", even though they were relevant in the context of the latter. Will merge to 'next'. source: <pull.1533.v2.git.1684832418299.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@0e256d7. |
|
This patch series was integrated into next via git@116b11e. |
|
This patch series was integrated into seen via git@2192e60. |
|
There was a status update in the "Cooking" section about the branch Some atoms that can be used in "--format=<format>" for "git ls-tree" were not supported by "git ls-files", even though they were relevant in the context of the latter. Will cook in 'next'. source: <pull.1533.v2.git.1684832418299.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@743145b. |
|
There was a status update in the "Cooking" section about the branch Some atoms that can be used in "--format=<format>" for "git ls-tree" were not supported by "git ls-files", even though they were relevant in the context of the latter. Will cook in 'next'. source: <pull.1533.v2.git.1684832418299.gitgitgadget@gmail.com> |
|
There was a status update in the "Cooking" section about the branch Some atoms that can be used in "--format=<format>" for "git ls-tree" were not supported by "git ls-files", even though they were relevant in the context of the latter. Will cook in 'next'. source: <pull.1533.v2.git.1684832418299.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@8399e52. |
|
This patch series was integrated into seen via git@fe61620. |
|
There was a status update in the "Cooking" section about the branch Some atoms that can be used in "--format=<format>" for "git ls-tree" were not supported by "git ls-files", even though they were relevant in the context of the latter. Will merge to 'master'. source: <pull.1533.v2.git.1684832418299.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@32fe7ff. |
|
This patch series was integrated into master via git@32fe7ff. |
|
This patch series was integrated into next via git@32fe7ff. |
|
Closed via 32fe7ff. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Users sometimes want all format atoms of git ls-files --format to be
compatible with the format atoms of git ls-tree --format [1]. However,
git ls-files --format lacks the %(objecttype) and %(objectsize),
%(objectsize:padded) atoms compared to git ls-tree --format, causing
incompatibility. Therefore, these atoms are added to the --format
of git ls-files to resolve the issue of incompatibility.
%(objecttype): get the object type of the file which is recorded in the index.
%(objectsize): get the object size of the file which is recorded in the index,
("-" if the object is a
commitortree).%(objectsize:padded): same as %(objectsize), but with a padded format.
v1:
add %(objecttype) and %(objectsize) atos to git ls-files --format.
v2:
[1]: https://lore.kernel.org/git/21183ea9-84e2-fd89-eb9b-419556680c07@gnieh.org/T/#u
cc: Junio C Hamano gitster@pobox.com
cc: Ævar Arnfjörð Bjarmason avarab@gmail.com
cc: Martin Monperrus martin.monperrus@gnieh.org