From 72ffcb341416a919b6209420c00dfa0f4f6f5b7b Mon Sep 17 00:00:00 2001 From: John Kerl Date: Wed, 8 Jul 2026 16:42:03 -0400 Subject: [PATCH] Support %a %A %e %h %c %x %X in strptime (#1518) (#2179) Miller's strftime delegates to the full-featured lestrrat-go/strftime library, but strptime uses an in-tree fork of a small subset-only package, so strftime output couldn't always be parsed back by strptime with the same format string. This adds the format codes needed to round-trip common formats like "%a %b %e %T %Y": * %a / %A (weekday name) and %h (alias of %b): straightforward formatMap entries, same pattern as the existing %b/%B. * %e (space-padded day of month): needed dedicated width-detection logic, since %e's own optional leading pad space is otherwise indistinguishable from a literal separator space when the code searches for the next literal text to bound a field -- this was the root cause of the round-trip failure in the linked issue. * %c, %x, %X: shorthand aliases (expanding to %a %b %e %H:%M:%S %Y, %m/%d/%y, and %H:%M:%S respectively), same mechanism already used for %T/%D/%F/%R/%r. Also documents the %D/%F/%r/%R/%T shorthands in the strptime table in reference-dsl-time.md.in, which were already supported but missing from the docs -- this was the exact confusion reported in the discussion linked from #1518. Round-trip tests added in pkg/bifs/datetime_test.go assert strptime(strftime(t, fmt), fmt) == t across the newly-supported formats, plus table-driven cases in pkg/pbnjay-strptime/strptime_test.go covering %e's edge cases (padded/unpadded single digit, double digit, adjacent to another code with no separator, at end of string). Co-authored-by: Claude Sonnet 5 --- docs/src/manpage.md | 2 +- docs/src/manpage.txt | 2 +- docs/src/reference-dsl-time.md | 12 ++ docs/src/reference-dsl-time.md.in | 12 ++ man/manpage.txt | 2 +- man/mlr.1 | 4 +- pkg/bifs/datetime_test.go | 98 +++++++++++++++ pkg/pbnjay-strptime/strptime.go | 119 +++++++++++++----- pkg/pbnjay-strptime/strptime_test.go | 77 +++++++++++- .../dsl-gmt-date-time-functions/0020/mlr | 2 +- 10 files changed, 290 insertions(+), 40 deletions(-) create mode 100644 pkg/bifs/datetime_test.go diff --git a/docs/src/manpage.md b/docs/src/manpage.md index ad1848539..41b5b3676 100644 --- a/docs/src/manpage.md +++ b/docs/src/manpage.md @@ -4110,5 +4110,5 @@ This is simply a copy of what you should see on running `man mlr` at a command p MIME Type for Comma-Separated Values (CSV) Files, the Miller docsite https://miller.readthedocs.io - 2026-07-07 4mMILLER24m(1) + 2026-07-08 4mMILLER24m(1) diff --git a/docs/src/manpage.txt b/docs/src/manpage.txt index ebbd0a727..a5e8bd3b8 100644 --- a/docs/src/manpage.txt +++ b/docs/src/manpage.txt @@ -4089,4 +4089,4 @@ MIME Type for Comma-Separated Values (CSV) Files, the Miller docsite https://miller.readthedocs.io - 2026-07-07 4mMILLER24m(1) + 2026-07-08 4mMILLER24m(1) diff --git a/docs/src/reference-dsl-time.md b/docs/src/reference-dsl-time.md index 6f93d6e98..9c492a7d9 100644 --- a/docs/src/reference-dsl-time.md +++ b/docs/src/reference-dsl-time.md @@ -298,17 +298,29 @@ Available format strings for `strptime`: | Pattern | Description | |---------|-------------| | `%%` | A literal '%' character. | +| `%a` | Weekday as locale’s abbreviated name. | +| `%A` | Weekday as locale’s full name. | | `%b` | Month as locale’s abbreviated name. | | `%B` | Month as locale’s full name. | +| `%c` | Locale’s date and time representation. Equivalent to `%a %b %e %H:%M:%S %Y`. | | `%d` | Day of the month as a zero-padded decimal number. | +| `%D` | Equivalent to `%m/%d/%y`. | +| `%e` | Day of the month as a space-padded decimal number. | | `%f` | Microsecond as a decimal number, zero-padded on the left. | +| `%F` | Equivalent to `%Y-%m-%d`. | +| `%h` | Same as `%b`. | | `%H` | Hour (24-hour clock) as a zero-padded decimal number. | | `%I` | Hour (12-hour clock) as a zero-padded decimal number. | | `%j` | Three-digit day of year, like 004 or 363. | | `%m` | Month as a zero-padded decimal number. | | `%M` | Minute as a zero-padded decimal number. | | `%p` | Locale’s equivalent of either AM or PM. | +| `%r` | Equivalent to `%I:%M:%S %p`. | +| `%R` | Equivalent to `%H:%M`. | | `%S` | Second as a zero-padded decimal number. | +| `%T` | Equivalent to `%H:%M:%S`. | +| `%x` | Locale’s date representation. Equivalent to `%m/%d/%y`. | +| `%X` | Locale’s time representation. Equivalent to `%H:%M:%S`. | | `%y` | Year without century as a zero-padded decimal number. | | `%Y` | Year with century as a decimal number. | | `%z` | UTC offset in the form +HHMM or -HHMM. | diff --git a/docs/src/reference-dsl-time.md.in b/docs/src/reference-dsl-time.md.in index 4a7268d09..4ff0c9a7b 100644 --- a/docs/src/reference-dsl-time.md.in +++ b/docs/src/reference-dsl-time.md.in @@ -230,17 +230,29 @@ Available format strings for `strptime`: | Pattern | Description | |---------|-------------| | `%%` | A literal '%' character. | +| `%a` | Weekday as locale’s abbreviated name. | +| `%A` | Weekday as locale’s full name. | | `%b` | Month as locale’s abbreviated name. | | `%B` | Month as locale’s full name. | +| `%c` | Locale’s date and time representation. Equivalent to `%a %b %e %H:%M:%S %Y`. | | `%d` | Day of the month as a zero-padded decimal number. | +| `%D` | Equivalent to `%m/%d/%y`. | +| `%e` | Day of the month as a space-padded decimal number. | | `%f` | Microsecond as a decimal number, zero-padded on the left. | +| `%F` | Equivalent to `%Y-%m-%d`. | +| `%h` | Same as `%b`. | | `%H` | Hour (24-hour clock) as a zero-padded decimal number. | | `%I` | Hour (12-hour clock) as a zero-padded decimal number. | | `%j` | Three-digit day of year, like 004 or 363. | | `%m` | Month as a zero-padded decimal number. | | `%M` | Minute as a zero-padded decimal number. | | `%p` | Locale’s equivalent of either AM or PM. | +| `%r` | Equivalent to `%I:%M:%S %p`. | +| `%R` | Equivalent to `%H:%M`. | | `%S` | Second as a zero-padded decimal number. | +| `%T` | Equivalent to `%H:%M:%S`. | +| `%x` | Locale’s date representation. Equivalent to `%m/%d/%y`. | +| `%X` | Locale’s time representation. Equivalent to `%H:%M:%S`. | | `%y` | Year without century as a zero-padded decimal number. | | `%Y` | Year with century as a decimal number. | | `%z` | UTC offset in the form +HHMM or -HHMM. | diff --git a/man/manpage.txt b/man/manpage.txt index ebbd0a727..a5e8bd3b8 100644 --- a/man/manpage.txt +++ b/man/manpage.txt @@ -4089,4 +4089,4 @@ MIME Type for Comma-Separated Values (CSV) Files, the Miller docsite https://miller.readthedocs.io - 2026-07-07 4mMILLER24m(1) + 2026-07-08 4mMILLER24m(1) diff --git a/man/mlr.1 b/man/mlr.1 index efcfdf33e..272395a6a 100644 --- a/man/mlr.1 +++ b/man/mlr.1 @@ -2,12 +2,12 @@ .\" Title: mlr .\" Author: [see the "AUTHOR" section] .\" Generator: ./mkman.rb -.\" Date: 2026-07-07 +.\" Date: 2026-07-08 .\" Manual: \ \& .\" Source: \ \& .\" Language: English .\" -.TH "MILLER" "1" "2026-07-07" "\ \&" "\ \&" +.TH "MILLER" "1" "2026-07-08" "\ \&" "\ \&" .\" ----------------------------------------------------------------- .\" * Portability definitions .\" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/pkg/bifs/datetime_test.go b/pkg/bifs/datetime_test.go new file mode 100644 index 000000000..ba6ae673a --- /dev/null +++ b/pkg/bifs/datetime_test.go @@ -0,0 +1,98 @@ +package bifs + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/johnkerl/miller/v6/pkg/mlrval" +) + +// TestBIF_strftime_strptime_roundtrip checks that for each of a set of formats +// exercising the newly-added %a %A %e %h %x %X %c strptime codes, +// strptime(strftime(t, fmt), fmt) == t. This is the exact failure mode from +// https://github.com/johnkerl/miller/issues/1518, where strptime accepted fewer +// format codes than strftime produced. +func TestBIF_strftime_strptime_roundtrip(t *testing.T) { + formats := []string{ + "%a %b %e %T %Y", + "%A %b %e %T %Y", + "%a %h %e %T %Y", + "%A %h %e %T %Y", + "%c", + "%x %X", + "%Y-%m-%d %H:%M:%S", + } + + epochSecondsValues := []int64{ + 0, // 1970-01-01, single-digit day + 1709552468, // 2024-03-04 11:41:08, single-digit day + 1729555200, // 2024-10-22 00:00:00, double-digit day + 1735732805, // 2025-01-01 12:00:05, single-digit day, new year + } + + for _, format := range formats { + for _, epochSeconds := range epochSecondsValues { + formatted := BIF_strftime(mlrval.FromInt(epochSeconds), mlrval.FromString(format)) + formattedString, isString := formatted.GetStringValue() + assert.True(t, isString, "strftime(%d, %q) produced non-string %v", epochSeconds, format, formatted) + + parsed := BIF_strptime(mlrval.FromString(formattedString), mlrval.FromString(format)) + parsedSeconds, isNumeric := parsed.GetNumericToFloatValue() + assert.True(t, isNumeric, "strptime(%q, %q) produced non-numeric %v", formattedString, format, parsed) + + assert.Equal( + t, float64(epochSeconds), parsedSeconds, + "round-trip mismatch for format %q: strftime(%d) = %q, strptime(...) = %v", + format, epochSeconds, formattedString, parsedSeconds, + ) + } + } +} + +// TestBIF_strptime_new_format_codes exercises the newly-supported %a %A %e %h +// %x %X %c strptime format codes directly, including %e's space-padding edge +// cases (single-digit day with/without padding, double-digit day, %e at the +// end of the string, and %e directly adjacent to another format code). +func TestBIF_strptime_new_format_codes(t *testing.T) { + type testCase struct { + input string + format string + want float64 + } + + cases := []testCase{ + {"Mon Mar 4 11:41:08 2024", "%a %b %e %T %Y", 1709552468}, + {"Monday Mar 4 11:41:08 2024", "%A %b %e %T %Y", 1709552468}, + {"Mon Mar 4 11:41:08 2024", "%a %h %e %T %Y", 1709552468}, + {"Tue Oct 22 00:00:00 2024", "%a %b %e %T %Y", 1729555200}, + {"Mon Mar 4 11:41:08 2024", "%c", 1709552468}, + {"03/04/24 11:41:08", "%x %X", 1709552468}, + // %e with a single (non-padded) space before a single-digit day. + {"Mar 4 2024", "%b %e %Y", 1709510400}, + // %e with two-digit day. + {"Mar 14 2024", "%b %e %Y", 1710374400}, + // %e directly adjacent to another format code, no intervening text. + {"4Mar2024", "%e%b%Y", 1709510400}, + {"14Mar2024", "%e%b%Y", 1710374400}, + // %e at the very end of the string. + {"2024-03-4", "%Y-%m-%e", 1709510400}, + {"2024-03-14", "%Y-%m-%e", 1710374400}, + } + + for _, c := range cases { + output := BIF_strptime(mlrval.FromString(c.input), mlrval.FromString(c.format)) + seconds, isNumeric := output.GetNumericToFloatValue() + assert.True(t, isNumeric, "strptime(%q, %q) produced non-numeric %v", c.input, c.format, output) + assert.Equal(t, c.want, seconds, "strptime(%q, %q)", c.input, c.format) + } +} + +// TestBIF_strptime_still_unsupported_format_code checks that a format code +// which remains unsupported (%U, week-of-year) still errors, i.e. that adding +// the new codes didn't accidentally cause unsupported codes to be silently +// ignored. +func TestBIF_strptime_still_unsupported_format_code(t *testing.T) { + output := BIF_strptime(mlrval.FromString("2024-03-04 09"), mlrval.FromString("%Y-%m-%d %U")) + assert.True(t, output.IsError()) +} diff --git a/pkg/pbnjay-strptime/strptime.go b/pkg/pbnjay-strptime/strptime.go index 4c9f1949e..36b50df27 100644 --- a/pkg/pbnjay-strptime/strptime.go +++ b/pkg/pbnjay-strptime/strptime.go @@ -23,9 +23,13 @@ THE SOFTWARE. // Package strptime provides a C-style strptime wrappers for time.Parse. // // It supports the following subset of format strings (stolen from python docs): +// %a Weekday as locale’s abbreviated name. +// %A Weekday as locale’s full name. // %d Day of the month as a zero-padded decimal number. +// %e Day of the month as a space-padded decimal number. // %b Month as locale’s abbreviated name. // %B Month as locale’s full name. +// %h Month as locale’s abbreviated name (alias of %b). // %m Month as a zero-padded decimal number. // %y Year without century as a zero-padded decimal number. // %Y Year with century as a decimal number. @@ -40,6 +44,9 @@ THE SOFTWARE. // "2022-04-03 17:38:20.123 UTC" all parse with "%Y-%m-%d %H:%M:%S.%f UTC"). // %z UTC offset in the form +HHMM or -HHMM. // %Z Time zone name. UTC, EST, CST +// %c Locale’s date and time representation. Shorthand for "%a %b %e %H:%M:%S %Y". +// %x Locale’s date representation. Shorthand for "%m/%d/%y". +// %X Locale’s time representation. Shorthand for "%H:%M:%S". // %% A literal '%' character. // // BUG(pbnjay): If an unsupported specifier is used, it may NOT directly precede a @@ -225,38 +232,46 @@ func strptime_tz( interveningLen := sil // Now sil becomes the offset of this part within the strptime-style input. if sil > 0 { - // Optional ".%f" (mixed modes): when format has ".%f" and next format code is 'f', - // the literal "." may be absent (e.g. "2022-04-03 17:38:20 UTC" vs "2022-04-03 17:38:20.123 UTC"). - dotOptional := false - if partsIndex+1 < nparts { - nextPart := partsBetweenPercentSigns[partsIndex+1] - if len(partBetweenPercentSigns) > 0 && - partBetweenPercentSigns[len(partBetweenPercentSigns)-1] == '.' && - len(nextPart) > 0 && nextPart[0] == 'f' { - dotOptional = true - } - } - if dotOptional { - dotSearch := partBetweenPercentSigns[1:] - sil = strings.Index(strptimeInput[inputIdx:], dotSearch) - if sil == -1 { - // Try without the dot: find the first char of the literal after %f - if partsIndex+1 < nparts { - nextPart := partsBetweenPercentSigns[partsIndex+1] - if len(nextPart) > 1 { - fallbackSearch := string(nextPart[1]) - sil = strings.Index(strptimeInput[inputIdx:], fallbackSearch) - if sil != -1 { - interveningLen = 0 - } - } else { - sil = len(strptimeInput) - inputIdx - interveningLen = 0 - } + if formatCode == 'e' { + // %e's own optional leading pad space is indistinguishable from a + // literal separator space (e.g. "%b %e %T" matching "Mar 4 ..."), so a + // plain search for the next literal text would stop at %e's own padding. + // Determine the field width directly instead. + sil = spacePaddedDayLen(strptimeInput[inputIdx:]) + } else { + // Optional ".%f" (mixed modes): when format has ".%f" and next format code is 'f', + // the literal "." may be absent (e.g. "2022-04-03 17:38:20 UTC" vs "2022-04-03 17:38:20.123 UTC"). + dotOptional := false + if partsIndex+1 < nparts { + nextPart := partsBetweenPercentSigns[partsIndex+1] + if len(partBetweenPercentSigns) > 0 && + partBetweenPercentSigns[len(partBetweenPercentSigns)-1] == '.' && + len(nextPart) > 0 && nextPart[0] == 'f' { + dotOptional = true } } - } else { - sil = strings.Index(strptimeInput[inputIdx:], partBetweenPercentSigns[1:]) + if dotOptional { + dotSearch := partBetweenPercentSigns[1:] + sil = strings.Index(strptimeInput[inputIdx:], dotSearch) + if sil == -1 { + // Try without the dot: find the first char of the literal after %f + if partsIndex+1 < nparts { + nextPart := partsBetweenPercentSigns[partsIndex+1] + if len(nextPart) > 1 { + fallbackSearch := string(nextPart[1]) + sil = strings.Index(strptimeInput[inputIdx:], fallbackSearch) + if sil != -1 { + interveningLen = 0 + } + } else { + sil = len(strptimeInput) - inputIdx + interveningLen = 0 + } + } + } + } else { + sil = strings.Index(strptimeInput[inputIdx:], partBetweenPercentSigns[1:]) + } } } if sil == -1 { @@ -273,13 +288,22 @@ func strptime_tz( if supported { // Accumulate the go-lib style template and input strings. if sil == 0 { // No intervening text, e.g. "%Y%m%d" - if formatCode == 'f' { + switch formatCode { + case 'f': // %f is optional decimal point + 0-6 digit runes (microseconds). // Supports mixed modes: no fraction ("20 UTC"), dot only ("20. UTC"), or digits ("20.123 UTC"). // Do not consume the rest of the string so that %f%z works: // e.g. ".160001+0100" -> %f takes ".160001", %z takes "+0100". sil, fracHasDigits = parseFracLen(strptimeInput[inputIdx:]) - } else { + case 'e': + sil = spacePaddedDayLen(strptimeInput[inputIdx:]) + if sil == -1 { + if _debug { + fmt.Printf("format/template mismatch 2\n") + } + return time.Time{}, ErrFormatMismatch + } + default: want := len(templateComponent) remaining := len(strptimeInput) - inputIdx if remaining == 0 { @@ -414,17 +438,42 @@ func parseFracLen(s string) (length int, hasDigits bool) { return n, digits > 0 } +// spacePaddedDayLen returns the byte length of a strptime %e field in s: a day-of-month +// as either a single digit ("4"), a space followed by a digit (" 4"), or two digits +// ("14"). Returns -1 if s does not start with a valid %e field. +func spacePaddedDayLen(s string) int { + isDigit := func(b byte) bool { return b >= '0' && b <= '9' } + if s == "" { + return -1 + } + if s[0] == ' ' { + if len(s) > 1 && isDigit(s[1]) { + return 2 + } + return -1 + } + if !isDigit(s[0]) { + return -1 + } + if len(s) > 1 && isDigit(s[1]) { + return 2 + } + return 1 +} + // expandShorthands handles some shorthands that the C library uses, which we can easily // replicate -- e.g. "%F" is "%Y-%m-%d". func expandShorthands(format string) string { // TODO: mem cache + format = strings.ReplaceAll(format, "%c", "%a %b %e %H:%M:%S %Y") + format = strings.ReplaceAll(format, "%x", "%m/%d/%y") + format = strings.ReplaceAll(format, "%X", "%H:%M:%S") format = strings.ReplaceAll(format, "%T", "%H:%M:%S") format = strings.ReplaceAll(format, "%D", "%m/%d/%y") format = strings.ReplaceAll(format, "%F", "%Y-%m-%d") format = strings.ReplaceAll(format, "%R", "%H:%M") format = strings.ReplaceAll(format, "%r", "%I:%M:%S %p") format = strings.ReplaceAll(format, "%T", "%H:%M:%S") - // We've no %e in this package // format = strings.ReplaceAll(format, "%v", "%e-%b-%Y") return format } @@ -437,9 +486,13 @@ var ( ErrFormatUnsupported = errors.New("date format contains unsupported percent-encodings") formatMap = map[int]string{ + 'a': "Mon", + 'A': "Monday", 'b': "Jan", 'B': "January", + 'h': "Jan", 'd': "02", + 'e': "_2", 'f': "999999", 'H': "15", 'I': "03", diff --git a/pkg/pbnjay-strptime/strptime_test.go b/pkg/pbnjay-strptime/strptime_test.go index e158b27bd..fa4e2d8bf 100644 --- a/pkg/pbnjay-strptime/strptime_test.go +++ b/pkg/pbnjay-strptime/strptime_test.go @@ -58,7 +58,7 @@ var testData = []testDataType{ }, { "01/02/70 14:20", - "%D %X", // no such format code + "%D %U", // no such format code false, 0, }, @@ -151,6 +151,81 @@ var testData = []testDataType{ true, 1649007500, // .1 second = 1649007500.1 in float, but Unix() truncates to 1649007500 }, + + // %a / %A (weekday name) and %e (space-padded day of month), and %h (alias of + // %b). This is the exact repro from https://github.com/johnkerl/miller/issues/1518: + // strptime rejected format codes that strftime itself produces. + { + "Mon Mar 4 11:41:08 2024", + "%a %b %e %T %Y", + true, + 1709552468, + }, + { + "Monday Mar 4 11:41:08 2024", + "%A %b %e %T %Y", + true, + 1709552468, + }, + { + "Mon Mar 4 11:41:08 2024", + "%a %h %e %T %Y", + true, + 1709552468, + }, + // %e with a double-digit day -- no space padding present in the source. + { + "Tue Oct 22 00:00:00 2024", + "%a %b %e %T %Y", + true, + 1729555200, + }, + // %e with a single (non-padded) space before a single-digit day. + { + "Mar 4 2024", + "%b %e %Y", + true, + 1709510400, + }, + // %e directly adjacent to another format code, no intervening literal text. + { + "4Mar2024", + "%e%b%Y", + true, + 1709510400, + }, + { + "14Mar2024", + "%e%b%Y", + true, + 1710374400, + }, + // %e at the very end of the string. + { + "2024-03-4", + "%Y-%m-%e", + true, + 1709510400, + }, + { + "2024-03-14", + "%Y-%m-%e", + true, + 1710374400, + }, + // %c, %x, %X shorthands. + { + "Mon Mar 4 11:41:08 2024", + "%c", + true, + 1709552468, + }, + { + "03/04/24 11:41:08", + "%x %X", + true, + 1709552468, + }, } func TestStrptime(t *testing.T) { diff --git a/test/cases/dsl-gmt-date-time-functions/0020/mlr b/test/cases/dsl-gmt-date-time-functions/0020/mlr index b582dd920..baa570fe0 100644 --- a/test/cases/dsl-gmt-date-time-functions/0020/mlr +++ b/test/cases/dsl-gmt-date-time-functions/0020/mlr @@ -7,6 +7,6 @@ end { print strptime("1970:363", "%Y:%j"); print strptime("1970-01-01 10:20:30 PM", "%F %r"); print strptime("01/02/70 14:20", "%D %R"); - print strptime("01/02/70 14:20", "%D %X"); # no such format code + print strptime("01/02/70 14:20", "%D %U"); # no such format code print fmtnum(strptime("1971-01-01T00:00:00.123456Z", "%Y-%m-%dT%H:%M:%S.%fZ"), "%.6f"); }