{"thread":{"id":"54009","subject":"[RFC PATCH 0/2] extend --abbrev support to diff-patch format","startedAt":"2020-08-09T02:19:35Z","lastAt":"2020-08-21T15:44:40Z","messageCount":41,"participants":["Đoàn Trần Công Danh","Junio C Hamano","Jeff King","SZEDER Gábor","brian m. carlson"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"403213","messageId":"cover.1596887883.git.congdanhqx@gmail.com","threadId":"54009","inReplyTo":null,"subject":"[RFC PATCH 0/2] extend --abbrev support to diff-patch format","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-09T02:19:03Z","receivedAt":"2020-08-09T02:19:35Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"For some reason, projects tracks local copy of released version of other\nprojects, and backports change to earlier version.\n\nDue to lower number of object in simplified history,\nabbreviated object id has fewer characters, thus generate some noise\nwhen projects try to compare the backported patch with original patch\nif the file hunk is an exact match.\n\nThis series try to lower that noise.\nSince this is very localised use case,\nand the noise is not completely eliminated,\nother experienced developers may have better opinions.\n\nĐoàn Trần Công Danh (2):\n revision: differentiate if --no-abbrev asked explicitly\n diff: extend --abbrev support to diff-patch format\n\n Documentation/diff-options.txt | 9 +++---\n diff.c | 5 +++-\n revision.c | 2 +-\n t/t4013-diff-various.sh | 3 ++\n ...ff.diff-tree_--root_-p_--abbrev=10_initial | 29 +++++++++++++++++++\n ...--root_-p_--full-index_--abbrev=10_initial | 29 +++++++++++++++++++\n ...f.diff-tree_--root_-p_--full-index_initial | 29 +++++++++++++++++++\n 7 files changed, 100 insertions(+), 6 deletions(-)\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n\n-- \n2.28.0.215.g32ffa52ee0\n\n"},{"id":"403214","messageId":"33c02761534f4e0a272f7bc9c7c9ac0218724ace.1596887883.git.congdanhqx@gmail.com","threadId":"54009","inReplyTo":"cover.1596887883.git.congdanhqx@gmail.com","subject":"[RFC PATCH 1/2] revision: differentiate if --no-abbrev asked explicitly","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-09T02:19:04Z","receivedAt":"2020-08-09T02:19:36Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"When we see `--no-abbrev' in command's arguments, we reset `abbrev' of\n`diff_options` to 0, thus, on a later stage, the object id won't\nbe shortened (by not set object_id[abbrev] to '\\0').\n\nWhile not doing anything is very effective way to show full object id,\nwe couldn't differentiate if --no-abbrev or not.\n\nIn a later change, we want to extend --abbrev support to diff-patch\nformat.\n\nLet's ask for full object id if we see --no-abbrev instead.\n\nSigned-off-by: Đoàn Trần Công Danh \n---\n revision.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/revision.c b/revision.c\nindex 6de29cdf7a..69cc834662 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2432,7 +2432,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--always\")) {\n \t\trevs->always_show_header = 1;\n \t} else if (!strcmp(arg, \"--no-abbrev\")) {\n-\t\trevs->abbrev = 0;\n+\t\trevs->abbrev = hexsz;\n \t} else if (!strcmp(arg, \"--abbrev\")) {\n \t\trevs->abbrev = DEFAULT_ABBREV;\n \t} else if (skip_prefix(arg, \"--abbrev=\", &optarg)) {\n-- \n2.28.0.215.g32ffa52ee0\n\n"},{"id":"403215","messageId":"36c46357e56ce91025c4e447eaf19d485dc997d7.1596887883.git.congdanhqx@gmail.com","threadId":"54009","inReplyTo":"cover.1596887883.git.congdanhqx@gmail.com","subject":"[RFC PATCH 2/2] diff: extend --abbrev support to diff-patch format","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-09T02:19:05Z","receivedAt":"2020-08-09T02:19:38Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"Signed-off-by: Đoàn Trần Công Danh \n---\n\nNot sure if this part should go to the commit message or not:\n\nTo preserve backward compatibility with old script that specify both\n--full-index and --abbrev, always shows full object id if --full-index\nis specified.\n\n Documentation/diff-options.txt | 9 +++---\n diff.c | 5 +++-\n t/t4013-diff-various.sh | 3 ++\n ...ff.diff-tree_--root_-p_--abbrev=10_initial | 29 +++++++++++++++++++\n ...--root_-p_--full-index_--abbrev=10_initial | 29 +++++++++++++++++++\n ...f.diff-tree_--root_-p_--full-index_initial | 29 +++++++++++++++++++\n 6 files changed, 99 insertions(+), 5 deletions(-)\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 7987d72b02..c11efa7865 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -441,10 +441,11 @@ endif::git-format-patch[]\n --abbrev[=]::\n \tInstead of showing the full 40-byte hexadecimal object\n \tname in diff-raw format output and diff-tree header\n-\tlines, show only a partial prefix. This is\n-\tindependent of the `--full-index` option above, which controls\n-\tthe diff-patch output format. Non default number of\n-\tdigits can be specified with `--abbrev=`.\n+\tlines, show only a partial prefix.\n+\tIn diff-patch output format, `--full-index` takes higher\n+\tprecedent, i.e. if `--full-index` is specified, full blob\n+\tnames will be shown regardless of `--abbrev`.\n+\tNon default number of digits can be specified with `--abbrev=`.\n \n -B[][/]::\n --break-rewrites[=[][/]]::\ndiff --git a/diff.c b/diff.c\nindex d24aaa3047..b5dedb0165 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4319,7 +4319,10 @@ static void fill_metainfo(struct strbuf *msg,\n \t}\n \tif (one && two && !oideq(&one->oid, &two->oid)) {\n \t\tconst unsigned hexsz = the_hash_algo->hexsz;\n-\t\tint abbrev = o->flags.full_index ? hexsz : DEFAULT_ABBREV;\n+\t\tint abbrev = o->abbrev ? o->abbrev : DEFAULT_ABBREV;\n+\n+\t\tif (o->flags.full_index)\n+\t\t\tabbrev = hexsz;\n \n \t\tif (o->flags.binary) {\n \t\t\tmmfile_t mf;\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex 3f60f7d96c..e6eb4dd4c7 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -221,6 +221,9 @@ diff-tree --root -r --abbrev=4 initial\n :noellipses diff-tree --root -r --abbrev=4 initial\n diff-tree -p initial\n diff-tree --root -p initial\n+diff-tree --root -p --abbrev=10 initial\n+diff-tree --root -p --full-index initial\n+diff-tree --root -p --full-index --abbrev=10 initial\n diff-tree --patch-with-stat initial\n diff-tree --root --patch-with-stat initial\n diff-tree --patch-with-raw initial\ndiff --git a/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial b/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\nnew file mode 100644\nindex 0000000000..7518a9044e\n--- /dev/null\n+++ b/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n@@ -0,0 +1,29 @@\n+$ git diff-tree --root -p --abbrev=10 initial\n+444ac553ac7612cc88969031b02b3767fb8a353a\n+diff --git a/dir/sub b/dir/sub\n+new file mode 100644\n+index 0000000000..35d242ba79\n+--- /dev/null\n++++ b/dir/sub\n+@@ -0,0 +1,2 @@\n++A\n++B\n+diff --git a/file0 b/file0\n+new file mode 100644\n+index 0000000000..01e79c32a8\n+--- /dev/null\n++++ b/file0\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+diff --git a/file2 b/file2\n+new file mode 100644\n+index 0000000000..01e79c32a8\n+--- /dev/null\n++++ b/file2\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+$\ndiff --git a/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial b/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\nnew file mode 100644\nindex 0000000000..69f913fbe5\n--- /dev/null\n+++ b/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n@@ -0,0 +1,29 @@\n+$ git diff-tree --root -p --full-index --abbrev=10 initial\n+444ac553ac7612cc88969031b02b3767fb8a353a\n+diff --git a/dir/sub b/dir/sub\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..35d242ba79ae89ac695e26b3d4c27a8e6f028f9e\n+--- /dev/null\n++++ b/dir/sub\n+@@ -0,0 +1,2 @@\n++A\n++B\n+diff --git a/file0 b/file0\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file0\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+diff --git a/file2 b/file2\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file2\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+$\ndiff --git a/t/t4013/diff.diff-tree_--root_-p_--full-index_initial b/t/t4013/diff.diff-tree_--root_-p_--full-index_initial\nnew file mode 100644\nindex 0000000000..1b0b6717fa\n--- /dev/null\n+++ b/t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n@@ -0,0 +1,29 @@\n+$ git diff-tree --root -p --full-index initial\n+444ac553ac7612cc88969031b02b3767fb8a353a\n+diff --git a/dir/sub b/dir/sub\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..35d242ba79ae89ac695e26b3d4c27a8e6f028f9e\n+--- /dev/null\n++++ b/dir/sub\n+@@ -0,0 +1,2 @@\n++A\n++B\n+diff --git a/file0 b/file0\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file0\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+diff --git a/file2 b/file2\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file2\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+$\n-- \n2.28.0.215.g32ffa52ee0\n\n"},{"id":"403237","messageId":"xmqqd03zej8w.fsf@gitster.c.googlers.com","threadId":"54009","inReplyTo":"cover.1596887883.git.congdanhqx@gmail.com","subject":"Re: [RFC PATCH 0/2] extend --abbrev support to diff-patch format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-09T19:01:35Z","receivedAt":"2020-08-09T19:01:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n> Đoàn Trần Công Danh (2):\n> revision: differentiate if --no-abbrev asked explicitly\n> diff: extend --abbrev support to diff-patch format\n\nIt was not clear, at least to me at all, what these patches are\ntrying to achieve (i.e. what end-users appreciate) until I saw the\ncode change X-<.\n\nThe changes to fill_metainfo() make sense to me. It just needs log\nmessages that explain the intent better. They do not even make it\nclear that they want to make the abbreviation length of the object\nnames on the \"index $from..$to $mode\" lines configurable.\n\nThanks.\n\n"},{"id":"403253","messageId":"20200810100038.GB37030@coredump.intra.peff.net","threadId":"54009","inReplyTo":"xmqqd03zej8w.fsf@gitster.c.googlers.com","subject":"Re: [RFC PATCH 0/2] extend --abbrev support to diff-patch format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-08-10T10:00:38Z","receivedAt":"2020-08-10T10:00:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 09, 2020 at 12:01:35PM -0700, Junio C Hamano wrote:\n\n> Đoàn Trần Công Danh writes:\n> \n> > Đoàn Trần Công Danh (2):\n> > revision: differentiate if --no-abbrev asked explicitly\n> > diff: extend --abbrev support to diff-patch format\n> \n> It was not clear, at least to me at all, what these patches are\n> trying to achieve (i.e. what end-users appreciate) until I saw the\n> code change X-<.\n> \n> The changes to fill_metainfo() make sense to me. It just needs log\n> messages that explain the intent better. They do not even make it\n> clear that they want to make the abbreviation length of the object\n> names on the \"index $from..$to $mode\" lines configurable.\n\nAfter reading the original including cover letter, I'm still confused\nusing why --full-index is not the solution for most cases. Perhaps that\nwould be worth touching on, as well.\n\n-Peff\n"},{"id":"403259","messageId":"20200810123148.GB17119@danh.dev","threadId":"54009","inReplyTo":"20200810100038.GB37030@coredump.intra.peff.net","subject":"Re: [RFC PATCH 0/2] extend --abbrev support to diff-patch format","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-10T12:31:48Z","receivedAt":"2020-08-10T12:31:56Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-08-10 06:00:38-0400, Jeff King wrote:\n> On Sun, Aug 09, 2020 at 12:01:35PM -0700, Junio C Hamano wrote:\n> \n> > Đoàn Trần Công Danh writes:\n> > \n> > > Đoàn Trần Công Danh (2):\n> > > revision: differentiate if --no-abbrev asked explicitly\n> > > diff: extend --abbrev support to diff-patch format\n> > \n> > It was not clear, at least to me at all, what these patches are\n> > trying to achieve (i.e. what end-users appreciate) until I saw the\n> > code change X-<.\n> > \n> > The changes to fill_metainfo() make sense to me. It just needs log\n> > messages that explain the intent better. They do not even make it\n> > clear that they want to make the abbreviation length of the object\n> > names on the \"index $from..$to $mode\" lines configurable.\n> \n> After reading the original including cover letter, I'm still confused\n> using why --full-index is not the solution for most cases. Perhaps that\n> would be worth touching on, as well.\n\nAt that time, I'm not really sure what should be written there.\nThe commit message was inspired by --abbrev documentation.\n\nReading both of your two's emails, I think this one could be used:\nI'll resend this serie if this serie is deemed useful with this\nexplaination.\n\n\tdiff: index-line: make object name's abbrev length configurable\n\n\tThere are some projects that want to archive and track only\n\treleased version of other software projects. They also want\n\tto backport some changes into those versions unsupported by\n\tupstream. Most of git hosting services support some method to\n\tdownload patches without cloning the full (potentially large)\n\trepository and/or fiddling with git partial-clone or\n\tsparse-checkout.\n\n\tMost of those git hosting services generate those patches with\n\tgit-format-patch(1) or something alike. Due to its large\n\tamount of objects, their object names' abbreviation in the\n\tindex-line is usually long but not full.\n\n\tA lot of those patches couldn't be applied cleanly to old\n\tversions of said software, thus requires some changes from\n\tdeveloper and they needs to be regenerated from their trimmed\n\ttree. Because the archive tree has significantly fewer\n\tobjects, the abbreviation in the index line is usually shorter\n\tthan the original patch. Thus, it generates some noise when\n\tsaid developers try to compare the new patch with the original\n\tpatch if there's an exact file-hunk match.\n\n\tMake the object name's abbreviation length configurable to\n\tlower those noise.\n\n\t\n\n\tTo preserve backward compatibility with old script that specify\n\tboth --full-index and --abbrev, always shows full object id\n\tif --full-index is specified.\n\n\n-- \nDanh\n"},{"id":"403266","messageId":"xmqq364uee1s.fsf@gitster.c.googlers.com","threadId":"54009","inReplyTo":"20200810100038.GB37030@coredump.intra.peff.net","subject":"Re: [RFC PATCH 0/2] extend --abbrev support to diff-patch format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-10T15:06:07Z","receivedAt":"2020-08-10T15:06:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sun, Aug 09, 2020 at 12:01:35PM -0700, Junio C Hamano wrote:\n>\n>> Đoàn Trần Công Danh writes:\n>> \n>> > Đoàn Trần Công Danh (2):\n>> > revision: differentiate if --no-abbrev asked explicitly\n>> > diff: extend --abbrev support to diff-patch format\n>> \n>> It was not clear, at least to me at all, what these patches are\n>> trying to achieve (i.e. what end-users appreciate) until I saw the\n>> code change X-<.\n>> \n>> The changes to fill_metainfo() make sense to me. It just needs log\n>> messages that explain the intent better. They do not even make it\n>> clear that they want to make the abbreviation length of the object\n>> names on the \"index $from..$to $mode\" lines configurable.\n>\n> After reading the original including cover letter, I'm still confused\n> using why --full-index is not the solution for most cases. Perhaps that\n> would be worth touching on, as well.\n\nTrue. \n\nPresumably you could force some stability without sacrificing line\nlength limit by using --abbrev=12 instead of --full-index but I do\nnot think it is such a big deal. But it does look odd that we use\na special/single-purpose option --full-index to control the length\nonly for those two object names on the \"index\" line, when all the\nother object names we see are controlled with the --abbrev option.\n\n"},{"id":"403267","messageId":"xmqqy2mmcz1e.fsf@gitster.c.googlers.com","threadId":"54009","inReplyTo":"20200810123148.GB17119@danh.dev","subject":"Re: [RFC PATCH 0/2] extend --abbrev support to diff-patch format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-10T15:15:41Z","receivedAt":"2020-08-10T15:15:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n> Reading both of your two's emails, I think this one could be used:\n> I'll resend this serie if this serie is deemed useful with this\n> explaination.\n>\n> \tdiff: index-line: make object name's abbrev length configurable\n>\n> \tThere are some projects that want to archive and track only\n> \treleased version of other software projects. They also want\n> \tto backport some changes into those versions unsupported by\n> \tupstream. Most of git hosting services support some method to\n> \tdownload patches without cloning the full (potentially large)\n> \trepository and/or fiddling with git partial-clone or\n> \tsparse-checkout.\n>\n> \tMost of those git hosting services generate those patches with\n> \tgit-format-patch(1) or something alike. Due to its large\n> \tamount of objects, their object names' abbreviation in the\n> \tindex-line is usually long but not full.\n>\n> \tA lot of those patches couldn't be applied cleanly to old\n> \tversions of said software, thus requires some changes from\n> \tdeveloper and they needs to be regenerated from their trimmed\n> \ttree. Because the archive tree has significantly fewer\n> \tobjects, the abbreviation in the index line is usually shorter\n> \tthan the original patch. Thus, it generates some noise when\n> \tsaid developers try to compare the new patch with the original\n> \tpatch if there's an exact file-hunk match.\n>\n> \tMake the object name's abbreviation length configurable to\n> \tlower those noise.\n\nI agree with Peff that with the above as the sole motivating use\ncase, the \"--full-index\" option is the right approach. It is a much\nmore robust solution than \"--abbrev=16 would be long enough for all\nproject participants to avoid length drift\". IOW these four\nparagraphs do not argue _for_ this change, at least to me.\n\nOn the other hand, I think you could argue that \"--full-index\" is\nmerely a synonym for \"--abbrev=40\", and the patch fixes the\ninconsistency between the object names on the \"index\" line, which\ncan choose only between the default abbrev length and the full\nabbrev length, and all the other places we show object names, which\nuniformly honor the \"--abbrev\" option.\n\n\n> \t \tinto commit message>\n>\n> \tTo preserve backward compatibility with old script that specify\n> \tboth --full-index and --abbrev, always shows full object id\n> \tif --full-index is specified.\n"},{"id":"403268","messageId":"20200810152705.GA61606@coredump.intra.peff.net","threadId":"54009","inReplyTo":"xmqqy2mmcz1e.fsf@gitster.c.googlers.com","subject":"Re: [RFC PATCH 0/2] extend --abbrev support to diff-patch format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-08-10T15:27:05Z","receivedAt":"2020-08-10T15:36:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 10, 2020 at 08:15:41AM -0700, Junio C Hamano wrote:\n\n> > \tA lot of those patches couldn't be applied cleanly to old\n> > \tversions of said software, thus requires some changes from\n> > \tdeveloper and they needs to be regenerated from their trimmed\n> > \ttree. Because the archive tree has significantly fewer\n> > \tobjects, the abbreviation in the index line is usually shorter\n> > \tthan the original patch. Thus, it generates some noise when\n> > \tsaid developers try to compare the new patch with the original\n> > \tpatch if there's an exact file-hunk match.\n> >\n> > \tMake the object name's abbreviation length configurable to\n> > \tlower those noise.\n> \n> I agree with Peff that with the above as the sole motivating use\n> case, the \"--full-index\" option is the right approach. It is a much\n> more robust solution than \"--abbrev=16 would be long enough for all\n> project participants to avoid length drift\". IOW these four\n> paragraphs do not argue _for_ this change, at least to me.\n\nYeah, that's what I was getting at: if you care about robust\nmachine-readability, then the full index is the best solution. Reading\nbetween the lines, I think the argument may be \"using --full-index is\ntoo long and therefore ugly, so people like the short-ish names but with\na bit of extra safety\".\n\nThere's an extra challenge here, which is that you have to convince the\nsender to use the extra --abbrev option, even though they themselves\nwon't be the ones running into the problem when applying. But I don't\nthink there's an elegant solution to that (we could just bump the\ndefault abbrev everywhere to 12+, which is enough in practice).\n\nThough I'm not 100% sure that \"git apply\" is smart enough to only look\nat blobs (i.e., if \"1234abcd\" is ambiguous between a tree and a blob,\nignore the tree since patches always apply to blobs). That might be\nanother avenue that would make things more likely to work without\nanybody having to configure anything.\n\n> On the other hand, I think you could argue that \"--full-index\" is\n> merely a synonym for \"--abbrev=40\", and the patch fixes the\n> inconsistency between the object names on the \"index\" line, which\n> can choose only between the default abbrev length and the full\n> abbrev length, and all the other places we show object names, which\n> uniformly honor the \"--abbrev\" option.\n\nYeah, I certainly don't mind the extra flexibility between \"full\" and\n\"default\" for \"index\" lines. I do wonder if people want to configure the\nabbreviations for those lines separately from other parts. I don't know\nthat I've ever particularly cared about that flexibility, but the fact\nthat they were set up separately all those years ago makes me think\nsomebody might.\n\n-Peff\n"},{"id":"403325","messageId":"20200811003359.GD17119@danh.dev","threadId":"54009","inReplyTo":"20200810152705.GA61606@coredump.intra.peff.net","subject":"Re: [RFC PATCH 0/2] extend --abbrev support to diff-patch format","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-11T00:33:59Z","receivedAt":"2020-08-11T00:34:04Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-08-10 11:27:05-0400, Jeff King wrote:\n> On Mon, Aug 10, 2020 at 08:15:41AM -0700, Junio C Hamano wrote:\n> \n> > > \tA lot of those patches couldn't be applied cleanly to old\n> > > \tversions of said software, thus requires some changes from\n> > > \tdeveloper and they needs to be regenerated from their trimmed\n> > > \ttree. Because the archive tree has significantly fewer\n> > > \tobjects, the abbreviation in the index line is usually shorter\n> > > \tthan the original patch. Thus, it generates some noise when\n> > > \tsaid developers try to compare the new patch with the original\n> > > \tpatch if there's an exact file-hunk match.\n> > >\n> > > \tMake the object name's abbreviation length configurable to\n> > > \tlower those noise.\n> > \n> > I agree with Peff that with the above as the sole motivating use\n> > case, the \"--full-index\" option is the right approach. It is a much\n> > more robust solution than \"--abbrev=16 would be long enough for all\n> > project participants to avoid length drift\". IOW these four\n> > paragraphs do not argue _for_ this change, at least to me.\n> \n> Yeah, that's what I was getting at: if you care about robust\n> machine-readability, then the full index is the best solution. Reading\n> between the lines, I think the argument may be \"using --full-index is\n> too long and therefore ugly, so people like the short-ish names but with\n> a bit of extra safety\".\n\nMy argument was people can either easily fetch the patch via HTTP like:\n\n\tcurl -LO https://github.com/git/git/commit/eb12adc74cf22add318f884072be2071d181abaa.patch\n\nor take it from a mailing list archive, bugzilla, instead of\ncloning a full repository. With those options, we can't say,\n\"we prefer full-index, please send us the patch with full-index\ninstead\".\n\n> \n> There's an extra challenge here, which is that you have to convince the\n> sender to use the extra --abbrev option, even though they themselves\n> won't be the ones running into the problem when applying.\n\nNot really, since the sender tree is usually larger than the archived\ntree, their abbrev is usually long enough, and the receiver will use\n--abbrev to lengthen their abbrev to reduce the noise instead.\n\n> But I don't\n> think there's an elegant solution to that (we could just bump the\n> default abbrev everywhere to 12+, which is enough in practice).\n> Though I'm not 100% sure that \"git apply\" is smart enough to only look\n> at blobs (i.e., if \"1234abcd\" is ambiguous between a tree and a blob,\n> ignore the tree since patches always apply to blobs). That might be\n> another avenue that would make things more likely to work without\n> anybody having to configure anything.\n> \n> > On the other hand, I think you could argue that \"--full-index\" is\n> > merely a synonym for \"--abbrev=40\", and the patch fixes the\n> > inconsistency between the object names on the \"index\" line, which\n> > can choose only between the default abbrev length and the full\n> > abbrev length, and all the other places we show object names, which\n> > uniformly honor the \"--abbrev\" option.\n\nI think this argument could be a way to go.\nIn fact, I always try to use --abbrev with diff family because I know\nit works with a handful with other tools, (describe, blame), then\nI surprise that it doesn't work, and the documentation tells me\n`--abbrev` only works with diff-raw and diff-tree header line.\n\nThen, I keep forgetting that documentation, and I tried again.\n\nFor now, I filtered out the index line before comparing 2 patches.\n\n> Yeah, I certainly don't mind the extra flexibility between \"full\" and\n> \"default\" for \"index\" lines. I do wonder if people want to configure the\n> abbreviations for those lines separately from other parts. I don't know\n> that I've ever particularly cared about that flexibility, but the fact\n> that they were set up separately all those years ago makes me think\n> somebody might.\n\nI don't think people particularly care about the index line (and to\nthe extent, its length) that much, since the default is number is\nactually a minimum number, if Git can't differentiate object with that\nnumber of characters, Git will show a longer object names anyway.\n\nI think most people scripts will put a regex for:\n\n\t/index [a-z0-9]{7,}\\.\\.[a-z0-9]{7,} [0-7]{6}/\n\nOr even:\n\n\t/index [a-z0-9]+\\.\\.[a-z0-9]+ [0-7]+/\n\nFor the former case, we could change the code in 2/2 to set the minimum\ndefault to DEFAULT_ABBREV instead of MINIMUM_ABBREV?\n\nFor the historical case that users put both --full-index and --abbrev\ninto there scripts, we still keep our promise to not break their\nscript by always respect --full-index, regardless of --abbrev.\n\n-- \nDanh\n"},{"id":"403332","messageId":"20200811052226.GA82699@coredump.intra.peff.net","threadId":"54009","inReplyTo":"20200811003359.GD17119@danh.dev","subject":"Re: [RFC PATCH 0/2] extend --abbrev support to diff-patch format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-08-11T05:22:26Z","receivedAt":"2020-08-11T05:22:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 11, 2020 at 07:33:59AM +0700, Đoàn Trần Công Danh wrote:\n\n> > Yeah, that's what I was getting at: if you care about robust\n> > machine-readability, then the full index is the best solution. Reading\n> > between the lines, I think the argument may be \"using --full-index is\n> > too long and therefore ugly, so people like the short-ish names but with\n> > a bit of extra safety\".\n> \n> My argument was people can either easily fetch the patch via HTTP like:\n> \n> \tcurl -LO https://github.com/git/git/commit/eb12adc74cf22add318f884072be2071d181abaa.patch\n> \n> or take it from a mailing list archive, bugzilla, instead of\n> cloning a full repository. With those options, we can't say,\n> \"we prefer full-index, please send us the patch with full-index\n> instead\".\n\nOK. But then how would they use \"--abbrev\" in that case? I.e., isn't it\ntoo late at that point (especially in the mailing list archive case) to\ndo change anything in the formatting of the patch?\n\nMaybe I'm confused...\n\n> > There's an extra challenge here, which is that you have to convince the\n> > sender to use the extra --abbrev option, even though they themselves\n> > won't be the ones running into the problem when applying.\n> \n> Not really, since the sender tree is usually larger than the archived\n> tree, their abbrev is usually long enough, and the receiver will use\n> --abbrev to lengthen their abbrev to reduce the noise instead.\n\nNow I'm doubly confused. If the sender has the larger tree then they'll\nhave the larger abbrev. So what's the problem?\n\nGoing back to re-read your earlier responses...So...this _isn't_ a\nproblem within Git itself? It's only about people trying to compare\ntextual patches byte-for-byte and seeing different index lines?\n\nIf that's the case, then it seems to me that the byte comparison is the\nproblem here. If I have:\n\n index 1234abcd..5678bcde\n\nand\n\n index 1234abcd87..5678bcde65\n\nthose should be considered equivalent to see if two patches are\nplausibly the same. And I think tools like git-cherry, etc, would do\nthat (and we provide git-patch-id for that purpose, too).\n\n> > Yeah, I certainly don't mind the extra flexibility between \"full\" and\n> > \"default\" for \"index\" lines. I do wonder if people want to configure the\n> > abbreviations for those lines separately from other parts. I don't know\n> > that I've ever particularly cared about that flexibility, but the fact\n> > that they were set up separately all those years ago makes me think\n> > somebody might.\n> \n> I don't think people particularly care about the index line (and to\n> the extent, its length) that much, since the default is number is\n> actually a minimum number, if Git can't differentiate object with that\n> number of characters, Git will show a longer object names anyway.\n> \n> I think most people scripts will put a regex for:\n> \n> \t/index [a-z0-9]{7,}\\.\\.[a-z0-9]{7,} [0-7]{6}/\n> \n> Or even:\n> \n> \t/index [a-z0-9]+\\.\\.[a-z0-9]+ [0-7]+/\n> \n> For the former case, we could change the code in 2/2 to set the minimum\n> default to DEFAULT_ABBREV instead of MINIMUM_ABBREV?\n> \n> For the historical case that users put both --full-index and --abbrev\n> into there scripts, we still keep our promise to not break their\n> script by always respect --full-index, regardless of --abbrev.\n\nI care less about scripting (as you note, anything consuming abbreviated\nobjects has to handle longer-than-minimum names anyway), and was more\nwondering whether anybody really cared that:\n\n git log --abbrev=30 -p\n\nkept the short index lines (e.g., because they're easier to read). But\nI'm having trouble coming up with a plausible reason somebody would want\nlong object names in earlier lines like \"Merge:\" but not in the patch\nindex lines. And already we respect --abbrev for --raw, so it's not like\nthe diff code isn't already affected. Making \"-p\" consistent with all\nthe rest of it is probably worth doing regardless.\n\n-Peff\n"},{"id":"403347","messageId":"cover.1597146478.git.congdanhqx@gmail.com","threadId":"54009","inReplyTo":"cover.1596887883.git.congdanhqx@gmail.com","subject":"[PATCH v2 0/2] diff: index-line: respect --abbrev in object's name","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-11T11:49:55Z","receivedAt":"2020-08-11T11:50:05Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"For diff-family, Git supports 2 different options for 2 different\npurposes, `--full-index' for showing diff-patch object's name in full,\nand `--abbrev' to customize the length of object names in diff-raw and\ndiff-tree header lines, without any options to customise the length of\nobject names in diff-patch format. When working with diff-patch format,\nwe only have two options, either full index, or default abbrev length.\n\nAlthough, that consistent is documented, it doesn't stop users from\ntrying to use `--abbrev' with the hope of customising diff-patch's\nobjects' name's abbreviation.\n\nLet's resolve that inconsistency.\n\nĐoàn Trần Công Danh (2):\n revision: differentiate if --no-abbrev asked explicitly\n diff: index-line: respect --abbrev in object's name\n\n Documentation/diff-options.txt | 9 +++---\n diff.c | 5 +++-\n revision.c | 2 +-\n t/t4013-diff-various.sh | 3 ++\n ...ff.diff-tree_--root_-p_--abbrev=10_initial | 29 +++++++++++++++++++\n ...--root_-p_--full-index_--abbrev=10_initial | 29 +++++++++++++++++++\n ...f.diff-tree_--root_-p_--full-index_initial | 29 +++++++++++++++++++\n 7 files changed, 100 insertions(+), 6 deletions(-)\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n\nRange-diff against v1:\n1: 9daef7445c = 1: 9daef7445c revision: differentiate if --no-abbrev asked explicitly\n2: 9146313893 ! 2: 12acf1fe5d diff: extend --abbrev support to diff-patch format\n @@ Metadata\n Author: Đoàn Trần Công Danh \n \n ## Commit message ##\n - diff: extend --abbrev support to diff-patch format\n + diff: index-line: respect --abbrev in object's name\n +\n + A handful of Git's commands respect `--abbrev' for customizing length\n + of abbreviation of object names.\n +\n + For diff-family, Git supports 2 different options for 2 different\n + purposes, `--full-index' for showing diff-patch object's name in full,\n + and `--abbrev' to customize the length of object names in diff-raw and\n + diff-tree header lines, without any options to customise the length of\n + object names in diff-patch format. When working with diff-patch format,\n + we only have two options, either full index, or default abbrev length.\n +\n + Although, that consistent is documented, it doesn't stop users from\n + trying to use `--abbrev' with the hope of customising diff-patch's\n + objects' name's abbreviation.\n +\n + Let's resolve that inconsistency.\n +\n + To preserve backward compatibility with old script that specify both\n + `--full-index' and `--abbrev', always shows full object id\n + if `--full-index' is specified.\n \n Signed-off-by: Đoàn Trần Công Danh \n \n-- \n2.28.0.215.g32ffa52ee0\n\n"},{"id":"403348","messageId":"9daef7445c09d355d4801f64e427c27dd6c44afb.1597146478.git.congdanhqx@gmail.com","threadId":"54009","inReplyTo":"cover.1597146478.git.congdanhqx@gmail.com","subject":"[PATCH v2 1/2] revision: differentiate if --no-abbrev asked explicitly","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-11T11:49:56Z","receivedAt":"2020-08-11T11:50:07Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"When we see `--no-abbrev' in command's arguments, we reset `abbrev' of\n`diff_options` to 0, thus, on a later stage, the object id won't\nbe shortened (by not set object_id[abbrev] to '\\0').\n\nWhile not doing anything is very effective way to show full object id,\nwe couldn't differentiate if --no-abbrev or not.\n\nIn a later change, we want to extend --abbrev support to diff-patch\nformat.\n\nLet's ask for full object id if we see --no-abbrev instead.\n\nSigned-off-by: Đoàn Trần Công Danh \n---\n revision.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/revision.c b/revision.c\nindex 3dcf689341..24027b1af3 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2430,7 +2430,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--always\")) {\n \t\trevs->always_show_header = 1;\n \t} else if (!strcmp(arg, \"--no-abbrev\")) {\n-\t\trevs->abbrev = 0;\n+\t\trevs->abbrev = hexsz;\n \t} else if (!strcmp(arg, \"--abbrev\")) {\n \t\trevs->abbrev = DEFAULT_ABBREV;\n \t} else if (skip_prefix(arg, \"--abbrev=\", &optarg)) {\n-- \n2.28.0.215.g32ffa52ee0\n\n"},{"id":"403349","messageId":"12acf1fe5db55199102d27eeb93524a96f914a05.1597146478.git.congdanhqx@gmail.com","threadId":"54009","inReplyTo":"cover.1597146478.git.congdanhqx@gmail.com","subject":"[PATCH v2 2/2] diff: index-line: respect --abbrev in object's name","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-11T11:49:57Z","receivedAt":"2020-08-11T11:50:11Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"A handful of Git's commands respect `--abbrev' for customizing length\nof abbreviation of object names.\n\nFor diff-family, Git supports 2 different options for 2 different\npurposes, `--full-index' for showing diff-patch object's name in full,\nand `--abbrev' to customize the length of object names in diff-raw and\ndiff-tree header lines, without any options to customise the length of\nobject names in diff-patch format. When working with diff-patch format,\nwe only have two options, either full index, or default abbrev length.\n\nAlthough, that consistent is documented, it doesn't stop users from\ntrying to use `--abbrev' with the hope of customising diff-patch's\nobjects' name's abbreviation.\n\nLet's resolve that inconsistency.\n\nTo preserve backward compatibility with old script that specify both\n`--full-index' and `--abbrev', always shows full object id\nif `--full-index' is specified.\n\nSigned-off-by: Đoàn Trần Công Danh \n---\n Documentation/diff-options.txt | 9 +++---\n diff.c | 5 +++-\n t/t4013-diff-various.sh | 3 ++\n ...ff.diff-tree_--root_-p_--abbrev=10_initial | 29 +++++++++++++++++++\n ...--root_-p_--full-index_--abbrev=10_initial | 29 +++++++++++++++++++\n ...f.diff-tree_--root_-p_--full-index_initial | 29 +++++++++++++++++++\n 6 files changed, 99 insertions(+), 5 deletions(-)\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 7987d72b02..c11efa7865 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -441,10 +441,11 @@ endif::git-format-patch[]\n --abbrev[=]::\n \tInstead of showing the full 40-byte hexadecimal object\n \tname in diff-raw format output and diff-tree header\n-\tlines, show only a partial prefix. This is\n-\tindependent of the `--full-index` option above, which controls\n-\tthe diff-patch output format. Non default number of\n-\tdigits can be specified with `--abbrev=`.\n+\tlines, show only a partial prefix.\n+\tIn diff-patch output format, `--full-index` takes higher\n+\tprecedent, i.e. if `--full-index` is specified, full blob\n+\tnames will be shown regardless of `--abbrev`.\n+\tNon default number of digits can be specified with `--abbrev=`.\n \n -B[][/]::\n --break-rewrites[=[][/]]::\ndiff --git a/diff.c b/diff.c\nindex f9709de7b4..20dedfe2a9 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4319,7 +4319,10 @@ static void fill_metainfo(struct strbuf *msg,\n \t}\n \tif (one && two && !oideq(&one->oid, &two->oid)) {\n \t\tconst unsigned hexsz = the_hash_algo->hexsz;\n-\t\tint abbrev = o->flags.full_index ? hexsz : DEFAULT_ABBREV;\n+\t\tint abbrev = o->abbrev ? o->abbrev : DEFAULT_ABBREV;\n+\n+\t\tif (o->flags.full_index)\n+\t\t\tabbrev = hexsz;\n \n \t\tif (o->flags.binary) {\n \t\t\tmmfile_t mf;\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex 3f60f7d96c..e6eb4dd4c7 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -221,6 +221,9 @@ diff-tree --root -r --abbrev=4 initial\n :noellipses diff-tree --root -r --abbrev=4 initial\n diff-tree -p initial\n diff-tree --root -p initial\n+diff-tree --root -p --abbrev=10 initial\n+diff-tree --root -p --full-index initial\n+diff-tree --root -p --full-index --abbrev=10 initial\n diff-tree --patch-with-stat initial\n diff-tree --root --patch-with-stat initial\n diff-tree --patch-with-raw initial\ndiff --git a/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial b/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\nnew file mode 100644\nindex 0000000000..7518a9044e\n--- /dev/null\n+++ b/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n@@ -0,0 +1,29 @@\n+$ git diff-tree --root -p --abbrev=10 initial\n+444ac553ac7612cc88969031b02b3767fb8a353a\n+diff --git a/dir/sub b/dir/sub\n+new file mode 100644\n+index 0000000000..35d242ba79\n+--- /dev/null\n++++ b/dir/sub\n+@@ -0,0 +1,2 @@\n++A\n++B\n+diff --git a/file0 b/file0\n+new file mode 100644\n+index 0000000000..01e79c32a8\n+--- /dev/null\n++++ b/file0\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+diff --git a/file2 b/file2\n+new file mode 100644\n+index 0000000000..01e79c32a8\n+--- /dev/null\n++++ b/file2\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+$\ndiff --git a/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial b/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\nnew file mode 100644\nindex 0000000000..69f913fbe5\n--- /dev/null\n+++ b/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n@@ -0,0 +1,29 @@\n+$ git diff-tree --root -p --full-index --abbrev=10 initial\n+444ac553ac7612cc88969031b02b3767fb8a353a\n+diff --git a/dir/sub b/dir/sub\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..35d242ba79ae89ac695e26b3d4c27a8e6f028f9e\n+--- /dev/null\n++++ b/dir/sub\n+@@ -0,0 +1,2 @@\n++A\n++B\n+diff --git a/file0 b/file0\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file0\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+diff --git a/file2 b/file2\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file2\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+$\ndiff --git a/t/t4013/diff.diff-tree_--root_-p_--full-index_initial b/t/t4013/diff.diff-tree_--root_-p_--full-index_initial\nnew file mode 100644\nindex 0000000000..1b0b6717fa\n--- /dev/null\n+++ b/t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n@@ -0,0 +1,29 @@\n+$ git diff-tree --root -p --full-index initial\n+444ac553ac7612cc88969031b02b3767fb8a353a\n+diff --git a/dir/sub b/dir/sub\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..35d242ba79ae89ac695e26b3d4c27a8e6f028f9e\n+--- /dev/null\n++++ b/dir/sub\n+@@ -0,0 +1,2 @@\n++A\n++B\n+diff --git a/file0 b/file0\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file0\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+diff --git a/file2 b/file2\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file2\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+$\n-- \n2.28.0.215.g32ffa52ee0\n\n"},{"id":"403350","messageId":"20200811120717.GE17119@danh.dev","threadId":"54009","inReplyTo":"20200811052226.GA82699@coredump.intra.peff.net","subject":"Re: [RFC PATCH 0/2] extend --abbrev support to diff-patch format","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-11T12:07:17Z","receivedAt":"2020-08-11T12:07:35Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-08-11 01:22:26-0400, Jeff King wrote:\n> On Tue, Aug 11, 2020 at 07:33:59AM +0700, Đoàn Trần Công Danh wrote:\n> \n> > > Yeah, that's what I was getting at: if you care about robust\n> > > machine-readability, then the full index is the best solution. Reading\n> > > between the lines, I think the argument may be \"using --full-index is\n> > > too long and therefore ugly, so people like the short-ish names but with\n> > > a bit of extra safety\".\n> > \n> > My argument was people can either easily fetch the patch via HTTP like:\n> > \n> > \tcurl -LO https://github.com/git/git/commit/eb12adc74cf22add318f884072be2071d181abaa.patch\n> > \n> > or take it from a mailing list archive, bugzilla, instead of\n> > cloning a full repository. With those options, we can't say,\n> > \"we prefer full-index, please send us the patch with full-index\n> > instead\".\n> \n> OK. But then how would they use \"--abbrev\" in that case? I.e., isn't it\n> too late at that point (especially in the mailing list archive case) to\n> do change anything in the formatting of the patch?\n> \n> Maybe I'm confused...\n> \n> > > There's an extra challenge here, which is that you have to convince the\n> > > sender to use the extra --abbrev option, even though they themselves\n> > > won't be the ones running into the problem when applying.\n> > \n> > Not really, since the sender tree is usually larger than the archived\n> > tree, their abbrev is usually long enough, and the receiver will use\n> > --abbrev to lengthen their abbrev to reduce the noise instead.\n> \n> Now I'm doubly confused. If the sender has the larger tree then they'll\n> have the larger abbrev. So what's the problem?\n> \n> Going back to re-read your earlier responses...So...this _isn't_ a\n> problem within Git itself?\n\nCorrect. It's NOT Git's problems by any mean.\n\n> It's only about people trying to compare\n> textual patches byte-for-byte and seeing different index lines?\n\nYeah, it's about people trying to backport patch to old tree.\nFixing conflicts, and try to compare to old patch to see if they have\nmade any errors.\n\nBecause conflicts resolving is complicated.\n\n> \n> If that's the case, then it seems to me that the byte comparison is the\n> problem here. If I have:\n> \n> index 1234abcd..5678bcde\n> \n> and\n> \n> index 1234abcd87..5678bcde65\n> \n> those should be considered equivalent to see if two patches are\n> plausibly the same. And I think tools like git-cherry, etc, would do\n> that (and we provide git-patch-id for that purpose, too).\n\nYes, git-patch-id is very useful tool.\n\nThere're time that half of the patch can be applied cleanly with the\nexact object names.\nAnother half needs to be fixed heavily, (maybe removed). git-cherry\nand git-patch-id couldn't cope well in those situation.\nThat condition is true if there's a major change to source tree.\n\n> > > Yeah, I certainly don't mind the extra flexibility between \"full\" and\n> > > \"default\" for \"index\" lines. I do wonder if people want to configure the\n> > > abbreviations for those lines separately from other parts. I don't know\n> > > that I've ever particularly cared about that flexibility, but the fact\n> > > that they were set up separately all those years ago makes me think\n> > > somebody might.\n> > \n> > I don't think people particularly care about the index line (and to\n> > the extent, its length) that much, since the default is number is\n> > actually a minimum number, if Git can't differentiate object with that\n> > number of characters, Git will show a longer object names anyway.\n> > \n> > I think most people scripts will put a regex for:\n> > \n> > \t/index [a-z0-9]{7,}\\.\\.[a-z0-9]{7,} [0-7]{6}/\n> > \n> > Or even:\n> > \n> > \t/index [a-z0-9]+\\.\\.[a-z0-9]+ [0-7]+/\n> > \n> > For the former case, we could change the code in 2/2 to set the minimum\n> > default to DEFAULT_ABBREV instead of MINIMUM_ABBREV?\n> > \n> > For the historical case that users put both --full-index and --abbrev\n> > into there scripts, we still keep our promise to not break their\n> > script by always respect --full-index, regardless of --abbrev.\n> \n> I care less about scripting (as you note, anything consuming abbreviated\n> objects has to handle longer-than-minimum names anyway), and was more\n> wondering whether anybody really cared that:\n> \n> git log --abbrev=30 -p\n> \n> kept the short index lines (e.g., because they're easier to read). But\n> I'm having trouble coming up with a plausible reason somebody would want\n> long object names in earlier lines like \"Merge:\" but not in the patch\n> index lines. And already we respect --abbrev for --raw, so it's not like\n> the diff code isn't already affected. Making \"-p\" consistent with all\n> the rest of it is probably worth doing regardless.\n\nYes, I think this is the easier to accept argument.\nI've gone with that in the resend.\n\n-- \nThanks.\nDanh\n"},{"id":"403374","messageId":"xmqqwo259fp2.fsf@gitster.c.googlers.com","threadId":"54009","inReplyTo":"9daef7445c09d355d4801f64e427c27dd6c44afb.1597146478.git.congdanhqx@gmail.com","subject":"Re: [PATCH v2 1/2] revision: differentiate if --no-abbrev asked explicitly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-11T18:54:01Z","receivedAt":"2020-08-11T18:54:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n> When we see `--no-abbrev' in command's arguments, we reset `abbrev' of\n> `diff_options` to 0, thus, on a later stage, the object id won't\n> be shortened (by not set object_id[abbrev] to '\\0').\n\ns/by not set/&ting/ probably? \n\nPlease do not write pseudo-code that does not exist anywhere in the\ncodebase that pretends to be a quote from some real code. It wasts\nreviewer's time looking for non-existing code to see what really is\ngoing on.\n\nI think you meant this line in diff_abbrev_oid():\n\n\tif (abbrev)\n\t\thex[abbrev] = '\\0';\n\nso you can say that more explicitly, perhaps:\n\n ... we reset the 'abbrev' field in diff-options to 0 and this\n value is looked at diff_abbrev_oid() to decide not to truncate\n the object name.\n\nor somesuch.\n\n> While not doing anything is very effective way to show full object id,\n> we couldn't differentiate if --no-abbrev or not.\n\nYou mean you want to tell if --no-abbrev and/or --full-index was\ngiven from the command line and act differently? You need to\njustify why you need to be able to do so (which you attempted in the\nremainder of the proposed log message), and also you need to justify\nwhy changing revs->abbrev's meaning is the best way to do so (which\nyou did not). In fill_metainfo(), which you are going to touch in\n[2/2], you can peek o->flags.full_index to see if --full-index was\ngiven, and the fact that revs->abbrev is not DEFAULT_ABBREV (which\nis how the field is initialized in repo_init_revisions()) but is 0\ntells us that \"--no-abbrev\" was given. --abbrev=0 would have busted\nminimum abbrev and set the field to MINIMUM_ABBREV, --abbrev=99 would\nhave busted hash size and set the field to hexsz. So 0 is the sign\nthat --no-abbrev was given. No?\n\n> In a later change, we want to extend --abbrev support to diff-patch\n> format.\n\nAnd it is unclear why this planned change requires it.\n\n> Let's ask for full object id if we see --no-abbrev instead.\n\nHence, this \"let's ask\" is not justified very well.\n\n> Signed-off-by: Đoàn Trần Công Danh \n> ---\n> revision.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/revision.c b/revision.c\n> index 3dcf689341..24027b1af3 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2430,7 +2430,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n> \t} else if (!strcmp(arg, \"--always\")) {\n> \t\trevs->always_show_header = 1;\n> \t} else if (!strcmp(arg, \"--no-abbrev\")) {\n> -\t\trevs->abbrev = 0;\n> +\t\trevs->abbrev = hexsz;\n> \t} else if (!strcmp(arg, \"--abbrev\")) {\n> \t\trevs->abbrev = DEFAULT_ABBREV;\n> \t} else if (skip_prefix(arg, \"--abbrev=\", &optarg)) {\n"},{"id":"403600","messageId":"cover.1597364493.git.congdanhqx@gmail.com","threadId":"54009","inReplyTo":"cover.1596887883.git.congdanhqx@gmail.com","subject":"[PATCH v3 0/2] diff: index-line: respect --abbrev in object's name","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-14T00:23:08Z","receivedAt":"2020-08-14T00:23:25Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"For diff-family, Git supports 2 different options for 2 different\npurposes, `--full-index' for showing diff-patch object's name in full,\nand `--abbrev' to customize the length of object names in diff-raw and\ndiff-tree header lines, without any options to customise the length of\nobject names in diff-patch format. When working with diff-patch format,\nwe only have two options, either full index, or default abbrev length.\n\nAlthough, that consistent is documented, it doesn't stop users from\ntrying to use `--abbrev' with the hope of customising diff-patch's\nobjects' name's abbreviation.\n\nLet's resolve that inconsistency.\n\nĐoàn Trần Công Danh (2):\n revision: differentiate if --no-abbrev asked explicitly\n diff: index-line: respect --abbrev in object's name\n\n Documentation/diff-options.txt | 9 +++---\n diff.c | 5 +++-\n revision.c | 2 +-\n t/t4013-diff-various.sh | 3 ++\n ...ff.diff-tree_--root_-p_--abbrev=10_initial | 29 +++++++++++++++++++\n ...--root_-p_--full-index_--abbrev=10_initial | 29 +++++++++++++++++++\n ...f.diff-tree_--root_-p_--full-index_initial | 29 +++++++++++++++++++\n 7 files changed, 100 insertions(+), 6 deletions(-)\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n\nRange-diff against v2:\n1: 9daef7445c ! 1: 9a26c5b611 revision: differentiate if --no-abbrev asked explicitly\n @@ Metadata\n ## Commit message ##\n revision: differentiate if --no-abbrev asked explicitly\n \n - When we see `--no-abbrev' in command's arguments, we reset `abbrev' of\n - `diff_options` to 0, thus, on a later stage, the object id won't\n - be shortened (by not set object_id[abbrev] to '\\0').\n + When we see --no-abbrev in command's arguments, we reset the 'abbrev'\n + field in diff-options to 0 and this value will be looked at\n + diff_abbrev_oid() to decide not to truncate the object name.\n +\n + In a later change, we want to extend --abbrev support to diff-patch\n + format. When --abbrev supporting diff-patch, we need to differentiate\n + those below scenarios:\n +\n + * None of those options --abbrev, --no-abbrev, and --full-index are\n + asked. diff-patch should keep old behavior of using DEFAULT_ABBREV\n + for the index length.\n + * --no-abbrev is asked, diff-patch should treat this option as same as\n + --full-index and show full object name in index line.\n \n While not doing anything is very effective way to show full object id,\n we couldn't differentiate if --no-abbrev or not.\n \n - In a later change, we want to extend --abbrev support to diff-patch\n - format.\n + We can differentiate those above scenarios by either:\n + * Add a new field in diff-options to mark if --no-abbrev was asked.\n + With this option, we have a new field for a single purpose, and one\n + more thing to worry about.\n + * Treat --no-abbrev as same as --full-index. This option is problematic,\n + since we want to allow --abbrev overwrite --no-abbrev again.\n + On the other hand, we also need to keep our promises with scripter\n + who uses --full-index to ask for full object names in index line,\n + so, we need to respsect --full-index regardless of --abbrev.\n + * Set 'abbrev' field in diff-options to the length of the hash we are\n + using. With this option, we can differentiate if --no-abbrev was asked\n + ('abbrev' is hash's length) or none of --[no-]abbrev was asked\n + ('abbrev' is '0'), script with --full-index still works and our\n + headache is kept as is.\n \n - Let's ask for full object id if we see --no-abbrev instead.\n + Let's ask for full object id if we see --no-abbrev.\n \n Signed-off-by: Đoàn Trần Công Danh \n \n2: 12acf1fe5d = 2: 760df7782d diff: index-line: respect --abbrev in object's name\n-- \n2.28.0.215.g32ffa52ee0\n\n"},{"id":"403601","messageId":"9a26c5b6110081cd8d029f2ab0327c4a1d228ef7.1597364493.git.congdanhqx@gmail.com","threadId":"54009","inReplyTo":"cover.1597364493.git.congdanhqx@gmail.com","subject":"[PATCH v3 1/2] revision: differentiate if --no-abbrev asked explicitly","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-14T00:23:09Z","receivedAt":"2020-08-14T00:23:28Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"When we see --no-abbrev in command's arguments, we reset the 'abbrev'\nfield in diff-options to 0 and this value will be looked at\ndiff_abbrev_oid() to decide not to truncate the object name.\n\nIn a later change, we want to extend --abbrev support to diff-patch\nformat. When --abbrev supporting diff-patch, we need to differentiate\nthose below scenarios:\n\n* None of those options --abbrev, --no-abbrev, and --full-index are\n asked. diff-patch should keep old behavior of using DEFAULT_ABBREV\n for the index length.\n* --no-abbrev is asked, diff-patch should treat this option as same as\n --full-index and show full object name in index line.\n\nWhile not doing anything is very effective way to show full object id,\nwe couldn't differentiate if --no-abbrev or not.\n\nWe can differentiate those above scenarios by either:\n* Add a new field in diff-options to mark if --no-abbrev was asked.\n With this option, we have a new field for a single purpose, and one\n more thing to worry about.\n* Treat --no-abbrev as same as --full-index. This option is problematic,\n since we want to allow --abbrev overwrite --no-abbrev again.\n On the other hand, we also need to keep our promises with scripter\n who uses --full-index to ask for full object names in index line,\n so, we need to respsect --full-index regardless of --abbrev.\n* Set 'abbrev' field in diff-options to the length of the hash we are\n using. With this option, we can differentiate if --no-abbrev was asked\n ('abbrev' is hash's length) or none of --[no-]abbrev was asked\n ('abbrev' is '0'), script with --full-index still works and our\n headache is kept as is.\n\nLet's ask for full object id if we see --no-abbrev.\n\nSigned-off-by: Đoàn Trần Công Danh \n---\n revision.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/revision.c b/revision.c\nindex 3dcf689341..24027b1af3 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2430,7 +2430,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--always\")) {\n \t\trevs->always_show_header = 1;\n \t} else if (!strcmp(arg, \"--no-abbrev\")) {\n-\t\trevs->abbrev = 0;\n+\t\trevs->abbrev = hexsz;\n \t} else if (!strcmp(arg, \"--abbrev\")) {\n \t\trevs->abbrev = DEFAULT_ABBREV;\n \t} else if (skip_prefix(arg, \"--abbrev=\", &optarg)) {\n-- \n2.28.0.215.g32ffa52ee0\n\n"},{"id":"403602","messageId":"760df7782dad9e9df7bb284ec57249e697a4cc92.1597364493.git.congdanhqx@gmail.com","threadId":"54009","inReplyTo":"cover.1597364493.git.congdanhqx@gmail.com","subject":"[PATCH v3 2/2] diff: index-line: respect --abbrev in object's name","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-14T00:23:10Z","receivedAt":"2020-08-14T00:23:28Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"A handful of Git's commands respect `--abbrev' for customizing length\nof abbreviation of object names.\n\nFor diff-family, Git supports 2 different options for 2 different\npurposes, `--full-index' for showing diff-patch object's name in full,\nand `--abbrev' to customize the length of object names in diff-raw and\ndiff-tree header lines, without any options to customise the length of\nobject names in diff-patch format. When working with diff-patch format,\nwe only have two options, either full index, or default abbrev length.\n\nAlthough, that consistent is documented, it doesn't stop users from\ntrying to use `--abbrev' with the hope of customising diff-patch's\nobjects' name's abbreviation.\n\nLet's resolve that inconsistency.\n\nTo preserve backward compatibility with old script that specify both\n`--full-index' and `--abbrev', always shows full object id\nif `--full-index' is specified.\n\nSigned-off-by: Đoàn Trần Công Danh \n---\n Documentation/diff-options.txt | 9 +++---\n diff.c | 5 +++-\n t/t4013-diff-various.sh | 3 ++\n ...ff.diff-tree_--root_-p_--abbrev=10_initial | 29 +++++++++++++++++++\n ...--root_-p_--full-index_--abbrev=10_initial | 29 +++++++++++++++++++\n ...f.diff-tree_--root_-p_--full-index_initial | 29 +++++++++++++++++++\n 6 files changed, 99 insertions(+), 5 deletions(-)\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 7987d72b02..c11efa7865 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -441,10 +441,11 @@ endif::git-format-patch[]\n --abbrev[=]::\n \tInstead of showing the full 40-byte hexadecimal object\n \tname in diff-raw format output and diff-tree header\n-\tlines, show only a partial prefix. This is\n-\tindependent of the `--full-index` option above, which controls\n-\tthe diff-patch output format. Non default number of\n-\tdigits can be specified with `--abbrev=`.\n+\tlines, show only a partial prefix.\n+\tIn diff-patch output format, `--full-index` takes higher\n+\tprecedent, i.e. if `--full-index` is specified, full blob\n+\tnames will be shown regardless of `--abbrev`.\n+\tNon default number of digits can be specified with `--abbrev=`.\n \n -B[][/]::\n --break-rewrites[=[][/]]::\ndiff --git a/diff.c b/diff.c\nindex f9709de7b4..20dedfe2a9 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4319,7 +4319,10 @@ static void fill_metainfo(struct strbuf *msg,\n \t}\n \tif (one && two && !oideq(&one->oid, &two->oid)) {\n \t\tconst unsigned hexsz = the_hash_algo->hexsz;\n-\t\tint abbrev = o->flags.full_index ? hexsz : DEFAULT_ABBREV;\n+\t\tint abbrev = o->abbrev ? o->abbrev : DEFAULT_ABBREV;\n+\n+\t\tif (o->flags.full_index)\n+\t\t\tabbrev = hexsz;\n \n \t\tif (o->flags.binary) {\n \t\t\tmmfile_t mf;\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex 3f60f7d96c..e6eb4dd4c7 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -221,6 +221,9 @@ diff-tree --root -r --abbrev=4 initial\n :noellipses diff-tree --root -r --abbrev=4 initial\n diff-tree -p initial\n diff-tree --root -p initial\n+diff-tree --root -p --abbrev=10 initial\n+diff-tree --root -p --full-index initial\n+diff-tree --root -p --full-index --abbrev=10 initial\n diff-tree --patch-with-stat initial\n diff-tree --root --patch-with-stat initial\n diff-tree --patch-with-raw initial\ndiff --git a/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial b/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\nnew file mode 100644\nindex 0000000000..7518a9044e\n--- /dev/null\n+++ b/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n@@ -0,0 +1,29 @@\n+$ git diff-tree --root -p --abbrev=10 initial\n+444ac553ac7612cc88969031b02b3767fb8a353a\n+diff --git a/dir/sub b/dir/sub\n+new file mode 100644\n+index 0000000000..35d242ba79\n+--- /dev/null\n++++ b/dir/sub\n+@@ -0,0 +1,2 @@\n++A\n++B\n+diff --git a/file0 b/file0\n+new file mode 100644\n+index 0000000000..01e79c32a8\n+--- /dev/null\n++++ b/file0\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+diff --git a/file2 b/file2\n+new file mode 100644\n+index 0000000000..01e79c32a8\n+--- /dev/null\n++++ b/file2\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+$\ndiff --git a/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial b/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\nnew file mode 100644\nindex 0000000000..69f913fbe5\n--- /dev/null\n+++ b/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n@@ -0,0 +1,29 @@\n+$ git diff-tree --root -p --full-index --abbrev=10 initial\n+444ac553ac7612cc88969031b02b3767fb8a353a\n+diff --git a/dir/sub b/dir/sub\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..35d242ba79ae89ac695e26b3d4c27a8e6f028f9e\n+--- /dev/null\n++++ b/dir/sub\n+@@ -0,0 +1,2 @@\n++A\n++B\n+diff --git a/file0 b/file0\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file0\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+diff --git a/file2 b/file2\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file2\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+$\ndiff --git a/t/t4013/diff.diff-tree_--root_-p_--full-index_initial b/t/t4013/diff.diff-tree_--root_-p_--full-index_initial\nnew file mode 100644\nindex 0000000000..1b0b6717fa\n--- /dev/null\n+++ b/t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n@@ -0,0 +1,29 @@\n+$ git diff-tree --root -p --full-index initial\n+444ac553ac7612cc88969031b02b3767fb8a353a\n+diff --git a/dir/sub b/dir/sub\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..35d242ba79ae89ac695e26b3d4c27a8e6f028f9e\n+--- /dev/null\n++++ b/dir/sub\n+@@ -0,0 +1,2 @@\n++A\n++B\n+diff --git a/file0 b/file0\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file0\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+diff --git a/file2 b/file2\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file2\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+$\n-- \n2.28.0.215.g32ffa52ee0\n\n"},{"id":"403603","messageId":"xmqq8sei125k.fsf@gitster.c.googlers.com","threadId":"54009","inReplyTo":"9a26c5b6110081cd8d029f2ab0327c4a1d228ef7.1597364493.git.congdanhqx@gmail.com","subject":"Re: [PATCH v3 1/2] revision: differentiate if --no-abbrev asked explicitly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-14T00:50:31Z","receivedAt":"2020-08-14T00:50:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n> When we see --no-abbrev in command's arguments, we reset the 'abbrev'\n> field in diff-options to 0 and this value will be looked at\n> diff_abbrev_oid() to decide not to truncate the object name.\n>\n> In a later change, we want to extend --abbrev support to diff-patch\n> format. When --abbrev supporting diff-patch, we need to differentiate\n> those below scenarios:\n>\n> * None of those options --abbrev, --no-abbrev, and --full-index are\n> asked. diff-patch should keep old behavior of using DEFAULT_ABBREV\n> for the index length.\n> * --no-abbrev is asked, diff-patch should treat this option as same as\n> --full-index and show full object name in index line.\n\nSorry, but are you saying that the above two cases cannot be\ndifferentiated in the current code?\n\n * If none of --abbrev, --no-abbrev, --full-index are given, then\n diff.c::prep_parse_options() will leave options->flags.full_index\n and options->abbrev untouched. They are initialized to false and\n DEFAULT_ABBREV (typically -1 when unconfigured).\n\n * If --no-abbrev is given, options->abbrev is set to 0.\n options->flags.full_index is not touched.\n\nSo you should be able to tell these two apart by only looking at\noptions->flags.full_index bit. Perhaps, even though you said \"we\nneed to differentiate\", you meant something else?\n\n> While not doing anything is very effective way to show full object id,\n> we couldn't differentiate if --no-abbrev or not.\n\nHmph. --no-abbrev without --full-index would not set\nflags.full_index bit; using --full-index would set the bit. Are you\nplanning to do something special when BOTH --no-abbrev and --full-index\nis given? I am confused X-<.\n\n"},{"id":"403605","messageId":"20200814005947.GA9518@danh.dev","threadId":"54009","inReplyTo":"xmqq8sei125k.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 1/2] revision: differentiate if --no-abbrev asked explicitly","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-14T00:59:47Z","receivedAt":"2020-08-14T00:59:52Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-08-13 17:50:31-0700, Junio C Hamano wrote:\n> Đoàn Trần Công Danh writes:\n> \n> > When we see --no-abbrev in command's arguments, we reset the 'abbrev'\n> > field in diff-options to 0 and this value will be looked at\n> > diff_abbrev_oid() to decide not to truncate the object name.\n> >\n> > In a later change, we want to extend --abbrev support to diff-patch\n> > format. When --abbrev supporting diff-patch, we need to differentiate\n> > those below scenarios:\n> >\n> > * None of those options --abbrev, --no-abbrev, and --full-index are\n> > asked. diff-patch should keep old behavior of using DEFAULT_ABBREV\n> > for the index length.\n> > * --no-abbrev is asked, diff-patch should treat this option as same as\n> > --full-index and show full object name in index line.\n> \n> Sorry, but are you saying that the above two cases cannot be\n> differentiated in the current code?\n> \n> * If none of --abbrev, --no-abbrev, --full-index are given, then\n> diff.c::prep_parse_options() will leave options->flags.full_index\n> and options->abbrev untouched. They are initialized to false and\n> DEFAULT_ABBREV (typically -1 when unconfigured).\n> \n> * If --no-abbrev is given, options->abbrev is set to 0.\n> options->flags.full_index is not touched.\n> \n> So you should be able to tell these two apart by only looking at\n> options->flags.full_index bit. Perhaps, even though you said \"we\n> need to differentiate\", you meant something else?\n\nOops, I shouldn't say anything about --full-index in the second point\nto reduce confusion.\n\nLet me list some combination here:\n\n* none of --abbrev --no-abbrev --full-index -> default short index\n* --abbrev --full-index -> full-index\n* --full-index --abbrev -> full-index\n* --abbrev --no-abbrev -> full-index\n* --no-abbrev --abbrev=[n] -> shortened index to n char\n\nSo, we can't use full_index bit, because --no-abbrev can be defeated\nby --abbrev, but --full-index will always win --abbrev.\n\n> \n> > While not doing anything is very effective way to show full object id,\n> > we couldn't differentiate if --no-abbrev or not.\n> \n> Hmph. --no-abbrev without --full-index would not set\n> flags.full_index bit; using --full-index would set the bit. Are you\n> planning to do something special when BOTH --no-abbrev and --full-index\n> is given? I am confused X-<.\n\nI'm not planning for anything special when both --no-abbrev and\n--full-index is given.\n\nI'm planning for:\n\n* BOTH --abbrev and --no-abbrev but NOT --full-index;\n* BOTH --abbrev AND --full-index\n\nSorry for the confusion,\nI hope it's clear now, and you could help me rephase a bit to reduce\nconfusion.\n\n-- \nDanh\n"},{"id":"403607","messageId":"xmqqzh6yyr1t.fsf@gitster.c.googlers.com","threadId":"54009","inReplyTo":"20200814005947.GA9518@danh.dev","subject":"Re: [PATCH v3 1/2] revision: differentiate if --no-abbrev asked explicitly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-14T01:06:22Z","receivedAt":"2020-08-14T01:06:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n> Let me list some combination here:\n>\n> * none of --abbrev --no-abbrev --full-index -> default short index\n> * --abbrev --full-index -> full-index\n> * --full-index --abbrev -> full-index\n> * --abbrev --no-abbrev -> full-index\n> * --no-abbrev --abbrev=[n] -> shortened index to n char\n>\n> So, we can't use full_index bit, because --no-abbrev can be defeated\n> by --abbrev, but --full-index will always win --abbrev.\n\nSure, I wasn't suggesting to flip the flags.full_index bit upon\nseeing \"--no-abbrev\". When --no-abbrev is in effect (i.e. the last\none among --no-abbrev, --abbrev, or --abbrev=n), .abbrev field is\nset to 0. So wouldn't it be sufficient to say\n\n - If flags.full_index bit is set, show the full object name\n\n - If abbrev is 0, show the full object name\n\n - All other cases, after clamping the value of abbrev to reasonable\n value, truncat the object name to that length\n\nWhat am I missing?\n\n> I'm planning for:\n>\n> * BOTH --abbrev and --no-abbrev but NOT --full-index;\n> * BOTH --abbrev AND --full-index\n\n"},{"id":"403667","messageId":"20200814145043.GB9518@danh.dev","threadId":"54009","inReplyTo":"xmqqzh6yyr1t.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 1/2] revision: differentiate if --no-abbrev asked explicitly","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-14T14:50:43Z","receivedAt":"2020-08-14T14:50:49Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-08-13 18:06:22-0700, Junio C Hamano wrote:\n> Đoàn Trần Công Danh writes:\n> \n> > Let me list some combination here:\n> >\n> > * none of --abbrev --no-abbrev --full-index -> default short index\n> > * --abbrev --full-index -> full-index\n> > * --full-index --abbrev -> full-index\n> > * --abbrev --no-abbrev -> full-index\n> > * --no-abbrev --abbrev=[n] -> shortened index to n char\n> >\n> > So, we can't use full_index bit, because --no-abbrev can be defeated\n> > by --abbrev, but --full-index will always win --abbrev.\n> \n> Sure, I wasn't suggesting to flip the flags.full_index bit upon\n> seeing \"--no-abbrev\". When --no-abbrev is in effect (i.e. the last\n> one among --no-abbrev, --abbrev, or --abbrev=n), .abbrev field is\n> set to 0. So wouldn't it be sufficient to say\n> \n> - If flags.full_index bit is set, show the full object name\n> \n> - If abbrev is 0, show the full object name\n> \n> - All other cases, after clamping the value of abbrev to reasonable\n> value, truncat the object name to that length\n> \n> What am I missing?\n\nNo, you didn't miss anything.\n\nIt's obviously me, who screwed up the logical thinking.\n\nOriginally, I come up with something along the line in 2/2:\n\n\tint abbrev = o->flags.full_index ? hexsz : DEFAULT_ABBREV;\n\tif (!o->abbrev)\n\t\tabbrev = o->abbrev;\n\nI couldn't recalled what I wrote, but that logic requires 1/2,\nafter I come up with 1/2, I re-analysed 2/2 and come up with current\nlogic.\n\nI failed to re-visit 1/2 to check if it's necessary.\nIt's all MY fault.\n\nSorry for wasting everyone's time in 1/2.\n\nPlease eject 1/2 from this series.\n\n-- \nThanks\n\nDanh\n"},{"id":"403672","messageId":"20200814151815.GA29528@szeder.dev","threadId":"54009","inReplyTo":"760df7782dad9e9df7bb284ec57249e697a4cc92.1597364493.git.congdanhqx@gmail.com","subject":"Re: [PATCH v3 2/2] diff: index-line: respect --abbrev in object's name","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-08-14T15:18:15Z","receivedAt":"2020-08-14T15:18:26Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Aug 14, 2020 at 07:23:10AM +0700, Đoàn Trần Công Danh wrote:\n> A handful of Git's commands respect `--abbrev' for customizing length\n> of abbreviation of object names.\n> \n> For diff-family, Git supports 2 different options for 2 different\n> purposes, `--full-index' for showing diff-patch object's name in full,\n> and `--abbrev' to customize the length of object names in diff-raw and\n> diff-tree header lines, without any options to customise the length of\n> object names in diff-patch format. When working with diff-patch format,\n> we only have two options, either full index, or default abbrev length.\n> \n> Although, that consistent is documented, it doesn't stop users from\n> trying to use `--abbrev' with the hope of customising diff-patch's\n> objects' name's abbreviation.\n> \n> Let's resolve that inconsistency.\n> \n> To preserve backward compatibility with old script that specify both\n> `--full-index' and `--abbrev', always shows full object id\n> if `--full-index' is specified.\n> \n> Signed-off-by: Đoàn Trần Công Danh \n> ---\n> Documentation/diff-options.txt | 9 +++---\n> diff.c | 5 +++-\n> t/t4013-diff-various.sh | 3 ++\n> ...ff.diff-tree_--root_-p_--abbrev=10_initial | 29 +++++++++++++++++++\n> ...--root_-p_--full-index_--abbrev=10_initial | 29 +++++++++++++++++++\n> ...f.diff-tree_--root_-p_--full-index_initial | 29 +++++++++++++++++++\n> 6 files changed, 99 insertions(+), 5 deletions(-)\n> create mode 100644 t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n> create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n> create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n> \n> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n> index 7987d72b02..c11efa7865 100644\n> --- a/Documentation/diff-options.txt\n> +++ b/Documentation/diff-options.txt\n> @@ -441,10 +441,11 @@ endif::git-format-patch[]\n> --abbrev[=]::\n> \tInstead of showing the full 40-byte hexadecimal object\n> \tname in diff-raw format output and diff-tree header\n> -\tlines, show only a partial prefix. This is\n> -\tindependent of the `--full-index` option above, which controls\n> -\tthe diff-patch output format. Non default number of\n> -\tdigits can be specified with `--abbrev=`.\n> +\tlines, show only a partial prefix.\n> +\tIn diff-patch output format, `--full-index` takes higher\n> +\tprecedent, i.e. if `--full-index` is specified, full blob\n> +\tnames will be shown regardless of `--abbrev`.\n> +\tNon default number of digits can be specified with `--abbrev=`.\n> \n> -B[][/]::\n> --break-rewrites[=[][/]]::\n> diff --git a/diff.c b/diff.c\n> index f9709de7b4..20dedfe2a9 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -4319,7 +4319,10 @@ static void fill_metainfo(struct strbuf *msg,\n> \t}\n> \tif (one && two && !oideq(&one->oid, &two->oid)) {\n> \t\tconst unsigned hexsz = the_hash_algo->hexsz;\n> -\t\tint abbrev = o->flags.full_index ? hexsz : DEFAULT_ABBREV;\n> +\t\tint abbrev = o->abbrev ? o->abbrev : DEFAULT_ABBREV;\n> +\n> +\t\tif (o->flags.full_index)\n> +\t\t\tabbrev = hexsz;\n> \n> \t\tif (o->flags.binary) {\n> \t\t\tmmfile_t mf;\n> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\n> index 3f60f7d96c..e6eb4dd4c7 100755\n> --- a/t/t4013-diff-various.sh\n> +++ b/t/t4013-diff-various.sh\n> @@ -221,6 +221,9 @@ diff-tree --root -r --abbrev=4 initial\n> :noellipses diff-tree --root -r --abbrev=4 initial\n> diff-tree -p initial\n> diff-tree --root -p initial\n> +diff-tree --root -p --abbrev=10 initial\n> +diff-tree --root -p --full-index initial\n> +diff-tree --root -p --full-index --abbrev=10 initial\n> diff-tree --patch-with-stat initial\n> diff-tree --root --patch-with-stat initial\n> diff-tree --patch-with-raw initial\n> diff --git a/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial b/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n> new file mode 100644\n> index 0000000000..7518a9044e\n> --- /dev/null\n> +++ b/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n> @@ -0,0 +1,29 @@\n> +$ git diff-tree --root -p --abbrev=10 initial\n> +444ac553ac7612cc88969031b02b3767fb8a353a\n> +diff --git a/dir/sub b/dir/sub\n> +new file mode 100644\n> +index 0000000000..35d242ba79\n> +--- /dev/null\n> ++++ b/dir/sub\n> +@@ -0,0 +1,2 @@\n> ++A\n> ++B\n> +diff --git a/file0 b/file0\n> +new file mode 100644\n> +index 0000000000..01e79c32a8\n> +--- /dev/null\n> ++++ b/file0\n> +@@ -0,0 +1,3 @@\n> ++1\n> ++2\n> ++3\n> +diff --git a/file2 b/file2\n> +new file mode 100644\n> +index 0000000000..01e79c32a8\n> +--- /dev/null\n> ++++ b/file2\n> +@@ -0,0 +1,3 @@\n> ++1\n> ++2\n> ++3\n> +$\n> diff --git a/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial b/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n> new file mode 100644\n> index 0000000000..69f913fbe5\n> --- /dev/null\n> +++ b/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n> @@ -0,0 +1,29 @@\n> +$ git diff-tree --root -p --full-index --abbrev=10 initial\n> +444ac553ac7612cc88969031b02b3767fb8a353a\n> +diff --git a/dir/sub b/dir/sub\n> +new file mode 100644\n> +index 0000000000000000000000000000000000000000..35d242ba79ae89ac695e26b3d4c27a8e6f028f9e\n> +--- /dev/null\n> ++++ b/dir/sub\n> +@@ -0,0 +1,2 @@\n> ++A\n> ++B\n> +diff --git a/file0 b/file0\n> +new file mode 100644\n> +index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n> +--- /dev/null\n> ++++ b/file0\n> +@@ -0,0 +1,3 @@\n> ++1\n> ++2\n> ++3\n> +diff --git a/file2 b/file2\n> +new file mode 100644\n> +index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n> +--- /dev/null\n> ++++ b/file2\n> +@@ -0,0 +1,3 @@\n> ++1\n> ++2\n> ++3\n> +$\n> diff --git a/t/t4013/diff.diff-tree_--root_-p_--full-index_initial b/t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n> new file mode 100644\n> index 0000000000..1b0b6717fa\n> --- /dev/null\n> +++ b/t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n> @@ -0,0 +1,29 @@\n> +$ git diff-tree --root -p --full-index initial\n> +444ac553ac7612cc88969031b02b3767fb8a353a\n> +diff --git a/dir/sub b/dir/sub\n> +new file mode 100644\n> +index 0000000000000000000000000000000000000000..35d242ba79ae89ac695e26b3d4c27a8e6f028f9e\n> +--- /dev/null\n> ++++ b/dir/sub\n> +@@ -0,0 +1,2 @@\n> ++A\n> ++B\n> +diff --git a/file0 b/file0\n> +new file mode 100644\n> +index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n> +--- /dev/null\n> ++++ b/file0\n> +@@ -0,0 +1,3 @@\n> ++1\n> ++2\n> ++3\n> +diff --git a/file2 b/file2\n> +new file mode 100644\n> +index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n> +--- /dev/null\n> ++++ b/file2\n> +@@ -0,0 +1,3 @@\n> ++1\n> ++2\n> ++3\n> +$\n\nAll these new tests break when run with GIT_TEST_DEFAULT_HASH=sha256\nwith something like:\n\n + test_cmp expect actual\n --- expect\t2020-08-14 15:05:12.209285397 +0000\n +++ actual\t2020-08-14 15:05:12.205285279 +0000\n @@ -2,7 +2,7 @@\n 0000000000000000000000000000000000000000000000000000000000000000\n diff --git a/dir/sub b/dir/sub\n new file mode ffff44\n -index ffffffffff..fffffffa79\n +index ffffffffff..fffffff895\n --- /dev/null\n +++ b/dir/sub\n @@ -0,0 +1,2 @@\n @@ -10,7 +10,7 @@\n +B\n diff --git a/file0 b/file0\n new file mode ffff44\n -index ffffffffff..fffffff2a8\n +index ffffffffff..fffffff5d7\n --- /dev/null\n +++ b/file0\n @@ -0,0 +1,3 @@\n @@ -19,7 +19,7 @@\n +3\n diff --git a/file2 b/file2\n new file mode ffff44\n -index ffffffffff..fffffff2a8\n +index ffffffffff..fffffff5d7\n --- /dev/null\n +++ b/file2\n @@ -0,0 +1,3 @@\n error: last command exited with $?=1\n\n\n"},{"id":"403686","messageId":"xmqqwo21xiuy.fsf@gitster.c.googlers.com","threadId":"54009","inReplyTo":"20200814151815.GA29528@szeder.dev","subject":"Re: [PATCH v3 2/2] diff: index-line: respect --abbrev in object's name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-14T17:00:53Z","receivedAt":"2020-08-14T17:01:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n>> +diff --git a/file0 b/file0\n>> +new file mode 100644\n>> +index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n>> +--- /dev/null\n>> ++++ b/file0\n>> +@@ -0,0 +1,3 @@\n>> ++1\n>> ++2\n>> ++3\n>> +diff --git a/file2 b/file2\n>> +new file mode 100644\n>> +index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n>> +--- /dev/null\n>> ++++ b/file2\n>> +@@ -0,0 +1,3 @@\n>> ++1\n>> ++2\n>> ++3\n>> +$\n>\n> All these new tests break when run with GIT_TEST_DEFAULT_HASH=sha256\n> with something like:\n>\n> + test_cmp expect actual\n> --- expect\t2020-08-14 15:05:12.209285397 +0000\n> +++ actual\t2020-08-14 15:05:12.205285279 +0000\n> @@ -2,7 +2,7 @@\n> 0000000000000000000000000000000000000000000000000000000000000000\n> diff --git a/dir/sub b/dir/sub\n> new file mode ffff44\n> -index ffffffffff..fffffffa79\n> +index ffffffffff..fffffff895\n\nOuch. These apparently come from\n\nprocess_diffs () {\n\t_x04=\"[0-9a-f][0-9a-f][0-9a-f][0-9a-f]\" &&\n\t_x07=\"$_x05[0-9a-f][0-9a-f]\" &&\n\tsed -e \"s/$OID_REGEX/$ZERO_OID/g\" \\\n\t -e \"s/From $_x40 /From $ZERO_OID /\" \\\n\t -e \"s/from $_x40)/from $ZERO_OID)/\" \\\n\t -e \"s/commit $_x40\\$/commit $ZERO_OID/\" \\\n\t -e \"s/commit $_x40 (/commit $ZERO_OID (/\" \\\n\t -e \"s/$_x40 $_x40 $_x40/$ZERO_OID $ZERO_OID $ZERO_OID/\" \\\n\t -e \"s/$_x40 $_x40 /$ZERO_OID $ZERO_OID /\" \\\n\t -e \"s/^$_x40 $_x40$/$ZERO_OID $ZERO_OID/\" \\\n\t -e \"s/^$_x40 /$ZERO_OID /\" \\\n\t -e \"s/^$_x40$/$ZERO_OID/\" \\\n\t -e \"s/$_x07\\.\\.$_x07/fffffff..fffffff/g\" \\\n\t -e \"s/$_x07,$_x07\\.\\.$_x07/fffffff,fffffff..fffffff/g\" \\\n\t -e \"s/$_x07 $_x07 $_x07/fffffff fffffff fffffff/g\" \\\n\t -e \"s/$_x07 $_x07 /fffffff fffffff /g\" \\\n\t -e \"s/Merge: $_x07 $_x07/Merge: fffffff fffffff/g\" \\\n\t -e \"s/$_x07\\.\\.\\./fffffff.../g\" \\\n\t -e \"s/ $_x04\\.\\.\\./ ffff.../g\" \\\n\t -e \"s/ $_x04/ ffff/g\" \\\n\t \"$1\"\n}\n\nHash-independence may be good, but it should not munge expected mode\nbits from 100644 to ffff44, which I think is a bug in the original\nintroduced in 72f936b1 (t4013: make test hash independent,\n2020-02-07).\n\nWhen we are adjusting the abbrev length of the index line, of course\n$_x07 would not be sufficient to match the abbreviated object name\nin full, so a79 vs 895 can be explained and is a bug in this patch\nthat did not update the process_diffs helper.\n\nAnother thing that I find somewhat problematic in the original\n(brian cc'ed) is that it does not special case all-zero object name\nspecially. By turning any and all instances of $_x40 to $ZERO_OID,\nwe lose the distinction between a random-looking object name which\ngot turned into $ZERO_OID by the processing, and an object name that\nwas $ZERO_OID from the beginning, so we won't catch a possible\nfuture bug where new file's preimage object name is not $ZERO_OID\n(this is plausible when you try to show an intent-to-add entry; the\ndiff between the index and the working tree would be \"new file\"\npatch, but the index entry records the object name for an empty\nblob, e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 instead of $ZERO_OID\ncan easily be emitted by a mistaken implementation).\n"},{"id":"403697","messageId":"xmqqy2mhvys6.fsf@gitster.c.googlers.com","threadId":"54009","inReplyTo":"xmqqwo21xiuy.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 2/2] diff: index-line: respect --abbrev in object's name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-14T18:59:53Z","receivedAt":"2020-08-14T18:59:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ouch. These apparently come from\n>\n> process_diffs () {\n> ...\n> }\n>\n> Hash-independence may be good, but it should not munge expected mode\n> bits from 100644 to ffff44, which I think is a bug in the original\n> introduced in 72f936b1 (t4013: make test hash independent,\n> 2020-02-07).\n>\n> When we are adjusting the abbrev length of the index line, of course\n> $_x07 would not be sufficient to match the abbreviated object name\n> in full, so a79 vs 895 can be explained and is a bug in this patch\n> that did not update the process_diffs helper.\n>\n> Another thing that I find somewhat problematic in the original\n> (brian cc'ed) is that it does not special case all-zero object name\n> specially. By turning any and all instances of $_x40 to $ZERO_OID,\n> we lose the distinction between a random-looking object name which\n> got turned into $ZERO_OID by the processing, and an object name that\n> was $ZERO_OID from the beginning, so we won't catch a possible\n> future bug where new file's preimage object name is not $ZERO_OID\n> (this is plausible when you try to show an intent-to-add entry; the\n> diff between the index and the working tree would be \"new file\"\n> patch, but the index entry records the object name for an empty\n> blob, e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 instead of $ZERO_OID\n> can easily be emitted by a mistaken implementation).\n\nSo here is what I came up with as a possible starting point. The\nidea is to grab hexadecimal strings at locations the original tried\nto isolate with various contexts, and\n\n - if the input happens to be all zero, use '0', otherwise use 'f'\n\n - if the input is 40-bytes (i.e. unabbreviated object name in the\n SHA-1 world), repeat the character chosen in the first step as\n many times as there are chars in $ZERO_OID\n\n - otherwise, repeat the character chosen in the first step as many\n times as there are chars in the input.\n\n - regardless of all of the above, special case possible in-tree\n blob modes (100644, 100755 and 120000) and don't munge them.\n\nI haven't tried it with the patch that started this discussion\nthread, nor with SHA-256 build, though.\n\n\n t/t4013-diff-various.sh | 58 +++++++++++++++++++++++++++++++------------------\n 1 file changed, 37 insertions(+), 21 deletions(-)\n\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex 43267d6024..b33e60ab9d 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -130,27 +130,43 @@ test_expect_success setup '\n EOF\n \n process_diffs () {\n-\t_x04=\"[0-9a-f][0-9a-f][0-9a-f][0-9a-f]\" &&\n-\t_x07=\"$_x05[0-9a-f][0-9a-f]\" &&\n-\tsed -e \"s/$OID_REGEX/$ZERO_OID/g\" \\\n-\t -e \"s/From $_x40 /From $ZERO_OID /\" \\\n-\t -e \"s/from $_x40)/from $ZERO_OID)/\" \\\n-\t -e \"s/commit $_x40\\$/commit $ZERO_OID/\" \\\n-\t -e \"s/commit $_x40 (/commit $ZERO_OID (/\" \\\n-\t -e \"s/$_x40 $_x40 $_x40/$ZERO_OID $ZERO_OID $ZERO_OID/\" \\\n-\t -e \"s/$_x40 $_x40 /$ZERO_OID $ZERO_OID /\" \\\n-\t -e \"s/^$_x40 $_x40$/$ZERO_OID $ZERO_OID/\" \\\n-\t -e \"s/^$_x40 /$ZERO_OID /\" \\\n-\t -e \"s/^$_x40$/$ZERO_OID/\" \\\n-\t -e \"s/$_x07\\.\\.$_x07/fffffff..fffffff/g\" \\\n-\t -e \"s/$_x07,$_x07\\.\\.$_x07/fffffff,fffffff..fffffff/g\" \\\n-\t -e \"s/$_x07 $_x07 $_x07/fffffff fffffff fffffff/g\" \\\n-\t -e \"s/$_x07 $_x07 /fffffff fffffff /g\" \\\n-\t -e \"s/Merge: $_x07 $_x07/Merge: fffffff fffffff/g\" \\\n-\t -e \"s/$_x07\\.\\.\\./fffffff.../g\" \\\n-\t -e \"s/ $_x04\\.\\.\\./ ffff.../g\" \\\n-\t -e \"s/ $_x04/ ffff/g\" \\\n-\t \"$1\"\n+\tperl -e '\n+\t\tmy $oid_length = length($ARGV[0]);\n+\t\tmy $x40 = \"[0-9a-f]{40}\";\n+\t\tmy $xab = \"[0-9a-f]{4,16}\";\n+\t\tmy $orx = \"[0-9a-f]\" x $oid_length;\n+\n+\t\tsub munge_oid {\n+\t\t\tmy ($oid) = @_;\n+\t\t\tmy $x;\n+\n+\t\t\tif ($oid =~ /^(100644|100755|120000)$/) {\n+\t\t\t\treturn $oid;\n+\t\t\t}\n+\n+\t\t\tif ($oid =~ /^0*$/) {\n+\t\t\t\t$x = \"0\";\n+\t\t\t} else {\n+\t\t\t\t$x = \"f\";\n+\t\t\t}\n+\n+\t\t\tif (length($oid) == 40) {\n+\t\t\t\treturn $x x $oid_length;\n+\t\t\t} else {\n+\t\t\t\treturn $x x length($oid);\n+\t\t\t}\n+\t\t}\n+\n+\t\twhile () {\n+\t\t\ts/($orx)/munge_oid($1)/ge;\n+\t\t\ts/From ($x40)( |\\))/\"From \" . munge_oid($1) . $2/ge;\n+\t\t\ts/commit ($x40)($| \\()/\"commit \" . munge_oid($1) . $2/ge;\n+\t\t\ts/($x40) /munge_oid($1) . \" \"/ge;\n+\t\t\ts/^($x40)($| )/munge_oid($1) . $2/e;\n+\t\t\ts/($xab)(\\.\\.|,| |\\.\\.\\.|$)/munge_oid($1) . $2/ge;\n+\t\t\tprint;\n+\t\t}\n+\t' \"$ZERO_OID\" <\"$1\"\n }\n \n V=$(git version | sed -e 's/^git version //' -e 's/\\./\\\\./g')\n"},{"id":"403732","messageId":"20200815002120.GQ8085@camp.crustytoothpaste.net","threadId":"54009","inReplyTo":"xmqqy2mhvys6.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 2/2] diff: index-line: respect --abbrev in object's name","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-08-15T00:21:20Z","receivedAt":"2020-08-15T00:21:30Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-08-14 at 18:59:53, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n> > Ouch. These apparently come from\n> >\n> > process_diffs () {\n> > ...\n> > }\n> >\n> > Hash-independence may be good, but it should not munge expected mode\n> > bits from 100644 to ffff44, which I think is a bug in the original\n> > introduced in 72f936b1 (t4013: make test hash independent,\n> > 2020-02-07).\n> >\n> > When we are adjusting the abbrev length of the index line, of course\n> > $_x07 would not be sufficient to match the abbreviated object name\n> > in full, so a79 vs 895 can be explained and is a bug in this patch\n> > that did not update the process_diffs helper.\n> >\n> > Another thing that I find somewhat problematic in the original\n> > (brian cc'ed) is that it does not special case all-zero object name\n> > specially. By turning any and all instances of $_x40 to $ZERO_OID,\n> > we lose the distinction between a random-looking object name which\n> > got turned into $ZERO_OID by the processing, and an object name that\n> > was $ZERO_OID from the beginning, so we won't catch a possible\n> > future bug where new file's preimage object name is not $ZERO_OID\n> > (this is plausible when you try to show an intent-to-add entry; the\n> > diff between the index and the working tree would be \"new file\"\n> > patch, but the index entry records the object name for an empty\n> > blob, e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 instead of $ZERO_OID\n> > can easily be emitted by a mistaken implementation).\n\nYeah, it wasn't intended that we munge mode bits, and I definitely agree\nwe'd be better off distinguishing between all-zero and non-zero OIDs.\n\nAs you might imagine, this is not my favorite test because have a large\namount of diff formats, and even I think the giant list of sed\nexpressions I wrote is hideous. It is, however, reasonably\ncomprehensive, which is pretty much the only nice thing that can be said\nabout it.\n\n> So here is what I came up with as a possible starting point. The\n> idea is to grab hexadecimal strings at locations the original tried\n> to isolate with various contexts, and\n> \n> - if the input happens to be all zero, use '0', otherwise use 'f'\n> \n> - if the input is 40-bytes (i.e. unabbreviated object name in the\n> SHA-1 world), repeat the character chosen in the first step as\n> many times as there are chars in $ZERO_OID\n> \n> - otherwise, repeat the character chosen in the first step as many\n> times as there are chars in the input.\n> \n> - regardless of all of the above, special case possible in-tree\n> blob modes (100644, 100755 and 120000) and don't munge them.\n> \n> I haven't tried it with the patch that started this discussion\n> thread, nor with SHA-256 build, though.\n\nThis seems like a sane approach.\n\n> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\n> index 43267d6024..b33e60ab9d 100755\n> --- a/t/t4013-diff-various.sh\n> +++ b/t/t4013-diff-various.sh\n> @@ -130,27 +130,43 @@ test_expect_success setup '\n> EOF\n> \n> process_diffs () {\n> -\t_x04=\"[0-9a-f][0-9a-f][0-9a-f][0-9a-f]\" &&\n> -\t_x07=\"$_x05[0-9a-f][0-9a-f]\" &&\n> -\tsed -e \"s/$OID_REGEX/$ZERO_OID/g\" \\\n> -\t -e \"s/From $_x40 /From $ZERO_OID /\" \\\n> -\t -e \"s/from $_x40)/from $ZERO_OID)/\" \\\n> -\t -e \"s/commit $_x40\\$/commit $ZERO_OID/\" \\\n> -\t -e \"s/commit $_x40 (/commit $ZERO_OID (/\" \\\n> -\t -e \"s/$_x40 $_x40 $_x40/$ZERO_OID $ZERO_OID $ZERO_OID/\" \\\n> -\t -e \"s/$_x40 $_x40 /$ZERO_OID $ZERO_OID /\" \\\n> -\t -e \"s/^$_x40 $_x40$/$ZERO_OID $ZERO_OID/\" \\\n> -\t -e \"s/^$_x40 /$ZERO_OID /\" \\\n> -\t -e \"s/^$_x40$/$ZERO_OID/\" \\\n> -\t -e \"s/$_x07\\.\\.$_x07/fffffff..fffffff/g\" \\\n> -\t -e \"s/$_x07,$_x07\\.\\.$_x07/fffffff,fffffff..fffffff/g\" \\\n> -\t -e \"s/$_x07 $_x07 $_x07/fffffff fffffff fffffff/g\" \\\n> -\t -e \"s/$_x07 $_x07 /fffffff fffffff /g\" \\\n> -\t -e \"s/Merge: $_x07 $_x07/Merge: fffffff fffffff/g\" \\\n> -\t -e \"s/$_x07\\.\\.\\./fffffff.../g\" \\\n> -\t -e \"s/ $_x04\\.\\.\\./ ffff.../g\" \\\n> -\t -e \"s/ $_x04/ ffff/g\" \\\n> -\t \"$1\"\n> +\tperl -e '\n> +\t\tmy $oid_length = length($ARGV[0]);\n> +\t\tmy $x40 = \"[0-9a-f]{40}\";\n> +\t\tmy $xab = \"[0-9a-f]{4,16}\";\n> +\t\tmy $orx = \"[0-9a-f]\" x $oid_length;\n> +\n> +\t\tsub munge_oid {\n> +\t\t\tmy ($oid) = @_;\n> +\t\t\tmy $x;\n> +\n> +\t\t\tif ($oid =~ /^(100644|100755|120000)$/) {\n> +\t\t\t\treturn $oid;\n> +\t\t\t}\n> +\n> +\t\t\tif ($oid =~ /^0*$/) {\n> +\t\t\t\t$x = \"0\";\n> +\t\t\t} else {\n> +\t\t\t\t$x = \"f\";\n> +\t\t\t}\n> +\n> +\t\t\tif (length($oid) == 40) {\n> +\t\t\t\treturn $x x $oid_length;\n> +\t\t\t} else {\n> +\t\t\t\treturn $x x length($oid);\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\twhile () {\n> +\t\t\ts/($orx)/munge_oid($1)/ge;\n> +\t\t\ts/From ($x40)( |\\))/\"From \" . munge_oid($1) . $2/ge;\n> +\t\t\ts/commit ($x40)($| \\()/\"commit \" . munge_oid($1) . $2/ge;\n> +\t\t\ts/($x40) /munge_oid($1) . \" \"/ge;\n> +\t\t\ts/^($x40)($| )/munge_oid($1) . $2/e;\n> +\t\t\ts/($xab)(\\.\\.|,| |\\.\\.\\.|$)/munge_oid($1) . $2/ge;\n> +\t\t\tprint;\n> +\t\t}\n> +\t' \"$ZERO_OID\" <\"$1\"\n> }\n\nThis is much nicer, but I think we need the following on top of it\nbecause we have a couple of tricky cases the original didn't consider:\n\n* Some of our 64-bit object IDs get processed twice, turning them into\n 88-character object IDs, which don't match. We therefore need \"\\b\".\n* The new unabbreviated index lines aren't accounted for, so I included\n them by possibly matching \"\\.\\.\".\n* We have some lines that look like \"commit $OID (from $OID)\" that\n aren't accounted for. Because we now have an optional OID in\n munge_oid, I had to account for that as well.\n\nSo this is what's on top:\n\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex b5c7e1a63b..dfc87a0d19 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -140,6 +140,8 @@ process_diffs () {\n \t\t\tmy ($oid) = @_;\n \t\t\tmy $x;\n \n+\t\t\treturn \"\" unless length $oid;\n+\n \t\t\tif ($oid =~ /^(100644|100755|120000)$/) {\n \t\t\t\treturn $oid;\n \t\t\t}\n@@ -160,8 +162,8 @@ process_diffs () {\n \t\twhile () {\n \t\t\ts/($orx)/munge_oid($1)/ge;\n \t\t\ts/From ($x40)( |\\))/\"From \" . munge_oid($1) . $2/ge;\n-\t\t\ts/commit ($x40)($| \\()/\"commit \" . munge_oid($1) . $2/ge;\n-\t\t\ts/($x40) /munge_oid($1) . \" \"/ge;\n+\t\t\ts/commit ($x40)($| \\(from )($x40?)/\"commit \" . munge_oid($1) . $2 . munge_oid($3)/ge;\n+\t\t\ts/\\b($x40)( |\\.\\.|$)/munge_oid($1) . $2/ge;\n \t\t\ts/^($x40)($| )/munge_oid($1) . $2/e;\n \t\t\ts/($xab)(\\.\\.|,| |\\.\\.\\.|$)/munge_oid($1) . $2/ge;\n \t\t\tprint;\n\nOr, a fresh original version:\n\n-- %< --\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex e6eb4dd4c7..dfc87a0d19 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -130,27 +130,45 @@ test_expect_success setup '\n EOF\n \n process_diffs () {\n-\t_x04=\"[0-9a-f][0-9a-f][0-9a-f][0-9a-f]\" &&\n-\t_x07=\"$_x05[0-9a-f][0-9a-f]\" &&\n-\tsed -e \"s/$OID_REGEX/$ZERO_OID/g\" \\\n-\t -e \"s/From $_x40 /From $ZERO_OID /\" \\\n-\t -e \"s/from $_x40)/from $ZERO_OID)/\" \\\n-\t -e \"s/commit $_x40\\$/commit $ZERO_OID/\" \\\n-\t -e \"s/commit $_x40 (/commit $ZERO_OID (/\" \\\n-\t -e \"s/$_x40 $_x40 $_x40/$ZERO_OID $ZERO_OID $ZERO_OID/\" \\\n-\t -e \"s/$_x40 $_x40 /$ZERO_OID $ZERO_OID /\" \\\n-\t -e \"s/^$_x40 $_x40$/$ZERO_OID $ZERO_OID/\" \\\n-\t -e \"s/^$_x40 /$ZERO_OID /\" \\\n-\t -e \"s/^$_x40$/$ZERO_OID/\" \\\n-\t -e \"s/$_x07\\.\\.$_x07/fffffff..fffffff/g\" \\\n-\t -e \"s/$_x07,$_x07\\.\\.$_x07/fffffff,fffffff..fffffff/g\" \\\n-\t -e \"s/$_x07 $_x07 $_x07/fffffff fffffff fffffff/g\" \\\n-\t -e \"s/$_x07 $_x07 /fffffff fffffff /g\" \\\n-\t -e \"s/Merge: $_x07 $_x07/Merge: fffffff fffffff/g\" \\\n-\t -e \"s/$_x07\\.\\.\\./fffffff.../g\" \\\n-\t -e \"s/ $_x04\\.\\.\\./ ffff.../g\" \\\n-\t -e \"s/ $_x04/ ffff/g\" \\\n-\t \"$1\"\n+\tperl -e '\n+\t\tmy $oid_length = length($ARGV[0]);\n+\t\tmy $x40 = \"[0-9a-f]{40}\";\n+\t\tmy $xab = \"[0-9a-f]{4,16}\";\n+\t\tmy $orx = \"[0-9a-f]\" x $oid_length;\n+\n+\t\tsub munge_oid {\n+\t\t\tmy ($oid) = @_;\n+\t\t\tmy $x;\n+\n+\t\t\treturn \"\" unless length $oid;\n+\n+\t\t\tif ($oid =~ /^(100644|100755|120000)$/) {\n+\t\t\t\treturn $oid;\n+\t\t\t}\n+\n+\t\t\tif ($oid =~ /^0*$/) {\n+\t\t\t\t$x = \"0\";\n+\t\t\t} else {\n+\t\t\t\t$x = \"f\";\n+\t\t\t}\n+\n+\t\t\tif (length($oid) == 40) {\n+\t\t\t\treturn $x x $oid_length;\n+\t\t\t} else {\n+\t\t\t\treturn $x x length($oid);\n+\t\t\t}\n+\t\t}\n+\n+\t\twhile () {\n+\t\t\ts/($orx)/munge_oid($1)/ge;\n+\t\t\ts/From ($x40)( |\\))/\"From \" . munge_oid($1) . $2/ge;\n+\t\t\ts/commit ($x40)($| \\(from )($x40?)/\"commit \" . munge_oid($1) . $2 . munge_oid($3)/ge;\n+\t\t\ts/\\b($x40)( |\\.\\.|$)/munge_oid($1) . $2/ge;\n+\t\t\ts/^($x40)($| )/munge_oid($1) . $2/e;\n+\t\t\ts/($xab)(\\.\\.|,| |\\.\\.\\.|$)/munge_oid($1) . $2/ge;\n+\t\t\tprint;\n+\t\t}\n+\t' \"$ZERO_OID\" <\"$1\"\n }\n \n V=$(git version | sed -e 's/^git version //' -e 's/\\./\\\\./g')\n-- %< --\n\nAnyone is welcome to have my sign-off if they pick up any part of this\npatch.\n-- \nbrian m. carlson: Houston, Texas, US\n"},{"id":"403753","messageId":"20200815022759.GC12363@danh.dev","threadId":"54009","inReplyTo":"20200815002120.GQ8085@camp.crustytoothpaste.net","subject":"Re: [PATCH v3 2/2] diff: index-line: respect --abbrev in object's name","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-15T02:27:59Z","receivedAt":"2020-08-15T21:57:41Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-08-15 00:21:20+0000, \"brian m. carlson\" wrote:\n> On 2020-08-14 at 18:59:53, Junio C Hamano wrote:\n> > Junio C Hamano writes:\n> > \n> > > Ouch. These apparently come from\n> > >\n> > > process_diffs () {\n> > > ...\n> > > }\n> > >\n> > > Hash-independence may be good, but it should not munge expected mode\n> > > bits from 100644 to ffff44, which I think is a bug in the original\n> > > introduced in 72f936b1 (t4013: make test hash independent,\n> > > 2020-02-07).\n> > >\n> > > When we are adjusting the abbrev length of the index line, of course\n> > > $_x07 would not be sufficient to match the abbreviated object name\n> > > in full, so a79 vs 895 can be explained and is a bug in this patch\n> > > that did not update the process_diffs helper.\n> > >\n> > > Another thing that I find somewhat problematic in the original\n> > > (brian cc'ed) is that it does not special case all-zero object name\n> > > specially. By turning any and all instances of $_x40 to $ZERO_OID,\n> > > we lose the distinction between a random-looking object name which\n> > > got turned into $ZERO_OID by the processing, and an object name that\n> > > was $ZERO_OID from the beginning, so we won't catch a possible\n> > > future bug where new file's preimage object name is not $ZERO_OID\n> > > (this is plausible when you try to show an intent-to-add entry; the\n> > > diff between the index and the working tree would be \"new file\"\n> > > patch, but the index entry records the object name for an empty\n> > > blob, e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 instead of $ZERO_OID\n> > > can easily be emitted by a mistaken implementation).\n> \n> Yeah, it wasn't intended that we munge mode bits, and I definitely agree\n> we'd be better off distinguishing between all-zero and non-zero OIDs.\n> \n> As you might imagine, this is not my favorite test because have a large\n> amount of diff formats, and even I think the giant list of sed\n> expressions I wrote is hideous. It is, however, reasonably\n> comprehensive, which is pretty much the only nice thing that can be said\n> about it.\n> \n> > So here is what I came up with as a possible starting point. The\n> > idea is to grab hexadecimal strings at locations the original tried\n> > to isolate with various contexts, and\n> > \n> > - if the input happens to be all zero, use '0', otherwise use 'f'\n> > \n> > - if the input is 40-bytes (i.e. unabbreviated object name in the\n> > SHA-1 world), repeat the character chosen in the first step as\n> > many times as there are chars in $ZERO_OID\n> > \n> > - otherwise, repeat the character chosen in the first step as many\n> > times as there are chars in the input.\n> > \n> > - regardless of all of the above, special case possible in-tree\n> > blob modes (100644, 100755 and 120000) and don't munge them.\n> > \n> > I haven't tried it with the patch that started this discussion\n> > thread, nor with SHA-256 build, though.\n> \n> This seems like a sane approach.\n> \n> > diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\n> > index 43267d6024..b33e60ab9d 100755\n> > --- a/t/t4013-diff-various.sh\n> > +++ b/t/t4013-diff-various.sh\n> > @@ -130,27 +130,43 @@ test_expect_success setup '\n> > EOF\n> > \n> > process_diffs () {\n> > -\t_x04=\"[0-9a-f][0-9a-f][0-9a-f][0-9a-f]\" &&\n> > -\t_x07=\"$_x05[0-9a-f][0-9a-f]\" &&\n> > -\tsed -e \"s/$OID_REGEX/$ZERO_OID/g\" \\\n> > -\t -e \"s/From $_x40 /From $ZERO_OID /\" \\\n> > -\t -e \"s/from $_x40)/from $ZERO_OID)/\" \\\n> > -\t -e \"s/commit $_x40\\$/commit $ZERO_OID/\" \\\n> > -\t -e \"s/commit $_x40 (/commit $ZERO_OID (/\" \\\n> > -\t -e \"s/$_x40 $_x40 $_x40/$ZERO_OID $ZERO_OID $ZERO_OID/\" \\\n> > -\t -e \"s/$_x40 $_x40 /$ZERO_OID $ZERO_OID /\" \\\n> > -\t -e \"s/^$_x40 $_x40$/$ZERO_OID $ZERO_OID/\" \\\n> > -\t -e \"s/^$_x40 /$ZERO_OID /\" \\\n> > -\t -e \"s/^$_x40$/$ZERO_OID/\" \\\n> > -\t -e \"s/$_x07\\.\\.$_x07/fffffff..fffffff/g\" \\\n> > -\t -e \"s/$_x07,$_x07\\.\\.$_x07/fffffff,fffffff..fffffff/g\" \\\n> > -\t -e \"s/$_x07 $_x07 $_x07/fffffff fffffff fffffff/g\" \\\n> > -\t -e \"s/$_x07 $_x07 /fffffff fffffff /g\" \\\n> > -\t -e \"s/Merge: $_x07 $_x07/Merge: fffffff fffffff/g\" \\\n> > -\t -e \"s/$_x07\\.\\.\\./fffffff.../g\" \\\n> > -\t -e \"s/ $_x04\\.\\.\\./ ffff.../g\" \\\n> > -\t -e \"s/ $_x04/ ffff/g\" \\\n> > -\t \"$1\"\n> > +\tperl -e '\n> > +\t\tmy $oid_length = length($ARGV[0]);\n> > +\t\tmy $x40 = \"[0-9a-f]{40}\";\n> > +\t\tmy $xab = \"[0-9a-f]{4,16}\";\n> > +\t\tmy $orx = \"[0-9a-f]\" x $oid_length;\n> > +\n> > +\t\tsub munge_oid {\n> > +\t\t\tmy ($oid) = @_;\n> > +\t\t\tmy $x;\n> > +\n> > +\t\t\tif ($oid =~ /^(100644|100755|120000)$/) {\n> > +\t\t\t\treturn $oid;\n> > +\t\t\t}\n> > +\n> > +\t\t\tif ($oid =~ /^0*$/) {\n> > +\t\t\t\t$x = \"0\";\n> > +\t\t\t} else {\n> > +\t\t\t\t$x = \"f\";\n> > +\t\t\t}\n> > +\n> > +\t\t\tif (length($oid) == 40) {\n> > +\t\t\t\treturn $x x $oid_length;\n> > +\t\t\t} else {\n> > +\t\t\t\treturn $x x length($oid);\n> > +\t\t\t}\n> > +\t\t}\n> > +\n> > +\t\twhile () {\n> > +\t\t\ts/($orx)/munge_oid($1)/ge;\n> > +\t\t\ts/From ($x40)( |\\))/\"From \" . munge_oid($1) . $2/ge;\n> > +\t\t\ts/commit ($x40)($| \\()/\"commit \" . munge_oid($1) . $2/ge;\n> > +\t\t\ts/($x40) /munge_oid($1) . \" \"/ge;\n> > +\t\t\ts/^($x40)($| )/munge_oid($1) . $2/e;\n> > +\t\t\ts/($xab)(\\.\\.|,| |\\.\\.\\.|$)/munge_oid($1) . $2/ge;\n> > +\t\t\tprint;\n> > +\t\t}\n> > +\t' \"$ZERO_OID\" <\"$1\"\n> > }\n> \n> This is much nicer, but I think we need the following on top of it\n> because we have a couple of tricky cases the original didn't consider:\n> \n> * Some of our 64-bit object IDs get processed twice, turning them into\n> 88-character object IDs, which don't match. We therefore need \"\\b\".\n> * The new unabbreviated index lines aren't accounted for, so I included\n> them by possibly matching \"\\.\\.\".\n> * We have some lines that look like \"commit $OID (from $OID)\" that\n> aren't accounted for. Because we now have an optional OID in\n> munge_oid, I had to account for that as well.\n> \n> So this is what's on top:\n> \n> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\n> index b5c7e1a63b..dfc87a0d19 100755\n> --- a/t/t4013-diff-various.sh\n> +++ b/t/t4013-diff-various.sh\n> @@ -140,6 +140,8 @@ process_diffs () {\n> \t\t\tmy ($oid) = @_;\n> \t\t\tmy $x;\n> \n> +\t\t\treturn \"\" unless length $oid;\n> +\n> \t\t\tif ($oid =~ /^(100644|100755|120000)$/) {\n> \t\t\t\treturn $oid;\n> \t\t\t}\n> @@ -160,8 +162,8 @@ process_diffs () {\n> \t\twhile () {\n> \t\t\ts/($orx)/munge_oid($1)/ge;\n> \t\t\ts/From ($x40)( |\\))/\"From \" . munge_oid($1) . $2/ge;\n> -\t\t\ts/commit ($x40)($| \\()/\"commit \" . munge_oid($1) . $2/ge;\n> -\t\t\ts/($x40) /munge_oid($1) . \" \"/ge;\n> +\t\t\ts/commit ($x40)($| \\(from )($x40?)/\"commit \" . munge_oid($1) . $2 . munge_oid($3)/ge;\n> +\t\t\ts/\\b($x40)( |\\.\\.|$)/munge_oid($1) . $2/ge;\n> \t\t\ts/^($x40)($| )/munge_oid($1) . $2/e;\n> \t\t\ts/($xab)(\\.\\.|,| |\\.\\.\\.|$)/munge_oid($1) . $2/ge;\n> \t\t\tprint;\n> \n> Or, a fresh original version:\n> \n> -- %< --\n> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\n> index e6eb4dd4c7..dfc87a0d19 100755\n> --- a/t/t4013-diff-various.sh\n> +++ b/t/t4013-diff-various.sh\n> @@ -130,27 +130,45 @@ test_expect_success setup '\n> EOF\n> \n> process_diffs () {\n> -\t_x04=\"[0-9a-f][0-9a-f][0-9a-f][0-9a-f]\" &&\n> -\t_x07=\"$_x05[0-9a-f][0-9a-f]\" &&\n> -\tsed -e \"s/$OID_REGEX/$ZERO_OID/g\" \\\n> -\t -e \"s/From $_x40 /From $ZERO_OID /\" \\\n> -\t -e \"s/from $_x40)/from $ZERO_OID)/\" \\\n> -\t -e \"s/commit $_x40\\$/commit $ZERO_OID/\" \\\n> -\t -e \"s/commit $_x40 (/commit $ZERO_OID (/\" \\\n> -\t -e \"s/$_x40 $_x40 $_x40/$ZERO_OID $ZERO_OID $ZERO_OID/\" \\\n> -\t -e \"s/$_x40 $_x40 /$ZERO_OID $ZERO_OID /\" \\\n> -\t -e \"s/^$_x40 $_x40$/$ZERO_OID $ZERO_OID/\" \\\n> -\t -e \"s/^$_x40 /$ZERO_OID /\" \\\n> -\t -e \"s/^$_x40$/$ZERO_OID/\" \\\n> -\t -e \"s/$_x07\\.\\.$_x07/fffffff..fffffff/g\" \\\n> -\t -e \"s/$_x07,$_x07\\.\\.$_x07/fffffff,fffffff..fffffff/g\" \\\n> -\t -e \"s/$_x07 $_x07 $_x07/fffffff fffffff fffffff/g\" \\\n> -\t -e \"s/$_x07 $_x07 /fffffff fffffff /g\" \\\n> -\t -e \"s/Merge: $_x07 $_x07/Merge: fffffff fffffff/g\" \\\n> -\t -e \"s/$_x07\\.\\.\\./fffffff.../g\" \\\n> -\t -e \"s/ $_x04\\.\\.\\./ ffff.../g\" \\\n> -\t -e \"s/ $_x04/ ffff/g\" \\\n> -\t \"$1\"\n> +\tperl -e '\n> +\t\tmy $oid_length = length($ARGV[0]);\n> +\t\tmy $x40 = \"[0-9a-f]{40}\";\n> +\t\tmy $xab = \"[0-9a-f]{4,16}\";\n> +\t\tmy $orx = \"[0-9a-f]\" x $oid_length;\n> +\n> +\t\tsub munge_oid {\n> +\t\t\tmy ($oid) = @_;\n> +\t\t\tmy $x;\n> +\n> +\t\t\treturn \"\" unless length $oid;\n> +\n> +\t\t\tif ($oid =~ /^(100644|100755|120000)$/) {\n> +\t\t\t\treturn $oid;\n> +\t\t\t}\n> +\n> +\t\t\tif ($oid =~ /^0*$/) {\n> +\t\t\t\t$x = \"0\";\n> +\t\t\t} else {\n> +\t\t\t\t$x = \"f\";\n> +\t\t\t}\n> +\n> +\t\t\tif (length($oid) == 40) {\n> +\t\t\t\treturn $x x $oid_length;\n> +\t\t\t} else {\n> +\t\t\t\treturn $x x length($oid);\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\twhile () {\n> +\t\t\ts/($orx)/munge_oid($1)/ge;\n> +\t\t\ts/From ($x40)( |\\))/\"From \" . munge_oid($1) . $2/ge;\n> +\t\t\ts/commit ($x40)($| \\(from )($x40?)/\"commit \" . munge_oid($1) . $2 . munge_oid($3)/ge;\n> +\t\t\ts/\\b($x40)( |\\.\\.|$)/munge_oid($1) . $2/ge;\n> +\t\t\ts/^($x40)($| )/munge_oid($1) . $2/e;\n> +\t\t\ts/($xab)(\\.\\.|,| |\\.\\.\\.|$)/munge_oid($1) . $2/ge;\n> +\t\t\tprint;\n> +\t\t}\n> +\t' \"$ZERO_OID\" <\"$1\"\n> }\n> \n> V=$(git version | sed -e 's/^git version //' -e 's/\\./\\\\./g')\n> -- %< --\n> \n> Anyone is welcome to have my sign-off if they pick up any part of this\n> patch.\n\nThis one seems to work with:\n\n\tmake -j9 test GIT_TEST_DEFAULT_HASH=sha256\n\nIf noone step up and write this into a patch in some days,\nI'll take this as first step in my series.\n\nAlso waiting some days so other people could come up with better idea,\nsed's y seems to be able to work if we don't have the constraint\non all-0 oid.\n\n\n-- \nDanh\n"},{"id":"403828","messageId":"xmqqo8n9456w.fsf@gitster.c.googlers.com","threadId":"54009","inReplyTo":"20200815022759.GC12363@danh.dev","subject":"Re: [PATCH v3 2/2] diff: index-line: respect --abbrev in object's name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-17T16:17:59Z","receivedAt":"2020-08-17T16:23:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n>> Anyone is welcome to have my sign-off if they pick up any part of this\n>> patch.\n\nLikewise.\n\n> This one seems to work with:\n>\n> \tmake -j9 test GIT_TEST_DEFAULT_HASH=sha256\n>\n> If noone step up and write this into a patch in some days,\n> I'll take this as first step in my series.\n>\n> Also waiting some days so other people could come up with better idea,\n> sed's y seems to be able to work if we don't have the constraint\n> on all-0 oid.\n\ny/// would help with the \"same length\" transformation, but I think\nthe primary reason why \"sed\" would not work well in this example is\n\"locate the things to tranform\" part is harder to separate from\n\"here is how to transform a token\" part when written in \"sed\".\n\nYes, you could use /pattern/{ s/from/to/; s/from2/to/; ...} but\nat least I didn't see an obvious way to simplify the original and\nreduce its repetitiveness.\n"},{"id":"404073","messageId":"xmqq4koycks8.fsf@gitster.c.googlers.com","threadId":"54009","inReplyTo":"20200814145043.GB9518@danh.dev","subject":"Re: [PATCH v3 1/2] revision: differentiate if --no-abbrev asked explicitly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-19T22:50:47Z","receivedAt":"2020-08-19T22:50:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n>> What am I missing?\n>\n> No, you didn't miss anything.\n>\n> It's obviously me, who screwed up the logical thinking.\n>\n> Originally, I come up with something along the line in 2/2:\n>\n> \tint abbrev = o->flags.full_index ? hexsz : DEFAULT_ABBREV;\n> \tif (!o->abbrev)\n> \t\tabbrev = o->abbrev;\n>\n> I couldn't recalled what I wrote, but that logic requires 1/2,\n> after I come up with 1/2, I re-analysed 2/2 and come up with current\n> logic.\n>\n> I failed to re-visit 1/2 to check if it's necessary.\n> It's all MY fault.\n>\n> Sorry for wasting everyone's time in 1/2.\n>\n> Please eject 1/2 from this series.\n\nOK. 2/2 would work without this step.\n\n"},{"id":"404081","messageId":"xmqqv9hdc3um.fsf@gitster.c.googlers.com","threadId":"54009","inReplyTo":"20200815022759.GC12363@danh.dev","subject":"Re: [PATCH v3 2/2] diff: index-line: respect --abbrev in object's name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-20T04:56:33Z","receivedAt":"2020-08-20T04:56:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n>> Anyone is welcome to have my sign-off if they pick up any part of this\n>> patch.\n>\n> This one seems to work with:\n>\n> \tmake -j9 test GIT_TEST_DEFAULT_HASH=sha256\n>\n> If noone step up and write this into a patch in some days,\n> I'll take this as first step in my series.\n\nSo, is there anything happening on this front?\n\nThanks.\n"},{"id":"404087","messageId":"3cec490500ebb7037fe13fd70bbbb43d035a65f5.1597926783.git.congdanhqx@gmail.com","threadId":"54009","inReplyTo":"cover.1597926783.git.congdanhqx@gmail.com","subject":"[PATCH v4 2/2] diff: index-line: respect --abbrev in object's name","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-20T12:35:35Z","receivedAt":"2020-08-20T12:36:16Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"A handful of Git's commands respect `--abbrev' for customizing length\nof abbreviation of object names.\n\nFor diff-family, Git supports 2 different options for 2 different\npurposes, `--full-index' for showing diff-patch object's name in full,\nand `--abbrev' to customize the length of object names in diff-raw and\ndiff-tree header lines, without any options to customise the length of\nobject names in diff-patch format. When working with diff-patch format,\nwe only have two options, either full index, or default abbrev length.\n\nAlthough, that consistent is documented, it doesn't stop users from\ntrying to use `--abbrev' with the hope of customising diff-patch's\nobjects' name's abbreviation.\n\nLet's resolve that inconsistency.\n\nTo preserve backward compatibility with old script that specify both\n`--full-index' and `--abbrev', always shows full object id\nif `--full-index' is specified.\n\nSigned-off-by: Đoàn Trần Công Danh \n---\n Documentation/diff-options.txt | 9 +++---\n diff.c | 5 +++-\n t/t4013-diff-various.sh | 3 ++\n ...ff.diff-tree_--root_-p_--abbrev=10_initial | 29 +++++++++++++++++++\n ...--root_-p_--full-index_--abbrev=10_initial | 29 +++++++++++++++++++\n ...f.diff-tree_--root_-p_--full-index_initial | 29 +++++++++++++++++++\n 6 files changed, 99 insertions(+), 5 deletions(-)\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex b7af973d9c..1bb897d665 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -446,10 +446,11 @@ endif::git-format-patch[]\n --abbrev[=]::\n \tInstead of showing the full 40-byte hexadecimal object\n \tname in diff-raw format output and diff-tree header\n-\tlines, show only a partial prefix. This is\n-\tindependent of the `--full-index` option above, which controls\n-\tthe diff-patch output format. Non default number of\n-\tdigits can be specified with `--abbrev=`.\n+\tlines, show only a partial prefix.\n+\tIn diff-patch output format, `--full-index` takes higher\n+\tprecedent, i.e. if `--full-index` is specified, full blob\n+\tnames will be shown regardless of `--abbrev`.\n+\tNon default number of digits can be specified with `--abbrev=`.\n \n -B[][/]::\n --break-rewrites[=[][/]]::\ndiff --git a/diff.c b/diff.c\nindex f9709de7b4..20dedfe2a9 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4319,7 +4319,10 @@ static void fill_metainfo(struct strbuf *msg,\n \t}\n \tif (one && two && !oideq(&one->oid, &two->oid)) {\n \t\tconst unsigned hexsz = the_hash_algo->hexsz;\n-\t\tint abbrev = o->flags.full_index ? hexsz : DEFAULT_ABBREV;\n+\t\tint abbrev = o->abbrev ? o->abbrev : DEFAULT_ABBREV;\n+\n+\t\tif (o->flags.full_index)\n+\t\t\tabbrev = hexsz;\n \n \t\tif (o->flags.binary) {\n \t\t\tmmfile_t mf;\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex f6bdfc13fd..5c7b0122b4 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -239,6 +239,9 @@ diff-tree --root -r --abbrev=4 initial\n :noellipses diff-tree --root -r --abbrev=4 initial\n diff-tree -p initial\n diff-tree --root -p initial\n+diff-tree --root -p --abbrev=10 initial\n+diff-tree --root -p --full-index initial\n+diff-tree --root -p --full-index --abbrev=10 initial\n diff-tree --patch-with-stat initial\n diff-tree --root --patch-with-stat initial\n diff-tree --patch-with-raw initial\ndiff --git a/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial b/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\nnew file mode 100644\nindex 0000000000..7518a9044e\n--- /dev/null\n+++ b/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n@@ -0,0 +1,29 @@\n+$ git diff-tree --root -p --abbrev=10 initial\n+444ac553ac7612cc88969031b02b3767fb8a353a\n+diff --git a/dir/sub b/dir/sub\n+new file mode 100644\n+index 0000000000..35d242ba79\n+--- /dev/null\n++++ b/dir/sub\n+@@ -0,0 +1,2 @@\n++A\n++B\n+diff --git a/file0 b/file0\n+new file mode 100644\n+index 0000000000..01e79c32a8\n+--- /dev/null\n++++ b/file0\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+diff --git a/file2 b/file2\n+new file mode 100644\n+index 0000000000..01e79c32a8\n+--- /dev/null\n++++ b/file2\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+$\ndiff --git a/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial b/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\nnew file mode 100644\nindex 0000000000..69f913fbe5\n--- /dev/null\n+++ b/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n@@ -0,0 +1,29 @@\n+$ git diff-tree --root -p --full-index --abbrev=10 initial\n+444ac553ac7612cc88969031b02b3767fb8a353a\n+diff --git a/dir/sub b/dir/sub\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..35d242ba79ae89ac695e26b3d4c27a8e6f028f9e\n+--- /dev/null\n++++ b/dir/sub\n+@@ -0,0 +1,2 @@\n++A\n++B\n+diff --git a/file0 b/file0\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file0\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+diff --git a/file2 b/file2\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file2\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+$\ndiff --git a/t/t4013/diff.diff-tree_--root_-p_--full-index_initial b/t/t4013/diff.diff-tree_--root_-p_--full-index_initial\nnew file mode 100644\nindex 0000000000..1b0b6717fa\n--- /dev/null\n+++ b/t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n@@ -0,0 +1,29 @@\n+$ git diff-tree --root -p --full-index initial\n+444ac553ac7612cc88969031b02b3767fb8a353a\n+diff --git a/dir/sub b/dir/sub\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..35d242ba79ae89ac695e26b3d4c27a8e6f028f9e\n+--- /dev/null\n++++ b/dir/sub\n+@@ -0,0 +1,2 @@\n++A\n++B\n+diff --git a/file0 b/file0\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file0\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+diff --git a/file2 b/file2\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file2\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+$\n-- \n2.28.0.143.g760df7782d.dirty\n\n"},{"id":"404088","messageId":"a52d0e59ecd5777f2a1d242a37c6bb6aaafb1ed2.1597926783.git.congdanhqx@gmail.com","threadId":"54009","inReplyTo":"cover.1597926783.git.congdanhqx@gmail.com","subject":"[PATCH v4 1/2] t4013: improve diff-post-processor logic","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-20T12:35:34Z","receivedAt":"2020-08-20T12:36:19Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"From: \"brian m. carlson\" \n\nFrom 72f936b1 (t4013: make test hash independent, 2020-02-07),\nwe started to adjust metadata of git-diff's output in order to\nignore uninteresting metadata which is dependent of underlying hash\nalgorithm.\n\nHowever, we forgot to special case all-zero object names, which is\nspecial for not-exist objects, in consequence, we could't catch\npossible future bugs where object names is all-zeros including but\nnot limiting to:\n* show intend-to-add entry\n* deleted entry\n* diff between index and working tree with new file\n\nIn addition, in the incoming change, we would like to test for\ndiff with 10 characters index, which is also not covered by current\ndiff-processor logic.\n\nLet's fix the bug for all-zero object names and extend object names'\nabbrev to 16 while we're at it.\n\nBased-on-patch-by: Junio C Hamano \nSigned-off-by: Junio C Hamano \nSigned-off-by: brian m. carlson \nSigned-off-by: Đoàn Trần Công Danh \n---\n t/t4013-diff-various.sh | 60 ++++++++++++++++++++++++++---------------\n 1 file changed, 39 insertions(+), 21 deletions(-)\n\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex 5f97dd6d65..f6bdfc13fd 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -130,27 +130,45 @@ test_expect_success setup '\n EOF\n \n process_diffs () {\n-\t_x04=\"[0-9a-f][0-9a-f][0-9a-f][0-9a-f]\" &&\n-\t_x07=\"$_x05[0-9a-f][0-9a-f]\" &&\n-\tsed -e \"s/$OID_REGEX/$ZERO_OID/g\" \\\n-\t -e \"s/From $_x40 /From $ZERO_OID /\" \\\n-\t -e \"s/from $_x40)/from $ZERO_OID)/\" \\\n-\t -e \"s/commit $_x40\\$/commit $ZERO_OID/\" \\\n-\t -e \"s/commit $_x40 (/commit $ZERO_OID (/\" \\\n-\t -e \"s/$_x40 $_x40 $_x40/$ZERO_OID $ZERO_OID $ZERO_OID/\" \\\n-\t -e \"s/$_x40 $_x40 /$ZERO_OID $ZERO_OID /\" \\\n-\t -e \"s/^$_x40 $_x40$/$ZERO_OID $ZERO_OID/\" \\\n-\t -e \"s/^$_x40 /$ZERO_OID /\" \\\n-\t -e \"s/^$_x40$/$ZERO_OID/\" \\\n-\t -e \"s/$_x07\\.\\.$_x07/fffffff..fffffff/g\" \\\n-\t -e \"s/$_x07,$_x07\\.\\.$_x07/fffffff,fffffff..fffffff/g\" \\\n-\t -e \"s/$_x07 $_x07 $_x07/fffffff fffffff fffffff/g\" \\\n-\t -e \"s/$_x07 $_x07 /fffffff fffffff /g\" \\\n-\t -e \"s/Merge: $_x07 $_x07/Merge: fffffff fffffff/g\" \\\n-\t -e \"s/$_x07\\.\\.\\./fffffff.../g\" \\\n-\t -e \"s/ $_x04\\.\\.\\./ ffff.../g\" \\\n-\t -e \"s/ $_x04/ ffff/g\" \\\n-\t \"$1\"\n+\tperl -e '\n+\t\tmy $oid_length = length($ARGV[0]);\n+\t\tmy $x40 = \"[0-9a-f]{40}\";\n+\t\tmy $xab = \"[0-9a-f]{4,16}\";\n+\t\tmy $orx = \"[0-9a-f]\" x $oid_length;\n+\n+\t\tsub munge_oid {\n+\t\t\tmy ($oid) = @_;\n+\t\t\tmy $x;\n+\n+\t\t\treturn \"\" unless length $oid;\n+\n+\t\t\tif ($oid =~ /^(100644|100755|120000)$/) {\n+\t\t\t\treturn $oid;\n+\t\t\t}\n+\n+\t\t\tif ($oid =~ /^0*$/) {\n+\t\t\t\t$x = \"0\";\n+\t\t\t} else {\n+\t\t\t\t$x = \"f\";\n+\t\t\t}\n+\n+\t\t\tif (length($oid) == 40) {\n+\t\t\t\treturn $x x $oid_length;\n+\t\t\t} else {\n+\t\t\t\treturn $x x length($oid);\n+\t\t\t}\n+\t\t}\n+\n+\t\twhile () {\n+\t\t\ts/($orx)/munge_oid($1)/ge;\n+\t\t\ts/From ($x40)( |\\))/\"From \" . munge_oid($1) . $2/ge;\n+\t\t\ts/commit ($x40)($| \\(from )($x40?)/\"commit \" . munge_oid($1) . $2 . munge_oid($3)/ge;\n+\t\t\ts/\\b($x40)( |\\.\\.|$)/munge_oid($1) . $2/ge;\n+\t\t\ts/^($x40)($| )/munge_oid($1) . $2/e;\n+\t\t\ts/($xab)(\\.\\.|,| |\\.\\.\\.|$)/munge_oid($1) . $2/ge;\n+\t\t\tprint;\n+\t\t}\n+\t' \"$ZERO_OID\" <\"$1\"\n }\n \n V=$(git version | sed -e 's/^git version //' -e 's/\\./\\\\./g')\n-- \n2.28.0.143.g760df7782d.dirty\n\n"},{"id":"404089","messageId":"cover.1597926783.git.congdanhqx@gmail.com","threadId":"54009","inReplyTo":"cover.1596887883.git.congdanhqx@gmail.com","subject":"[PATCH v4 0/2] diff: index-line: respect --abbrev in object's name","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-20T12:35:33Z","receivedAt":"2020-08-20T12:36:21Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-08-19 21:56:33-0700, Junio C Hamano wrote:\n> Đoàn Trần Công Danh writes:\n> \n> >> Anyone is welcome to have my sign-off if they pick up any part of this\n> >> patch.\n> >\n> > This one seems to work with:\n> >\n> > \tmake -j9 test GIT_TEST_DEFAULT_HASH=sha256\n> >\n> > If noone step up and write this into a patch in some days,\n> > I'll take this as first step in my series.\n> \n> So, is there anything happening on this front?\n\nSorry, I was busy with work-life this week.\n\n--------------8<-----------\n\nFor diff-family, Git supports 2 different options for 2 different\npurposes, `--full-index' for showing diff-patch object's name in full,\nand `--abbrev' to customize the length of object names in diff-raw and\ndiff-tree header lines, without any options to customise the length of\nobject names in diff-patch format. When working with diff-patch format,\nwe only have two options, either full index, or default abbrev length.\n\nAlthough, that consistent is documented, it doesn't stop users from\ntrying to use `--abbrev' with the hope of customising diff-patch's\nobjects' name's abbreviation.\n\nLet's resolve that inconsistency.\n\n\nbrian m. carlson (1):\n t4013: improve diff-post-processor logic\n\nĐoàn Trần Công Danh (1):\n diff: index-line: respect --abbrev in object's name\n\n Documentation/diff-options.txt | 9 +--\n diff.c | 5 +-\n t/t4013-diff-various.sh | 63 ++++++++++++-------\n ...ff.diff-tree_--root_-p_--abbrev=10_initial | 29 +++++++++\n ...--root_-p_--full-index_--abbrev=10_initial | 29 +++++++++\n ...f.diff-tree_--root_-p_--full-index_initial | 29 +++++++++\n 6 files changed, 138 insertions(+), 26 deletions(-)\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n\nRange-diff against v3:\n1: 7966f6ca1d < -: ---------- revision: differentiate if --no-abbrev asked explicitly\n-: ---------- > 1: a52d0e59ec t4013: improve diff-post-processor logic\n2: 1d32e791a4 = 2: 3cec490500 diff: index-line: respect --abbrev in object's name\n-- \n2.28.0.143.g760df7782d.dirty\n\n"},{"id":"404110","messageId":"xmqqr1s1ayj2.fsf@gitster.c.googlers.com","threadId":"54009","inReplyTo":"a52d0e59ecd5777f2a1d242a37c6bb6aaafb1ed2.1597926783.git.congdanhqx@gmail.com","subject":"Re: [PATCH v4 1/2] t4013: improve diff-post-processor logic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-20T19:49:05Z","receivedAt":"2020-08-20T19:49:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n> From: \"brian m. carlson\" \n>\n> From 72f936b1 (t4013: make test hash independent, 2020-02-07),\n> we started to adjust metadata of git-diff's output in order to\n> ignore uninteresting metadata which is dependent of underlying hash\n> algorithm.\n>\n> However, we forgot to special case all-zero object names, which is\n> special for not-exist objects, in consequence, we could't catch\n\n\"for missing objects\" would probably the best, even though\n\"non-existing objects\" would also work.\n\n> possible future bugs where object names is all-zeros including but\n\ns/is/are/\n\n> not limiting to:\n\n\"not limited to\", I think.\n\n> * show intend-to-add entry\n> * deleted entry\n> * diff between index and working tree with new file\n>\n> In addition, in the incoming change, we would like to test for\n\ns/incoming/upcoming/?\n\n> diff with 10 characters index, which is also not covered by current\n\n\"test for customizing the length of abbreviated blob object names on\nthe index line\"?\n\ns/covered/supported/\n\n> diff-processor logic.\n>\n> Let's fix the bug for all-zero object names and extend object names'\n> abbrev to 16 while we're at it.\n\n\"and support abbreviation of object names up to 16 bytes\"?\n\nAlso while we are at it, we fixed the post-processing not to touch\nthe file modes, which were mistakenly munged in the older code as if\nthey were object names abbreviated to 7 hexdigits.\n\n> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\n> index 5f97dd6d65..f6bdfc13fd 100755\n> --- a/t/t4013-diff-various.sh\n> +++ b/t/t4013-diff-various.sh\n> @@ -130,27 +130,45 @@ test_expect_success setup '\n> EOF\n> \n> process_diffs () {\n> -\t_x04=\"[0-9a-f][0-9a-f][0-9a-f][0-9a-f]\" &&\n> -\t_x07=\"$_x05[0-9a-f][0-9a-f]\" &&\n> -\tsed -e \"s/$OID_REGEX/$ZERO_OID/g\" \\\n> -\t -e \"s/From $_x40 /From $ZERO_OID /\" \\\n> -\t -e \"s/from $_x40)/from $ZERO_OID)/\" \\\n> -\t -e \"s/commit $_x40\\$/commit $ZERO_OID/\" \\\n> -\t -e \"s/commit $_x40 (/commit $ZERO_OID (/\" \\\n> -\t -e \"s/$_x40 $_x40 $_x40/$ZERO_OID $ZERO_OID $ZERO_OID/\" \\\n> -\t -e \"s/$_x40 $_x40 /$ZERO_OID $ZERO_OID /\" \\\n> -\t -e \"s/^$_x40 $_x40$/$ZERO_OID $ZERO_OID/\" \\\n> -\t -e \"s/^$_x40 /$ZERO_OID /\" \\\n> -\t -e \"s/^$_x40$/$ZERO_OID/\" \\\n> -\t -e \"s/$_x07\\.\\.$_x07/fffffff..fffffff/g\" \\\n> -\t -e \"s/$_x07,$_x07\\.\\.$_x07/fffffff,fffffff..fffffff/g\" \\\n> -\t -e \"s/$_x07 $_x07 $_x07/fffffff fffffff fffffff/g\" \\\n> -\t -e \"s/$_x07 $_x07 /fffffff fffffff /g\" \\\n> -\t -e \"s/Merge: $_x07 $_x07/Merge: fffffff fffffff/g\" \\\n> -\t -e \"s/$_x07\\.\\.\\./fffffff.../g\" \\\n> -\t -e \"s/ $_x04\\.\\.\\./ ffff.../g\" \\\n> -\t -e \"s/ $_x04/ ffff/g\" \\\n> -\t \"$1\"\n> +\tperl -e '\n> +\t\tmy $oid_length = length($ARGV[0]);\n> +\t\tmy $x40 = \"[0-9a-f]{40}\";\n> +\t\tmy $xab = \"[0-9a-f]{4,16}\";\n> +\t\tmy $orx = \"[0-9a-f]\" x $oid_length;\n> +\n> +\t\tsub munge_oid {\n> +\t\t\tmy ($oid) = @_;\n> +\t\t\tmy $x;\n> +\n> +\t\t\treturn \"\" unless length $oid;\n> +\n> +\t\t\tif ($oid =~ /^(100644|100755|120000)$/) {\n> +\t\t\t\treturn $oid;\n> +\t\t\t}\n> +\n> +\t\t\tif ($oid =~ /^0*$/) {\n> +\t\t\t\t$x = \"0\";\n> +\t\t\t} else {\n> +\t\t\t\t$x = \"f\";\n> +\t\t\t}\n> +\n> +\t\t\tif (length($oid) == 40) {\n> +\t\t\t\treturn $x x $oid_length;\n> +\t\t\t} else {\n> +\t\t\t\treturn $x x length($oid);\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\twhile () {\n> +\t\t\ts/($orx)/munge_oid($1)/ge;\n> +\t\t\ts/From ($x40)( |\\))/\"From \" . munge_oid($1) . $2/ge;\n> +\t\t\ts/commit ($x40)($| \\(from )($x40?)/\"commit \" . munge_oid($1) . $2 . munge_oid($3)/ge;\n> +\t\t\ts/\\b($x40)( |\\.\\.|$)/munge_oid($1) . $2/ge;\n> +\t\t\ts/^($x40)($| )/munge_oid($1) . $2/e;\n> +\t\t\ts/($xab)(\\.\\.|,| |\\.\\.\\.|$)/munge_oid($1) . $2/ge;\n> +\t\t\tprint;\n> +\t\t}\n> +\t' \"$ZERO_OID\" <\"$1\"\n> }\n> \n> V=$(git version | sed -e 's/^git version //' -e 's/\\./\\\\./g')\n"},{"id":"404111","messageId":"xmqqmu2pay3v.fsf@gitster.c.googlers.com","threadId":"54009","inReplyTo":"3cec490500ebb7037fe13fd70bbbb43d035a65f5.1597926783.git.congdanhqx@gmail.com","subject":"Re: [PATCH v4 2/2] diff: index-line: respect --abbrev in object's name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-20T19:58:12Z","receivedAt":"2020-08-20T19:58:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n> A handful of Git's commands respect `--abbrev' for customizing length\n> of abbreviation of object names.\n>\n> For diff-family, Git supports 2 different options for 2 different\n> purposes, `--full-index' for showing diff-patch object's name in full,\n> and `--abbrev' to customize the length of object names in diff-raw and\n> diff-tree header lines, without any options to customise the length of\n> object names in diff-patch format. When working with diff-patch format,\n> we only have two options, either full index, or default abbrev length.\n\nCorrect.\n\n\n> Although, that consistent is documented, it doesn't stop users from\n\nI am not sure what you meant by the word \"consistent\" here.\nDropping the word altogether makes the sentence work just fine, but\nit may not be what you wanted to say, so I dunno.\n\n> trying to use `--abbrev' with the hope of customising diff-patch's\n> objects' name's abbreviation.\n\nOK.\n\n> Let's resolve that inconsistency.\n\nI am not sure if this should even be called \"inconsistency\". We\nhave two things to control, (1) length of abbreviated object names\nin the \"raw\" output and (2) length of abbreviated object names on\nthe \"index\" line, and they can be independently controlled with\ndifferent options. What this patch does is, as the first paragraph\nexplained, the latter does not take an arbitrary length as end users\nmay expect.\n\n\tLet's allow the blob object names shown on the \"index\" line\n\tto be abbreviated to arbitrary length given via the \"--abbrev\"\n\toption.\n\nperhaps?\n\n> To preserve backward compatibility with old script that specify both\n> `--full-index' and `--abbrev', always shows full object id\n> if `--full-index' is specified.\n\ns/shows/show/ but otherwise good.\n\n> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n> index b7af973d9c..1bb897d665 100644\n> --- a/Documentation/diff-options.txt\n> +++ b/Documentation/diff-options.txt\n> @@ -446,10 +446,11 @@ endif::git-format-patch[]\n> --abbrev[=]::\n> \tInstead of showing the full 40-byte hexadecimal object\n> \tname in diff-raw format output and diff-tree header\n> -\tlines, show only a partial prefix. This is\n> -\tindependent of the `--full-index` option above, which controls\n> -\tthe diff-patch output format. Non default number of\n> -\tdigits can be specified with `--abbrev=`.\n> +\tlines, show only a partial prefix.\n> +\tIn diff-patch output format, `--full-index` takes higher\n> +\tprecedent, i.e. if `--full-index` is specified, full blob\n\n\"precedence\", I think.\n\n> +\tnames will be shown regardless of `--abbrev`.\n> +\tNon default number of digits can be specified with `--abbrev=`.\n"},{"id":"404130","messageId":"a9eb73ceab88aea517e978bfcba83bfafeaff3f7.1598010556.git.congdanhqx@gmail.com","threadId":"54009","inReplyTo":"cover.1598010556.git.congdanhqx@gmail.com","subject":"[PATCH v5 2/2] diff: index-line: respect --abbrev in object's name","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-21T11:51:47Z","receivedAt":"2020-08-21T11:54:21Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"A handful of Git's commands respect `--abbrev' for customizing length\nof abbreviation of object names.\n\nFor diff-family, Git supports 2 different options for 2 different\npurposes, `--full-index' for showing diff-patch object's name in full,\nand `--abbrev' to customize the length of object names in diff-raw and\ndiff-tree header lines, without any options to customise the length of\nobject names in diff-patch format. When working with diff-patch format,\nwe only have two options, either full index, or default abbrev length.\n\nAlthough, that behaviour is documented, it doesn't stop users from\ntrying to use `--abbrev' with the hope of customising diff-patch's\nobjects' name's abbreviation.\n\nLet's allow the blob object names shown on the \"index\" line to be\nabbreviated to arbitrary length given via the \"--abbrev\" option.\n\nTo preserve backward compatibility with old script that specify both\n`--full-index' and `--abbrev', always show full object id\nif `--full-index' is specified.\n\nSigned-off-by: Đoàn Trần Công Danh \n---\n Documentation/diff-options.txt | 9 +++---\n diff.c | 5 +++-\n t/t4013-diff-various.sh | 3 ++\n ...ff.diff-tree_--root_-p_--abbrev=10_initial | 29 +++++++++++++++++++\n ...--root_-p_--full-index_--abbrev=10_initial | 29 +++++++++++++++++++\n ...f.diff-tree_--root_-p_--full-index_initial | 29 +++++++++++++++++++\n 6 files changed, 99 insertions(+), 5 deletions(-)\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex b7af973d9c..573fb9bb71 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -446,10 +446,11 @@ endif::git-format-patch[]\n --abbrev[=]::\n \tInstead of showing the full 40-byte hexadecimal object\n \tname in diff-raw format output and diff-tree header\n-\tlines, show only a partial prefix. This is\n-\tindependent of the `--full-index` option above, which controls\n-\tthe diff-patch output format. Non default number of\n-\tdigits can be specified with `--abbrev=`.\n+\tlines, show only a partial prefix.\n+\tIn diff-patch output format, `--full-index` takes higher\n+\tprecedence, i.e. if `--full-index` is specified, full blob\n+\tnames will be shown regardless of `--abbrev`.\n+\tNon default number of digits can be specified with `--abbrev=`.\n \n -B[][/]::\n --break-rewrites[=[][/]]::\ndiff --git a/diff.c b/diff.c\nindex f9709de7b4..20dedfe2a9 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4319,7 +4319,10 @@ static void fill_metainfo(struct strbuf *msg,\n \t}\n \tif (one && two && !oideq(&one->oid, &two->oid)) {\n \t\tconst unsigned hexsz = the_hash_algo->hexsz;\n-\t\tint abbrev = o->flags.full_index ? hexsz : DEFAULT_ABBREV;\n+\t\tint abbrev = o->abbrev ? o->abbrev : DEFAULT_ABBREV;\n+\n+\t\tif (o->flags.full_index)\n+\t\t\tabbrev = hexsz;\n \n \t\tif (o->flags.binary) {\n \t\t\tmmfile_t mf;\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex f6bdfc13fd..5c7b0122b4 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -239,6 +239,9 @@ diff-tree --root -r --abbrev=4 initial\n :noellipses diff-tree --root -r --abbrev=4 initial\n diff-tree -p initial\n diff-tree --root -p initial\n+diff-tree --root -p --abbrev=10 initial\n+diff-tree --root -p --full-index initial\n+diff-tree --root -p --full-index --abbrev=10 initial\n diff-tree --patch-with-stat initial\n diff-tree --root --patch-with-stat initial\n diff-tree --patch-with-raw initial\ndiff --git a/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial b/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\nnew file mode 100644\nindex 0000000000..7518a9044e\n--- /dev/null\n+++ b/t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n@@ -0,0 +1,29 @@\n+$ git diff-tree --root -p --abbrev=10 initial\n+444ac553ac7612cc88969031b02b3767fb8a353a\n+diff --git a/dir/sub b/dir/sub\n+new file mode 100644\n+index 0000000000..35d242ba79\n+--- /dev/null\n++++ b/dir/sub\n+@@ -0,0 +1,2 @@\n++A\n++B\n+diff --git a/file0 b/file0\n+new file mode 100644\n+index 0000000000..01e79c32a8\n+--- /dev/null\n++++ b/file0\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+diff --git a/file2 b/file2\n+new file mode 100644\n+index 0000000000..01e79c32a8\n+--- /dev/null\n++++ b/file2\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+$\ndiff --git a/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial b/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\nnew file mode 100644\nindex 0000000000..69f913fbe5\n--- /dev/null\n+++ b/t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n@@ -0,0 +1,29 @@\n+$ git diff-tree --root -p --full-index --abbrev=10 initial\n+444ac553ac7612cc88969031b02b3767fb8a353a\n+diff --git a/dir/sub b/dir/sub\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..35d242ba79ae89ac695e26b3d4c27a8e6f028f9e\n+--- /dev/null\n++++ b/dir/sub\n+@@ -0,0 +1,2 @@\n++A\n++B\n+diff --git a/file0 b/file0\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file0\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+diff --git a/file2 b/file2\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file2\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+$\ndiff --git a/t/t4013/diff.diff-tree_--root_-p_--full-index_initial b/t/t4013/diff.diff-tree_--root_-p_--full-index_initial\nnew file mode 100644\nindex 0000000000..1b0b6717fa\n--- /dev/null\n+++ b/t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n@@ -0,0 +1,29 @@\n+$ git diff-tree --root -p --full-index initial\n+444ac553ac7612cc88969031b02b3767fb8a353a\n+diff --git a/dir/sub b/dir/sub\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..35d242ba79ae89ac695e26b3d4c27a8e6f028f9e\n+--- /dev/null\n++++ b/dir/sub\n+@@ -0,0 +1,2 @@\n++A\n++B\n+diff --git a/file0 b/file0\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file0\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+diff --git a/file2 b/file2\n+new file mode 100644\n+index 0000000000000000000000000000000000000000..01e79c32a8c99c557f0757da7cb6d65b3414466d\n+--- /dev/null\n++++ b/file2\n+@@ -0,0 +1,3 @@\n++1\n++2\n++3\n+$\n-- \n2.28.0.143.g760df7782d.dirty\n\n"},{"id":"404131","messageId":"d7e0f13ebaeae0545b74a64f13171ea85cc26d18.1598010556.git.congdanhqx@gmail.com","threadId":"54009","inReplyTo":"cover.1598010556.git.congdanhqx@gmail.com","subject":"[PATCH v5 1/2] t4013: improve diff-post-processor logic","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-21T11:51:46Z","receivedAt":"2020-08-21T11:54:23Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"From: \"brian m. carlson\" \n\nFrom 72f936b1 (t4013: make test hash independent, 2020-02-07),\nwe started to adjust metadata of git-diff's output in order to\nignore uninteresting metadata which is dependent of underlying hash\nalgorithm.\n\nHowever, we forgot to special case all-zero object names, which is\nspecial for missing objects, in consequence, we could't catch\npossible future bugs where object names is all-zeros including but\nnot limited to:\n* show intend-to-add entry\n* deleted entry\n* diff between index and working tree with new file\n\nWe also mistakenly munged file-modes as if they were object names\nabbreviated to 6 hexadecimal digits.\n\nIn addition, in the upcoming change, we would like to test for\ncustomizing the length of abbreviated blob objects on the index line,\nwhich is not supported by current diff-processor logic.\n\nLet's fix the bug for all-zero object names, and file modes.\nWhile we're at it, support abbreviation of object names up to 16 bytes.\n\nBased-on-patch-by: Junio C Hamano \nSigned-off-by: Junio C Hamano \nSigned-off-by: brian m. carlson \nSigned-off-by: Đoàn Trần Công Danh \n---\n t/t4013-diff-various.sh | 60 ++++++++++++++++++++++++++---------------\n 1 file changed, 39 insertions(+), 21 deletions(-)\n\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex 5f97dd6d65..f6bdfc13fd 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -130,27 +130,45 @@ test_expect_success setup '\n EOF\n \n process_diffs () {\n-\t_x04=\"[0-9a-f][0-9a-f][0-9a-f][0-9a-f]\" &&\n-\t_x07=\"$_x05[0-9a-f][0-9a-f]\" &&\n-\tsed -e \"s/$OID_REGEX/$ZERO_OID/g\" \\\n-\t -e \"s/From $_x40 /From $ZERO_OID /\" \\\n-\t -e \"s/from $_x40)/from $ZERO_OID)/\" \\\n-\t -e \"s/commit $_x40\\$/commit $ZERO_OID/\" \\\n-\t -e \"s/commit $_x40 (/commit $ZERO_OID (/\" \\\n-\t -e \"s/$_x40 $_x40 $_x40/$ZERO_OID $ZERO_OID $ZERO_OID/\" \\\n-\t -e \"s/$_x40 $_x40 /$ZERO_OID $ZERO_OID /\" \\\n-\t -e \"s/^$_x40 $_x40$/$ZERO_OID $ZERO_OID/\" \\\n-\t -e \"s/^$_x40 /$ZERO_OID /\" \\\n-\t -e \"s/^$_x40$/$ZERO_OID/\" \\\n-\t -e \"s/$_x07\\.\\.$_x07/fffffff..fffffff/g\" \\\n-\t -e \"s/$_x07,$_x07\\.\\.$_x07/fffffff,fffffff..fffffff/g\" \\\n-\t -e \"s/$_x07 $_x07 $_x07/fffffff fffffff fffffff/g\" \\\n-\t -e \"s/$_x07 $_x07 /fffffff fffffff /g\" \\\n-\t -e \"s/Merge: $_x07 $_x07/Merge: fffffff fffffff/g\" \\\n-\t -e \"s/$_x07\\.\\.\\./fffffff.../g\" \\\n-\t -e \"s/ $_x04\\.\\.\\./ ffff.../g\" \\\n-\t -e \"s/ $_x04/ ffff/g\" \\\n-\t \"$1\"\n+\tperl -e '\n+\t\tmy $oid_length = length($ARGV[0]);\n+\t\tmy $x40 = \"[0-9a-f]{40}\";\n+\t\tmy $xab = \"[0-9a-f]{4,16}\";\n+\t\tmy $orx = \"[0-9a-f]\" x $oid_length;\n+\n+\t\tsub munge_oid {\n+\t\t\tmy ($oid) = @_;\n+\t\t\tmy $x;\n+\n+\t\t\treturn \"\" unless length $oid;\n+\n+\t\t\tif ($oid =~ /^(100644|100755|120000)$/) {\n+\t\t\t\treturn $oid;\n+\t\t\t}\n+\n+\t\t\tif ($oid =~ /^0*$/) {\n+\t\t\t\t$x = \"0\";\n+\t\t\t} else {\n+\t\t\t\t$x = \"f\";\n+\t\t\t}\n+\n+\t\t\tif (length($oid) == 40) {\n+\t\t\t\treturn $x x $oid_length;\n+\t\t\t} else {\n+\t\t\t\treturn $x x length($oid);\n+\t\t\t}\n+\t\t}\n+\n+\t\twhile () {\n+\t\t\ts/($orx)/munge_oid($1)/ge;\n+\t\t\ts/From ($x40)( |\\))/\"From \" . munge_oid($1) . $2/ge;\n+\t\t\ts/commit ($x40)($| \\(from )($x40?)/\"commit \" . munge_oid($1) . $2 . munge_oid($3)/ge;\n+\t\t\ts/\\b($x40)( |\\.\\.|$)/munge_oid($1) . $2/ge;\n+\t\t\ts/^($x40)($| )/munge_oid($1) . $2/e;\n+\t\t\ts/($xab)(\\.\\.|,| |\\.\\.\\.|$)/munge_oid($1) . $2/ge;\n+\t\t\tprint;\n+\t\t}\n+\t' \"$ZERO_OID\" <\"$1\"\n }\n \n V=$(git version | sed -e 's/^git version //' -e 's/\\./\\\\./g')\n-- \n2.28.0.143.g760df7782d.dirty\n\n"},{"id":"404132","messageId":"cover.1598010556.git.congdanhqx@gmail.com","threadId":"54009","inReplyTo":"cover.1596887883.git.congdanhqx@gmail.com","subject":"[PATCH v5 0/2] diff: index-line: respect --abbrev in object's name","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-21T11:51:45Z","receivedAt":"2020-08-21T11:54:55Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"For diff-family, Git supports 2 different options for 2 different\npurposes, `--full-index' for showing diff-patch object's name in full,\nand `--abbrev' to customize the length of object names in diff-raw and\ndiff-tree header lines, without any options to customise the length of\nobject names in diff-patch format. When working with diff-patch format,\nwe only have two options, either full index, or default abbrev length.\n\nAlthough, that consistent is documented, it doesn't stop users from\ntrying to use `--abbrev' with the hope of customising diff-patch's\nobjects' name's abbreviation.\n\nLet's resolve that inconsistency.\n\n\nbrian m. carlson (1):\n t4013: improve diff-post-processor logic\n\nĐoàn Trần Công Danh (1):\n diff: index-line: respect --abbrev in object's name\n\n Documentation/diff-options.txt | 9 +--\n diff.c | 5 +-\n t/t4013-diff-various.sh | 63 ++++++++++++-------\n ...ff.diff-tree_--root_-p_--abbrev=10_initial | 29 +++++++++\n ...--root_-p_--full-index_--abbrev=10_initial | 29 +++++++++\n ...f.diff-tree_--root_-p_--full-index_initial | 29 +++++++++\n 6 files changed, 138 insertions(+), 26 deletions(-)\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_--abbrev=10_initial\n create mode 100644 t/t4013/diff.diff-tree_--root_-p_--full-index_initial\n\nRange-diff against v4:\n1: a52d0e59ec ! 1: d7e0f13eba t4013: improve diff-post-processor logic\n @@ Commit message\n algorithm.\n \n However, we forgot to special case all-zero object names, which is\n - special for not-exist objects, in consequence, we could't catch\n + special for missing objects, in consequence, we could't catch\n possible future bugs where object names is all-zeros including but\n - not limiting to:\n + not limited to:\n * show intend-to-add entry\n * deleted entry\n * diff between index and working tree with new file\n \n - In addition, in the incoming change, we would like to test for\n - diff with 10 characters index, which is also not covered by current\n - diff-processor logic.\n + We also mistakenly munged file-modes as if they were object names\n + abbreviated to 6 hexadecimal digits.\n \n - Let's fix the bug for all-zero object names and extend object names'\n - abbrev to 16 while we're at it.\n + In addition, in the upcoming change, we would like to test for\n + customizing the length of abbreviated blob objects on the index line,\n + which is not supported by current diff-processor logic.\n +\n + Let's fix the bug for all-zero object names, and file modes.\n + While we're at it, support abbreviation of object names up to 16 bytes.\n \n Based-on-patch-by: Junio C Hamano \n Signed-off-by: Junio C Hamano \n2: 3cec490500 ! 2: a9eb73ceab diff: index-line: respect --abbrev in object's name\n @@ Commit message\n object names in diff-patch format. When working with diff-patch format,\n we only have two options, either full index, or default abbrev length.\n \n - Although, that consistent is documented, it doesn't stop users from\n + Although, that behaviour is documented, it doesn't stop users from\n trying to use `--abbrev' with the hope of customising diff-patch's\n objects' name's abbreviation.\n \n - Let's resolve that inconsistency.\n + Let's allow the blob object names shown on the \"index\" line to be\n + abbreviated to arbitrary length given via the \"--abbrev\" option.\n \n To preserve backward compatibility with old script that specify both\n - `--full-index' and `--abbrev', always shows full object id\n + `--full-index' and `--abbrev', always show full object id\n if `--full-index' is specified.\n \n Signed-off-by: Đoàn Trần Công Danh \n @@ Documentation/diff-options.txt: endif::git-format-patch[]\n -\tdigits can be specified with `--abbrev=`.\n +\tlines, show only a partial prefix.\n +\tIn diff-patch output format, `--full-index` takes higher\n -+\tprecedent, i.e. if `--full-index` is specified, full blob\n ++\tprecedence, i.e. if `--full-index` is specified, full blob\n +\tnames will be shown regardless of `--abbrev`.\n +\tNon default number of digits can be specified with `--abbrev=`.\n \n-- \n2.28.0.143.g760df7782d.dirty\n\n"},{"id":"404133","messageId":"20200821120525.GA599@danh.dev","threadId":"54009","inReplyTo":"xmqqr1s1ayj2.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v4 1/2] t4013: improve diff-post-processor logic","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-08-21T12:05:25Z","receivedAt":"2020-08-21T12:05:47Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-08-20 12:49:05-0700, Junio C Hamano wrote:\n> > diff-processor logic.\n> >\n> > Let's fix the bug for all-zero object names and extend object names'\n> > abbrev to 16 while we're at it.\n> \n> \"and support abbreviation of object names up to 16 bytes\"?\n> \n> Also while we are at it, we fixed the post-processing not to touch\n> the file modes, which were mistakenly munged in the older code as if\n> they were object names abbreviated to 7 hexdigits.\n\nI've integrated your suggestion into newest series.\nI think you meant 6 hex-digits here, and I took the liberty to change\nto 7 digits.\n\nThanks,\n\n-- \nDanh\n"},{"id":"404144","messageId":"xmqqzh6o9f6o.fsf@gitster.c.googlers.com","threadId":"54009","inReplyTo":"20200821120525.GA599@danh.dev","subject":"Re: [PATCH v4 1/2] t4013: improve diff-post-processor logic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-21T15:44:31Z","receivedAt":"2020-08-21T15:44:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n> On 2020-08-20 12:49:05-0700, Junio C Hamano wrote:\n>> > diff-processor logic.\n>> >\n>> > Let's fix the bug for all-zero object names and extend object names'\n>> > abbrev to 16 while we're at it.\n>> \n>> \"and support abbreviation of object names up to 16 bytes\"?\n>> \n>> Also while we are at it, we fixed the post-processing not to touch\n>> the file modes, which were mistakenly munged in the older code as if\n>> they were object names abbreviated to 7 hexdigits.\n>\n> I've integrated your suggestion into newest series.\n> I think you meant 6 hex-digits here, and I took the liberty to change\n> to 7 digits.\n\nThanks, yeah, 100644 has 6 digits but I somehow couldn't count ;-)\n"}]}