{"thread":{"id":"60871","subject":"[PATCH] ref-filter.c: sort formatted dates by byte value","startedAt":"2024-02-08T01:57:23Z","lastAt":"2024-02-09T06:31:59Z","messageCount":4,"participants":["Victoria Dye via GitGitGadget","Junio C Hamano","Victoria Dye"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"488202","messageId":"pull.1655.git.1707357439586.gitgitgadget@gmail.com","threadId":"60871","inReplyTo":null,"subject":"[PATCH] ref-filter.c: sort formatted dates by byte value","fromName":"Victoria Dye via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-02-08T01:57:19Z","receivedAt":"2024-02-08T01:57:23Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"From: Victoria Dye <vdye@github.com>\n\nUpdate the ref sorting functions of 'ref-filter.c' so that when date fields\nare specified with a format string (such as in 'git for-each-ref\n--sort=creatordate:<something>'), they are sorted by their formatted string\nvalue rather than by the underlying numeric timestamp. Currently, date\nfields are always sorted by timestamp, regardless of whether formatting\ninformation is included in the '--sort' key.\n\nLeaving the default (unformatted) date sorting unchanged, sorting by the\nformatted date string adds some flexibility to 'for-each-ref' by allowing\nfor behavior like \"sort by year, then by refname within each year\" or \"sort\nby time of day\". Because the inclusion of a format string previously had no\neffect on sort behavior, this change likely will not affect existing usage\nof 'for-each-ref' or other ref listing commands.\n\nAdditionally, update documentation & tests to document the new sorting\nmechanism.\n\nSigned-off-by: Victoria Dye <vdye@github.com>\n---\n    ref-filter.c: sort formatted dates by byte value\n    \n    I came across a use case for 'git for-each-ref' at $DAYJOB in which I'd\n    want to sort by a portion of a formatted 'creatordate' (e.g., only the\n    time of day, sans date). When I tried to run something like 'git\n    for-each-ref --sort=creatordate:format:%H:%M:%S', though, I was\n    surprised to find that the refs were still sorted according to the full\n    date/time value (as they would be with '--sort=creatordate').\n    \n    This patch attempts to make date-based sorting a bit more flexible by\n    ordering based on the formatted date string if and only if a custom\n    format is specified. The implementation is fairly simple (manually set\n    the comparison type to 'FIELD_STR' if the format string is not null),\n    but I'm interested in hearing from reviewers whether this seems like a\n    reasonable extension to 'git for-each-ref --sort', or if there's another\n    (better) way to add this kind of functionality.\n    \n    Thanks!\n    \n     * Victoria\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1655%2Fvdye%2Fvdye%2Ffor-each-ref-date-sorting-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1655/vdye/vdye/for-each-ref-date-sorting-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1655\n\n Documentation/git-for-each-ref.txt |  8 ++++--\n ref-filter.c                       |  6 ++++\n t/t6300-for-each-ref.sh            | 46 ++++++++++++++++++++++++++++++\n 3 files changed, 57 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex be9543f6840..3a9ad91b7af 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -359,9 +359,11 @@ In any case, a field name that refers to a field inapplicable to\n the object referred by the ref does not cause an error.  It\n returns an empty string instead.\n \n-As a special case for the date-type fields, you may specify a format for\n-the date by adding `:` followed by date format name (see the\n-values the `--date` option to linkgit:git-rev-list[1] takes).\n+As a special case for the date-type fields, you may specify a format for the\n+date by adding `:` followed by date format name (see the values the `--date`\n+option to linkgit:git-rev-list[1] takes). If this formatting is provided in\n+a `--sort` key, references will be sorted according to the byte-value of the\n+formatted string rather than the numeric value of the underlying timestamp.\n \n Some atoms like %(align) and %(if) always require a matching %(end).\n We call them \"opening atoms\" and sometimes denote them as %($open).\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 35b989e1dfe..be14b56e324 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1611,6 +1611,12 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam\n \tif (formatp) {\n \t\tformatp++;\n \t\tparse_date_format(formatp, &date_mode);\n+\n+\t\t/*\n+\t\t * If this is a sort field and a format was specified, we'll\n+\t\t * want to compare formatted date by string value.\n+\t\t */\n+\t\tv->atom->type = FIELD_STR;\n \t}\n \n \tif (!eoemail)\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 843a7fe1431..eb6c8204e8b 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -1356,6 +1356,52 @@ test_expect_success '--no-sort without subsequent --sort prints expected refs' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'set up custom date sorting' '\n+\t# Dates:\n+\t# - Wed Feb 07 2024 21:34:20 +0000\n+\t# - Tue Dec 14 1999 00:05:22 +0000\n+\t# - Fri Jun 04 2021 11:26:51 +0000\n+\t# - Mon Jan 22 2007 16:44:01 GMT+0000\n+\ti=1 &&\n+\tfor when in 1707341660 945129922 1622806011 1169484241\n+\tdo\n+\t\tGIT_COMMITTER_DATE=\"@$when +0000\" \\\n+\t\tGIT_COMMITTER_EMAIL=\"user@example.com\" \\\n+\t\tgit tag -m \"tag $when\" custom-dates-$i &&\n+\t\ti=$(($i+1)) || return 1\n+\tdone\n+'\n+\n+test_expect_success 'sort by date defaults to full timestamp' '\n+\tcat >expected <<-\\EOF &&\n+\t945129922 refs/tags/custom-dates-2\n+\t1169484241 refs/tags/custom-dates-4\n+\t1622806011 refs/tags/custom-dates-3\n+\t1707341660 refs/tags/custom-dates-1\n+\tEOF\n+\n+\tgit for-each-ref \\\n+\t\t--format=\"%(creatordate:unix) %(refname)\" \\\n+\t\t--sort=creatordate \\\n+\t\t\"refs/tags/custom-dates-*\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'sort by custom date format' '\n+\tcat >expected <<-\\EOF &&\n+\t00:05:22 refs/tags/custom-dates-2\n+\t11:26:51 refs/tags/custom-dates-3\n+\t16:44:01 refs/tags/custom-dates-4\n+\t21:34:20 refs/tags/custom-dates-1\n+\tEOF\n+\n+\tgit for-each-ref \\\n+\t\t--format=\"%(creatordate:format:%H:%M:%S) %(refname)\" \\\n+\t\t--sort=\"creatordate:format:%H:%M:%S\" \\\n+\t\t\"refs/tags/custom-dates-*\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'do not dereference NULL upon %(HEAD) on unborn branch' '\n \ttest_when_finished \"git checkout main\" &&\n \tgit for-each-ref --format=\"%(HEAD) %(refname:short)\" refs/heads/ >actual &&\n\nbase-commit: 235986be822c9f8689be2e9a0b7804d0b1b6d821\n-- \ngitgitgadget\n"},{"id":"488205","messageId":"xmqqzfwbps43.fsf@gitster.g","threadId":"60871","inReplyTo":"pull.1655.git.1707357439586.gitgitgadget@gmail.com","subject":"Re: [PATCH] ref-filter.c: sort formatted dates by byte value","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-08T03:11:40Z","receivedAt":"2024-02-08T03:11:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Victoria Dye via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> Leaving the default (unformatted) date sorting unchanged, sorting by the\n> formatted date string adds some flexibility to 'for-each-ref' by allowing\n> for behavior like \"sort by year, then by refname within each year\" or \"sort\n> by time of day\".\n\nHmph, what a strange use case, but understandable.\n\n>     I came across a use case for 'git for-each-ref' at $DAYJOB in which I'd\n>     want to sort by a portion of a formatted 'creatordate' (e.g., only the\n>     time of day, sans date). When I tried to run something like 'git\n>     for-each-ref --sort=creatordate:format:%H:%M:%S',\n\nHmph, this indeed is interesting ;-)\n\nI wonder if there are other \"sort by numeric but the thing could be\nstringified by the end-user\" atoms offered by for-each-ref\nmachinery.  IOW, is the timestamp the only thing that needs this\nfix?\n\nThanks.\n"},{"id":"488264","messageId":"5ed018da-2150-42d8-995e-59a35a2e3821@github.com","threadId":"60871","inReplyTo":"xmqqzfwbps43.fsf@gitster.g","subject":"Re: [PATCH] ref-filter.c: sort formatted dates by byte value","fromName":"Victoria Dye","fromEmail":"vdye@github.com","sentAt":"2024-02-09T02:46:50Z","receivedAt":"2024-02-09T02:46:52Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"Junio C Hamano wrote:\n>>     I came across a use case for 'git for-each-ref' at $DAYJOB in which I'd\n>>     want to sort by a portion of a formatted 'creatordate' (e.g., only the\n>>     time of day, sans date). When I tried to run something like 'git\n>>     for-each-ref --sort=creatordate:format:%H:%M:%S',\n> \n> Hmph, this indeed is interesting ;-)\n> \n> I wonder if there are other \"sort by numeric but the thing could be\n> stringified by the end-user\" atoms offered by for-each-ref\n> machinery.  IOW, is the timestamp the only thing that needs this\n> fix?\n\nThe only non-FIELD_STR atoms other than the date ones are \"objectsize\" and\n\"numparent\". \"objectsize\" has an optional \":disk\" modifier, but that doesn't\nchange formatting (just the value of the integer printed). \"numparent\"\ndoesn't have any modifiers, it just prints the integer number of parents.\nOtherwise, everything is sorted by string value, so I think only the date\natoms have this kind of mismatch between formatted value and sort value.\n\n> \n> Thanks.\n\n"},{"id":"488266","messageId":"xmqqeddmkv1i.fsf@gitster.g","threadId":"60871","inReplyTo":"5ed018da-2150-42d8-995e-59a35a2e3821@github.com","subject":"Re: [PATCH] ref-filter.c: sort formatted dates by byte value","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-09T06:31:53Z","receivedAt":"2024-02-09T06:31:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Victoria Dye <vdye@github.com> writes:\n\n> Junio C Hamano wrote:\n>>>     I came across a use case for 'git for-each-ref' at $DAYJOB in which I'd\n>>>     want to sort by a portion of a formatted 'creatordate' (e.g., only the\n>>>     time of day, sans date). When I tried to run something like 'git\n>>>     for-each-ref --sort=creatordate:format:%H:%M:%S',\n>> \n>> Hmph, this indeed is interesting ;-)\n>> \n>> I wonder if there are other \"sort by numeric but the thing could be\n>> stringified by the end-user\" atoms offered by for-each-ref\n>> machinery.  IOW, is the timestamp the only thing that needs this\n>> fix?\n>\n> The only non-FIELD_STR atoms other than the date ones are \"objectsize\" and\n> \"numparent\". \"objectsize\" has an optional \":disk\" modifier, but that doesn't\n> change formatting (just the value of the integer printed). \"numparent\"\n> doesn't have any modifiers, it just prints the integer number of parents.\n> Otherwise, everything is sorted by string value, so I think only the date\n> atoms have this kind of mismatch between formatted value and sort value.\n\nThanks.\n"}]}