[PATCH] ref-filter.c: sort formatted dates by byte value

["Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com>]

From: Victoria Dye <vdye@github.com>

Update the ref sorting functions of 'ref-filter.c' so that when date fields
are specified with a format string (such as in 'git for-each-ref
--sort=creatordate:<something>'), they are sorted by their formatted string
value rather than by the underlying numeric timestamp. Currently, date
fields are always sorted by timestamp, regardless of whether formatting
information is included in the '--sort' key.

Leaving the default (unformatted) date sorting unchanged, sorting by the
formatted date string adds some flexibility to 'for-each-ref' by allowing
for behavior like "sort by year, then by refname within each year" or "sort
by time of day". Because the inclusion of a format string previously had no
effect on sort behavior, this change likely will not affect existing usage
of 'for-each-ref' or other ref listing commands.

Additionally, update documentation & tests to document the new sorting
mechanism.

Signed-off-by: Victoria Dye <vdye@github.com>
---
    ref-filter.c: sort formatted dates by byte value
    
    I came across a use case for 'git for-each-ref' at $DAYJOB in which I'd
    want to sort by a portion of a formatted 'creatordate' (e.g., only the
    time of day, sans date). When I tried to run something like 'git
    for-each-ref --sort=creatordate:format:%H:%M:%S', though, I was
    surprised to find that the refs were still sorted according to the full
    date/time value (as they would be with '--sort=creatordate').
    
    This patch attempts to make date-based sorting a bit more flexible by
    ordering based on the formatted date string if and only if a custom
    format is specified. The implementation is fairly simple (manually set
    the comparison type to 'FIELD_STR' if the format string is not null),
    but I'm interested in hearing from reviewers whether this seems like a
    reasonable extension to 'git for-each-ref --sort', or if there's another
    (better) way to add this kind of functionality.
    
    Thanks!
    
     * Victoria

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1655%2Fvdye%2Fvdye%2Ffor-each-ref-date-sorting-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1655/vdye/vdye/for-each-ref-date-sorting-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1655

 Documentation/git-for-each-ref.txt |  8 ++++--
 ref-filter.c                       |  6 ++++
 t/t6300-for-each-ref.sh            | 46 ++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index be9543f6840..3a9ad91b7af 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -359,9 +359,11 @@ In any case, a field name that refers to a field inapplicable to
 the object referred by the ref does not cause an error.  It
 returns an empty string instead.
 
-As a special case for the date-type fields, you may specify a format for
-the date by adding `:` followed by date format name (see the
-values the `--date` option to linkgit:git-rev-list[1] takes).
+As a special case for the date-type fields, you may specify a format for the
+date by adding `:` followed by date format name (see the values the `--date`
+option to linkgit:git-rev-list[1] takes). If this formatting is provided in
+a `--sort` key, references will be sorted according to the byte-value of the
+formatted string rather than the numeric value of the underlying timestamp.
 
 Some atoms like %(align) and %(if) always require a matching %(end).
 We call them "opening atoms" and sometimes denote them as %($open).
diff --git a/ref-filter.c b/ref-filter.c
index 35b989e1dfe..be14b56e324 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1611,6 +1611,12 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
 	if (formatp) {
 		formatp++;
 		parse_date_format(formatp, &date_mode);
+
+		/*
+		 * If this is a sort field and a format was specified, we'll
+		 * want to compare formatted date by string value.
+		 */
+		v->atom->type = FIELD_STR;
 	}
 
 	if (!eoemail)
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 843a7fe1431..eb6c8204e8b 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -1356,6 +1356,52 @@ test_expect_success '--no-sort without subsequent --sort prints expected refs' '
 	test_cmp expected actual
 '
 
+test_expect_success 'set up custom date sorting' '
+	# Dates:
+	# - Wed Feb 07 2024 21:34:20 +0000
+	# - Tue Dec 14 1999 00:05:22 +0000
+	# - Fri Jun 04 2021 11:26:51 +0000
+	# - Mon Jan 22 2007 16:44:01 GMT+0000
+	i=1 &&
+	for when in 1707341660 945129922 1622806011 1169484241
+	do
+		GIT_COMMITTER_DATE="@$when +0000" \
+		GIT_COMMITTER_EMAIL="user@example.com" \
+		git tag -m "tag $when" custom-dates-$i &&
+		i=$(($i+1)) || return 1
+	done
+'
+
+test_expect_success 'sort by date defaults to full timestamp' '
+	cat >expected <<-\EOF &&
+	945129922 refs/tags/custom-dates-2
+	1169484241 refs/tags/custom-dates-4
+	1622806011 refs/tags/custom-dates-3
+	1707341660 refs/tags/custom-dates-1
+	EOF
+
+	git for-each-ref \
+		--format="%(creatordate:unix) %(refname)" \
+		--sort=creatordate \
+		"refs/tags/custom-dates-*" >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'sort by custom date format' '
+	cat >expected <<-\EOF &&
+	00:05:22 refs/tags/custom-dates-2
+	11:26:51 refs/tags/custom-dates-3
+	16:44:01 refs/tags/custom-dates-4
+	21:34:20 refs/tags/custom-dates-1
+	EOF
+
+	git for-each-ref \
+		--format="%(creatordate:format:%H:%M:%S) %(refname)" \
+		--sort="creatordate:format:%H:%M:%S" \
+		"refs/tags/custom-dates-*" >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'do not dereference NULL upon %(HEAD) on unborn branch' '
 	test_when_finished "git checkout main" &&
 	git for-each-ref --format="%(HEAD) %(refname:short)" refs/heads/ >actual &&

base-commit: 235986be822c9f8689be2e9a0b7804d0b1b6d821
-- 
gitgitgadget