Skip to content

Commit ebdef70

Browse files
committed
refactor(time-range): address human review feedback
- Make PERIOD_BRIEF and all error message dates dynamic (relative to today) - Use new Set(Object.keys(UNIT_SECONDS)) for PERIOD_UNITS - Remove unnecessary ?? on .at(-1) (use as-cast, length pre-checked) - Accept space separator as T alternative in datetime parsing - Add test for space-separated datetime input
1 parent 19e2d68 commit ebdef70

File tree

2 files changed

+40
-19
lines changed

2 files changed

+40
-19
lines changed

src/lib/time-range.ts

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,6 @@ export type TimeRangeApiParams = {
4646
// Constants
4747
// ---------------------------------------------------------------------------
4848

49-
/** Brief text for --period flag help, shared across commands */
50-
export const PERIOD_BRIEF =
51-
'Time range: "7d", "2024-01-01..2024-02-01", ">=2024-01-01"';
52-
53-
/** Valid unit suffixes for relative period strings */
54-
const PERIOD_UNITS = "smhdw";
55-
5649
/** Seconds per unit for relative period computation */
5750
const UNIT_SECONDS: Record<string, number> = {
5851
s: 1,
@@ -62,6 +55,23 @@ const UNIT_SECONDS: Record<string, number> = {
6255
w: 604_800,
6356
};
6457

58+
/** Valid unit suffixes for relative period strings, derived from UNIT_SECONDS */
59+
const PERIOD_UNITS = new Set(Object.keys(UNIT_SECONDS));
60+
61+
/**
62+
* Dynamic example dates relative to "today" so examples never look stale.
63+
* Computed once at module load.
64+
*/
65+
const EXAMPLE_START = (() => {
66+
const d = new Date();
67+
d.setMonth(d.getMonth() - 1);
68+
return d.toISOString().slice(0, 10);
69+
})();
70+
const EXAMPLE_END = new Date().toISOString().slice(0, 10);
71+
72+
/** Brief text for --period flag help, shared across commands */
73+
export const PERIOD_BRIEF = `Time range: "7d", "${EXAMPLE_START}..${EXAMPLE_END}", ">=${EXAMPLE_START}"`;
74+
6575
/**
6676
* Try to parse a relative period string (e.g., "7d") into its numeric value and unit.
6777
* Returns null if the string isn't a valid relative period.
@@ -72,8 +82,8 @@ function parseRelativeParts(
7282
if (value.length < 2) {
7383
return null;
7484
}
75-
const unit = value.at(-1) ?? "";
76-
if (!PERIOD_UNITS.includes(unit)) {
85+
const unit = value.at(-1) as string;
86+
if (!PERIOD_UNITS.has(unit)) {
7787
return null;
7888
}
7989
const numStr = value.slice(0, -1);
@@ -106,7 +116,7 @@ function validateDate(raw: string): Date {
106116
const d = new Date(raw);
107117
if (Number.isNaN(d.getTime())) {
108118
throw new ValidationError(
109-
`Invalid date: '${raw}'. Expected ISO-8601 format (e.g., 2024-01-01 or 2024-01-01T12:00:00).`,
119+
`Invalid date: '${raw}'. Expected ISO-8601 format (e.g., ${EXAMPLE_START} or ${EXAMPLE_START}T12:00:00).`,
110120
"period"
111121
);
112122
}
@@ -128,18 +138,23 @@ export function parseDate(raw: string, position: DatePosition): string {
128138
const trimmed = raw.trim();
129139
if (trimmed.length === 0) {
130140
throw new ValidationError(
131-
"Empty date value. Expected ISO-8601 format (e.g., 2024-01-01)",
141+
`Empty date value. Expected ISO-8601 format (e.g., ${EXAMPLE_START})`,
132142
"period"
133143
);
134144
}
135145

146+
// Normalize space-separated datetime to ISO "T" separator (common user input)
147+
const normalized = trimmed.includes(" ")
148+
? trimmed.replace(" ", "T")
149+
: trimmed;
150+
136151
// Date-only (no "T"): apply position-aware time boundaries in local time
137-
if (!trimmed.includes("T")) {
138-
return normalizeDateOnly(trimmed, position);
152+
if (!normalized.includes("T")) {
153+
return normalizeDateOnly(normalized, position);
139154
}
140155

141156
// Datetime: validate and convert to UTC
142-
return validateDate(trimmed).toISOString();
157+
return validateDate(normalized).toISOString();
143158
}
144159

145160
/**
@@ -203,7 +218,7 @@ function tryParseOperator(value: string): AbsoluteTimeRange | null {
203218
const dateStr = value.slice(op.prefix.length);
204219
if (dateStr.length === 0) {
205220
throw new ValidationError(
206-
`Missing date after '${op.prefix}'. Expected e.g., '${op.prefix}2024-01-01'.`,
221+
`Missing date after '${op.prefix}'. Expected e.g., '${op.prefix}${EXAMPLE_START}'.`,
207222
"period"
208223
);
209224
}
@@ -233,7 +248,7 @@ function tryParseRange(value: string): AbsoluteTimeRange | null {
233248
if (left.length === 0 && right.length === 0) {
234249
throw new ValidationError(
235250
"Empty range '..'. Provide at least one date " +
236-
"(e.g., '2024-01-01..', '..2024-02-01', '2024-01-01..2024-02-01').",
251+
`(e.g., '${EXAMPLE_START}..', '..${EXAMPLE_END}', '${EXAMPLE_START}..${EXAMPLE_END}').`,
237252
"period"
238253
);
239254
}
@@ -290,7 +305,7 @@ export function parsePeriod(value: string): TimeRange {
290305
if (trimmed.length === 0) {
291306
throw new ValidationError(
292307
"Empty period value. Use a relative duration (e.g., '7d', '24h') " +
293-
"or a date range (e.g., '2024-01-01..2024-02-01', '>=2024-01-01').",
308+
`or a date range (e.g., '${EXAMPLE_START}..${EXAMPLE_END}', '>=${EXAMPLE_START}').`,
294309
"period"
295310
);
296311
}
@@ -307,8 +322,8 @@ export function parsePeriod(value: string): TimeRange {
307322
function throwInvalidPeriod(value: string): never {
308323
throw new ValidationError(
309324
`Invalid period '${value}'. Use a relative duration (e.g., '7d', '24h'), ` +
310-
"a date range (e.g., '2024-01-01..2024-02-01'), " +
311-
"or a comparison operator (e.g., '>=2024-01-01', '<2024-02-01').",
325+
`a date range (e.g., '${EXAMPLE_START}..${EXAMPLE_END}'), ` +
326+
`or a comparison operator (e.g., '>=${EXAMPLE_START}', '<${EXAMPLE_END}').`,
312327
"period"
313328
);
314329
}

test/lib/time-range.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,12 @@ describe("parseDate: output format", () => {
232232
expect(result).toBe(expected);
233233
});
234234

235+
test("space separator is accepted as T alternative", () => {
236+
const withSpace = parseDate("2024-06-15 12:00:00Z", "start");
237+
const withT = parseDate("2024-06-15T12:00:00Z", "start");
238+
expect(withSpace).toBe(withT);
239+
});
240+
235241
test("rejects invalid date", () => {
236242
expect(() => parseDate("not-a-date", "start")).toThrow("Invalid");
237243
});

0 commit comments

Comments
 (0)