Skip to contentSkip to conversation

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

PATCH In review Victoria Dye via GitGitGadget 3 participants 4 messages Feb 8, 2024 – Feb 9, 2024 Documentation

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ref-filter.c: sort formatted dates by byte value

From: Victoria Dye <[email protected]>

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.

---
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(-)
Documentation/git-for-each-ref.txt +5 -3
index be9543f6840..3a9ad91b7af 100644
@@ -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).
ref-filter.c +6 -0
index 35b989e1dfe..be14b56e324 100644
@@ -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)
t/t6300-for-each-ref.sh +46 -0

52 lines not on this page · raw message

base-commit: 235986be822c9f8689be2e9a0b7804d0b1b6d821

Signature · 1 line
gitgitgadget
#1 lore raw
Quoted 4 lines · Victoria Dye via GitGitGadget

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".

Hmph, what a strange use case, but understandable.

Quoted 4 lines

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',

Hmph, this indeed is interesting ;-)

I wonder if there are other "sort by numeric but the thing could be stringified by the end-user" atoms offered by for-each-ref machinery. IOW, is the timestamp the only thing that needs this fix?

Thanks.

#2 lore raw
Quoted 11 lines · Junio C Hamano
Quoted 4 lines

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',

Hmph, this indeed is interesting ;-)

I wonder if there are other "sort by numeric but the thing could be stringified by the end-user" atoms offered by for-each-ref machinery. IOW, is the timestamp the only thing that needs this fix?

The only non-FIELD_STR atoms other than the date ones are "objectsize" and "numparent". "objectsize" has an optional ":disk" modifier, but that doesn't change formatting (just the value of the integer printed). "numparent" doesn't have any modifiers, it just prints the integer number of parents. Otherwise, everything is sorted by string value, so I think only the date atoms have this kind of mismatch between formatted value and sort value.

Quoted 2 lines

Thanks.

#3 lore raw
Quoted 13 lines · Victoria Dye
Quoted 11 lines · Junio C Hamano
Quoted 4 lines

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',

Hmph, this indeed is interesting ;-)

I wonder if there are other "sort by numeric but the thing could be stringified by the end-user" atoms offered by for-each-ref machinery. IOW, is the timestamp the only thing that needs this fix?

Quoted 6 lines

The only non-FIELD_STR atoms other than the date ones are "objectsize" and "numparent". "objectsize" has an optional ":disk" modifier, but that doesn't change formatting (just the value of the integer printed). "numparent" doesn't have any modifiers, it just prints the integer number of parents. Otherwise, everything is sorted by string value, so I think only the date atoms have this kind of mismatch between formatted value and sort value.

Thanks.