Roel Janssen <r...@gnu.org> skribis:
> Ludovic Courtès writes:
>
>> Hi Roel,
>>
>> I’ve just tried the patch and I think it’s awesome! I’ve also always
>> been dissatisfied with what ‘--list-generations’ provides—it’s
>> inconvenient as soon as you have more than a handful packages.
>>
>> Perhaps we could add an optional argument to --list-generations where:
>>
>> --list-generations
>>
>> would be equivalent to:
>>
>> --list-generations=diff
>>
>> and:
>>
>> --list-generations=full
>>
>> would restore the previous behavior. It doesn’t cost much to do that.
>>
>> WDYT?
>
> That would be great. Currently, the --list-generations already takes a
> generation number.
Ah, good point. Well, forget my suggestion then; I’d say, let’s just do
diffs, and that’s it. If someone wants to list the contents of one
generation, they can always do “--list-generations=42”, which does
exactly that.
Anyone against ‘--list-generations’ always displaying diffs? Time to
speak up! :-)
>> In general, I find it clearer to define the singular form and inline the
>> ‘for-each’:
>>
>> (define display-entry
>> (match-lambda …))
>>
>> …
>>
>> (for-each display-entry entries)
>>
>> Otherwise LGTM!
>
> Right. That does look clearer. I refactored the function further to
> the following:
>
> ------- BEGIN -------
> (define (display-profile-content-diff profile number)
> "Display the changed packages in PROFILE compared to generation NUMBER."
>
> (define (equal-entry? first second)
> (string= (manifest-entry-item first)
> (manifest-entry-item second)))
>
> (define display-entry
> (match-lambda
> (($ <manifest-entry> name version output location _)
> (format #f "~a\t~a\t~a\t~a~%" name version output location))))
>
> (define (display-entries entries prefix)
> (for-each (lambda (entry)
> (format #t " ~a ~a" prefix (display-entry entry))) entries))
[...]
> The thing with `display-entries' is because I cannot pass two arguments
> in the function used in the `for-each', and the prefix for `installed'
> or `removed' differs, I cannot remove it that easily :-).
Maybe like this:
(define (display-entry entry prefix)
(match entry …))
…
(for-each (cut display-entry <> "+") added)
(for-each (cut display-entry <> "-") removed)
Let’s wait a bit to see what people think.
Thanks!
Ludo’.
