Re: New ‘--list-generations’ and ‘--delete-generations’ options

[Ludovic Courtès]

Nikita Karetnikov <address@hidden> skribis:

> The attached procedure will be invoked when either option is called with
> an argument.

Nice.

BTW, what did you think of the idea of using recutils format as the
output?  (Either as the sole output format, or otherwise as a secondary
format.)

> Do you see any problems?  Please check everything (especially the
> ‘first-month’ and ‘last-month’ functions).

Better yet: write test cases.  :-)

> ;; XXX: (avail-generations "") returns () (because of (csi)).  This case
> ;; should be handled by a different procedure.  Basically, it means that no
> ;; arguments were passed to '--list-generations' or '--delete-generations'.
> (define* (avail-generations str #:optional (profile %current-profile))

Please, never use abbreviations in public identifiers, and avoid them in
private identifiers too (‘valid-generation?’, ‘maybe-integer’ instead of
‘int’, etc.)

>   "Return a list of generations matching the pattern in STR."

What about splitting it in two functions:

  ‘string->time-range’ → return two SRFI-19 time objects representing a
                          time interval, or #f and #f on failure

  ‘generation-within-time-range?’

Writing tests for the former will be easy.

The code otherwise looks OK, but disentangling parsing from validation
will make it even more pleasant IMO.

Thanks,
Ludo’.