Re: Display diffs between generations.

[Ludovic Courtès]

Roel Janssen <address@hidden> 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’.