Display diffs between generations.

Display diffs between generations.

[Roel Janssen]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-guix-package-Display-generation-diffs.patch --]
[-- Type: text/x-patch, Size: 3002 bytes --]

From 1ea5eaae4b492c82358c7394c22cd46497388449 Mon Sep 17 00:00:00 2001
From: Roel Janssen <roel@gnu.org>
Date: Sun, 28 Aug 2016 00:54:06 +0200
Subject: [PATCH] guix package: Display generation diffs.

---
 guix/scripts/package.scm |  2 +-
 guix/ui.scm              | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 2a751a4..32cbcdc 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -640,7 +640,7 @@ processed, #f otherwise."
        (define (list-generation number)
          (unless (zero? number)
            (display-generation profile number)
-           (display-profile-content profile number)
+           (display-profile-content-diff profile number)
            (newline)))
 
        (cond ((not (file-exists? profile))      ; XXX: race condition
diff --git a/guix/ui.scm b/guix/ui.scm
index 906b349..cb056a0 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -87,6 +87,7 @@
             matching-generations
             display-generation
             display-profile-content
+            display-profile-content-diff
             roll-back*
             switch-to-generation*
             delete-generation*
@@ -1070,6 +1071,39 @@ DURATION-RELATION with the current time."
           (format #t (_ "~a\t(current)~%") header)
           (format #t "~a~%" header)))))
 
+(define (display-profile-content-diff profile number)
+  "Display the changed packages in PROFILE with generation specified by NUMBER."
+
+  (define (equal-entry? first second)
+    (string= (manifest-entry-item first)
+             (manifest-entry-item second)))
+
+  (define* (display-entries entries #:optional (prefix " "))
+    (for-each
+     (match-lambda
+       (($ <manifest-entry> name version output location _)
+        (format #t "  ~a ~a\t~a\t~a\t~a~%"
+                prefix name version output location)))
+     entries))
+  
+  (define (display-profile-content-diff-between profile older newer)
+    (if (= older 0)
+        (display-profile-content profile newer)
+        (let* ((old (profile-manifest (generation-file-name profile older)))
+               (new (profile-manifest (generation-file-name profile newer)))
+               (installed (lset-difference equal-entry?
+                                           (manifest-entries new)
+                                           (manifest-entries old)))
+               (removed (lset-difference equal-entry?
+                                         (manifest-entries old)
+                                         (manifest-entries new))))
+
+          (display-entries installed "+")
+          (display-entries removed "-"))))
+
+  (let ((previous (previous-generation-number profile number)))
+    (display-profile-content-diff-between profile previous number)))
+
 (define (display-profile-content profile number)
   "Display the packages in PROFILE, generation NUMBER, in a human-readable
 way."
-- 
2.9.3


[-- Attachment #2: Type: text/plain, Size: 579 bytes --]

Dear Guix,

After looking at a terribly long output of `guix package
--list-generations', I thought it may be a good idea to only display the
difference between profile versions.

I came up with the following patch.  What do you think about changing
from displaying the full profile contents for each generation to only
showing the differences between the profiles?

And, I am probably missing something, because some of my generation
entries are empty..  So how could I improve this patch for it to be
complete?

Thanks in advance for your feedback!

Kind regards,
Roel Janssen

Re: Display diffs between generations.

[Ludovic Courtès]

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?

I have only minor stylistic comments:

Roel Janssen <roel@gnu.org> skribis:

> +(define (display-profile-content-diff profile number)

Or just ‘display-profile-diff’?

> +  "Display the changed packages in PROFILE with generation specified by NUMBER."

“… compared to generation NUMBER”?

> +  (define (equal-entry? first second)
> +    (string= (manifest-entry-item first)
> +             (manifest-entry-item second)))
> +
> +  (define* (display-entries entries #:optional (prefix " "))
> +    (for-each
> +     (match-lambda
> +       (($ <manifest-entry> name version output location _)
> +        (format #t "  ~a ~a\t~a\t~a\t~a~%"
> +                prefix name version output location)))
> +     entries))

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!

Thank you for addressing this!

Ludo’.

Re: Display diffs between generations.

[Roel Janssen]

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.  How would we deal with the situation where we want
to view the differences between generation 2 and 3 only?

  --list-generations=3,diff

That doesn't look appealing to me.

Maybe we could do this instead?:

  --list-generations-diff

> I have only minor stylistic comments:
>
> Roel Janssen <roel@gnu.org> skribis:
>
>> +(define (display-profile-content-diff profile number)
>
> Or just ‘display-profile-diff’?
>
>> +  "Display the changed packages in PROFILE with generation specified by NUMBER."
>
> “… compared to generation NUMBER”?
>
>> +  (define (equal-entry? first second)
>> +    (string= (manifest-entry-item first)
>> +             (manifest-entry-item second)))
>> +
>> +  (define* (display-entries entries #:optional (prefix " "))
>> +    (for-each
>> +     (match-lambda
>> +       (($ <manifest-entry> name version output location _)
>> +        (format #t "  ~a ~a\t~a\t~a\t~a~%"
>> +                prefix name version output location)))
>> +     entries))
>
> 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))

  (define (list-entries input)
    (manifest-entries (profile-manifest (generation-file-name profile input))))

  (define (display-diff profile older newer)
    (if (= older 0)
        (display-profile-content profile newer)
        (begin
          ;; List newly installed packages.
          (display-entries (lset-difference equal-entry?
                                            (list-entries newer)
                                            (list-entries older)) "+")
          ;; List newly removed packages.
          (display-entries (lset-difference equal-entry?
                                            (list-entries older)
                                            (list-entries newer)) "-"))))

  (display-diff profile (previous-generation-number profile number) number))
------- END -------

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 :-).

Kind regards,
Roel Janssen

Re: Display diffs between generations.

[Ludovic Courtès]

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

Re: Display diffs between generations.

[Vincent Legoll]

Hello

>>>   --list-generations=full

I think keeping a way to see all generations in one go would be cool

-- 
Vincent Legoll

Re: Display diffs between generations.

[Ludovic Courtès]

Vincent Legoll <vincent.legoll@gmail.com> skribis:

>>>>   --list-generations=full
>
> I think keeping a way to see all generations in one go would be cool

--list-generations would still display all the generations.  Or did you
mean “a way to see *the content* of each generation”?

Ludo’.

Re: Display diffs between generations.

[Vincent Legoll]

On Thu, Sep 1, 2016 at 2:12 PM, Ludovic Courtès <ludo@gnu.org> wrote:
>>>>>   --list-generations=full
>>
>> I think keeping a way to see all generations in one go would be cool
>
> --list-generations would still display all the generations.  Or did you
> mean “a way to see *the content* of each generation”?

I'm not sure to understand what you mean by "the content", what I meant
was to keep what current guix package -l shows us, i.e. do not alter current
UI, only add new things, but do not break existing practices when not mandated.

-- 
Vincent Legoll

Re: Display diffs between generations.

[Roel Janssen]

Vincent Legoll writes:

> On Thu, Sep 1, 2016 at 2:12 PM, Ludovic Courtès <ludo@gnu.org> wrote:
>>>>>>   --list-generations=full
>>>
>>> I think keeping a way to see all generations in one go would be cool
>>
>> --list-generations would still display all the generations.  Or did you
>> mean “a way to see *the content* of each generation”?
>
> I'm not sure to understand what you mean by "the content", what I meant
> was to keep what current guix package -l shows us, i.e. do not alter current
> UI, only add new things, but do not break existing practices when not mandated.

What about adding a `--no-diff-format' or a `--diff-format' argument to
`--list-generations'?

We could then make two choices:  Default to the proposed diff format, or
default to the full format (what we have now).  Depending on what we
choose as default, we can implement the appropriate negating argument.

I would prefer defaulting to the diff format..

Thanks for your feedback!

Kind regards,
Roel Janssen

Re: Display diffs between generations.

[Ludovic Courtès]

Vincent Legoll <vincent.legoll@gmail.com> skribis:

> On Thu, Sep 1, 2016 at 2:12 PM, Ludovic Courtès <ludo@gnu.org> wrote:
>>>>>>   --list-generations=full
>>>
>>> I think keeping a way to see all generations in one go would be cool
>>
>> --list-generations would still display all the generations.  Or did you
>> mean “a way to see *the content* of each generation”?
>
> I'm not sure to understand what you mean by "the content", what I meant
> was to keep what current guix package -l shows us, i.e. do not alter current
> UI, only add new things, but do not break existing practices when not mandated.

I agree with this as a general principle, of course.  However, my
experience is that currently -l is hardly usable in practice, because it
spits out way too many lines.

For example, I have 200+ packages in my user profile, so it prints out
several times 200 lines, which is useless for me.  Conversely, the diff
format that Roel proposes would be more immediately readable.

Does that make sense?

Ludo’.

Re: Display diffs between generations.

[Hartmut Goebel]

[-- Attachment #1.1: Type: text/plain, Size: 646 bytes --]

Am 05.09.2016 um 00:12 schrieb Ludovic Courtès:
> I agree with this as a general principle, of course.  However, my
> experience is that currently -l is hardly usable in practice, because it
> spits out way too many lines.

+1

-- 
Schönen Gruß
Hartmut Goebel
Dipl.-Informatiker (univ), CISSP, CSSLP, ISO 27001 Lead Implementer
Information Security Management, Security Governance, Secure Software
Development

Goebel Consult, Landshut
http://www.goebel-consult.de

Blog:
http://www.goebel-consult.de/blog/digitale-burgerrechte-in-der-ara-snowden
Kolumne: http://www.cissp-gefluester.de/2010-07-passwoerter-lieben-lernen


[-- Attachment #1.2: Type: text/html, Size: 1760 bytes --]

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 2430 bytes --]

Re: Display diffs between generations.

[Vincent Legoll]

Hello,

On Mon, Sep 5, 2016 at 9:52 AM, Hartmut Goebel
<h.goebel@goebel-consult.de> wrote:
> I agree with this as a general principle, of course.  However, my
> experience is that currently -l is hardly usable in practice, because it
> spits out way too many lines.
>
> +1

Ah, I don't have a lot of packages in my profiles because I'm not using it as my
distro, only experimenting, so I didn't see the problem... So maybe the default
needs to change.

-- 
Vincent Legoll

Re: Display diffs between generations.

[Ludovic Courtès]

Hi Roel!

I realized we have not applied this --list-generations patch of yours,
which is a pity.  So…

Roel Janssen <roel@gnu.org> skribis:

> Vincent Legoll writes:
>
>> On Thu, Sep 1, 2016 at 2:12 PM, Ludovic Courtès <ludo@gnu.org> wrote:
>>>>>>>   --list-generations=full
>>>>
>>>> I think keeping a way to see all generations in one go would be cool
>>>
>>> --list-generations would still display all the generations.  Or did you
>>> mean “a way to see *the content* of each generation”?
>>
>> I'm not sure to understand what you mean by "the content", what I meant
>> was to keep what current guix package -l shows us, i.e. do not alter current
>> UI, only add new things, but do not break existing practices when not mandated.
>
> What about adding a `--no-diff-format' or a `--diff-format' argument to
> `--list-generations'?
>
> We could then make two choices:  Default to the proposed diff format, or
> default to the full format (what we have now).  Depending on what we
> choose as default, we can implement the appropriate negating argument.
>
> I would prefer defaulting to the diff format..

In the discussion that ensued, it seems there was a consensus to provide
only the diff format:

  https://lists.gnu.org/archive/html/guix-devel/2016-09/msg00385.html

So it seems all the lights are green.  :-)  Let me know if there’s
anything we should do to help!

Thanks,
Ludo’.

Re: Display diffs between generations.

[Roel Janssen]

Ludovic Courtès writes:

> Hi Roel!
>
> I realized we have not applied this --list-generations patch of yours,
> which is a pity.  So…
>
> Roel Janssen <roel@gnu.org> skribis:
>
>> Vincent Legoll writes:
>>
>>> On Thu, Sep 1, 2016 at 2:12 PM, Ludovic Courtès <ludo@gnu.org> wrote:
>>>>>>>>   --list-generations=full
>>>>>
>>>>> I think keeping a way to see all generations in one go would be cool
>>>>
>>>> --list-generations would still display all the generations.  Or did you
>>>> mean “a way to see *the content* of each generation”?
>>>
>>> I'm not sure to understand what you mean by "the content", what I meant
>>> was to keep what current guix package -l shows us, i.e. do not alter current
>>> UI, only add new things, but do not break existing practices when not mandated.
>>
>> What about adding a `--no-diff-format' or a `--diff-format' argument to
>> `--list-generations'?
>>
>> We could then make two choices:  Default to the proposed diff format, or
>> default to the full format (what we have now).  Depending on what we
>> choose as default, we can implement the appropriate negating argument.
>>
>> I would prefer defaulting to the diff format..
>
> In the discussion that ensued, it seems there was a consensus to provide
> only the diff format:
>
>   https://lists.gnu.org/archive/html/guix-devel/2016-09/msg00385.html
>
> So it seems all the lights are green.  :-)  Let me know if there’s
> anything we should do to help!

Thanks for the reminder.  I seem to have a lot to finish up :-).  I will
work out the arguments and post a new version of the patch for final review.

Kind regards,
Roel Janssen

Re: Display diffs between generations.

[Roel Janssen]

Roel Janssen writes:

> Ludovic Courtès writes:
>
>> Hi Roel!
>>
>> I realized we have not applied this --list-generations patch of yours,
>> which is a pity.  So…
>>
>> Roel Janssen <roel@gnu.org> skribis:
>>
>>> Vincent Legoll writes:
>>>
>>>> On Thu, Sep 1, 2016 at 2:12 PM, Ludovic Courtès <ludo@gnu.org> wrote:
>>>>>>>>>   --list-generations=full
>>>>>>
>>>>>> I think keeping a way to see all generations in one go would be cool
>>>>>
>>>>> --list-generations would still display all the generations.  Or did you
>>>>> mean “a way to see *the content* of each generation”?
>>>>
>>>> I'm not sure to understand what you mean by "the content", what I meant
>>>> was to keep what current guix package -l shows us, i.e. do not alter current
>>>> UI, only add new things, but do not break existing practices when not mandated.
>>>
>>> What about adding a `--no-diff-format' or a `--diff-format' argument to
>>> `--list-generations'?
>>>
>>> We could then make two choices:  Default to the proposed diff format, or
>>> default to the full format (what we have now).  Depending on what we
>>> choose as default, we can implement the appropriate negating argument.
>>>
>>> I would prefer defaulting to the diff format..
>>
>> In the discussion that ensued, it seems there was a consensus to provide
>> only the diff format:
>>
>>   https://lists.gnu.org/archive/html/guix-devel/2016-09/msg00385.html
>>
>> So it seems all the lights are green.  :-)  Let me know if there’s
>> anything we should do to help!
>
> Thanks for the reminder.  I seem to have a lot to finish up :-).  I will
> work out the arguments and post a new version of the patch for final review.

My GNU mail hasn't been working for a couple of days, but now that it
does, I wonder if the initial patch I sent is fine as-is, since we agree
upon only showing the diff format.

Should I implement the --no-diff-format option to change to the
old behavior?

Kind regards,
Roel Janssen

Re: Display diffs between generations.

[Ludovic Courtès]

Roel Janssen <roel@gnu.org> skribis:

> Roel Janssen writes:
>
>> Ludovic Courtès writes:

[...]

>>> In the discussion that ensued, it seems there was a consensus to provide
>>> only the diff format:
>>>
>>>   https://lists.gnu.org/archive/html/guix-devel/2016-09/msg00385.html
>>>
>>> So it seems all the lights are green.  :-)  Let me know if there’s
>>> anything we should do to help!
>>
>> Thanks for the reminder.  I seem to have a lot to finish up :-).  I will
>> work out the arguments and post a new version of the patch for final review.
>
> My GNU mail hasn't been working for a couple of days, but now that it
> does, I wonder if the initial patch I sent is fine as-is, since we agree
> upon only showing the diff format.
>
> Should I implement the --no-diff-format option to change to the
> old behavior?

I don’t think so.  The old behavior will always be available by
specifying a single generation anyway:

  guix package --list-generations=42

It may be worth mentioning this in the manual.

My understanding of the discussion referred above is that people were
fine with that (or indifferent ;-)).

Thanks,
Ludo’.

Re: Display diffs between generations.

[Roel Janssen]

Ludovic Courtès writes:

> Roel Janssen <roel@gnu.org> skribis:
>
>> Roel Janssen writes:
>>
>>> Ludovic Courtès writes:
>
> [...]
>
>>>> In the discussion that ensued, it seems there was a consensus to provide
>>>> only the diff format:
>>>>
>>>>   https://lists.gnu.org/archive/html/guix-devel/2016-09/msg00385.html
>>>>
>>>> So it seems all the lights are green.  :-)  Let me know if there’s
>>>> anything we should do to help!
>>>
>>> Thanks for the reminder.  I seem to have a lot to finish up :-).  I will
>>> work out the arguments and post a new version of the patch for final review.
>>
>> My GNU mail hasn't been working for a couple of days, but now that it
>> does, I wonder if the initial patch I sent is fine as-is, since we agree
>> upon only showing the diff format.
>>
>> Should I implement the --no-diff-format option to change to the
>> old behavior?
>
> I don’t think so.  The old behavior will always be available by
> specifying a single generation anyway:
>
>   guix package --list-generations=42
>
> It may be worth mentioning this in the manual.
>
> My understanding of the discussion referred above is that people were
> fine with that (or indifferent ;-)).

Then the following patch should be it.  I applied your genius
suggestions of using @code{display-entry} instead of
@code{display-entries}.  I learned something new again today. :-)

From 5630a512f16c6677fd5b0b8085e2ef15cb10499b Mon Sep 17 00:00:00 2001
From: Roel Janssen <roel@gnu.org>
Date: Wed, 19 Oct 2016 23:38:11 +0200
Subject: [PATCH] guix package: Display generation diffs.

* guix/ui.scm (display-profile-content-diff): New variable.
* guix/scripts/package.scm (process-query): Use display-profile-content-diff.
---
 guix/scripts/package.scm |  2 +-
 guix/ui.scm              | 27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index b87aee0..5d93b7b 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -670,7 +670,7 @@ processed, #f otherwise."
        (define (list-generation number)
          (unless (zero? number)
            (display-generation profile number)
-           (display-profile-content profile number)
+           (display-profile-content-diff profile number)
            (newline)))
 
        (cond ((not (file-exists? profile))      ; XXX: race condition
diff --git a/guix/ui.scm b/guix/ui.scm
index eb85df3..7c835c2 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -87,6 +87,7 @@
             matching-generations
             display-generation
             display-profile-content
+            display-profile-content-diff
             roll-back*
             switch-to-generation*
             delete-generation*
@@ -1070,6 +1071,32 @@ DURATION-RELATION with the current time."
           (format #t (_ "~a\t(current)~%") header)
           (format #t "~a~%" header)))))
 
+(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 entry prefix)
+    (match entry
+      (($ <manifest-entry> name version output location _)
+       (format #t " ~a ~a\t~a\t~a\t~a~%" prefix name version output location))))
+
+  (define (list-entries input)
+    (manifest-entries (profile-manifest (generation-file-name profile input))))
+
+  (define (display-diff profile old new)
+    (if (= old 0)
+        (display-profile-content profile new)
+        (let ((added (lset-difference
+                      equal-entry? (list-entries new) (list-entries old)))
+              (removed (lset-difference
+                        equal-entry? (list-entries old) (list-entries new))))
+          (for-each (cut display-entry <> "+") added)
+          (for-each (cut display-entry <> "-") removed))))
+
+  (display-diff profile (previous-generation-number profile number) number))
+
 (define (display-profile-content profile number)
   "Display the packages in PROFILE, generation NUMBER, in a human-readable
 way."
-- 
2.10.0

Kind regards,
Roel Janssen

Re: Display diffs between generations.

[Ludovic Courtès]

Roel Janssen <roel@gnu.org> skribis:

> Ludovic Courtès writes:

[...]

>> I don’t think so.  The old behavior will always be available by
>> specifying a single generation anyway:
>>
>>   guix package --list-generations=42
>>
>> It may be worth mentioning this in the manual.
>>
>> My understanding of the discussion referred above is that people were
>> fine with that (or indifferent ;-)).
>
> Then the following patch should be it.  I applied your genius
> suggestions of using @code{display-entry} instead of
> @code{display-entries}.  I learned something new again today. :-)

Heh.  :-)

>From 5630a512f16c6677fd5b0b8085e2ef15cb10499b Mon Sep 17 00:00:00 2001
> From: Roel Janssen <roel@gnu.org>
> Date: Wed, 19 Oct 2016 23:38:11 +0200
> Subject: [PATCH] guix package: Display generation diffs.
>
> * guix/ui.scm (display-profile-content-diff): New variable.
> * guix/scripts/package.scm (process-query): Use display-profile-content-diff.

The code LGTM, but it doesn’t implement the behavior I suggested above
for --list-generations=42, which I think I didn’t explain correctly.

I think what I’m suggesting is that the first matching generation should
be displayed in full using ‘display-profile-content’, and subsequent
generations would be displayed as a diff.

Thus, when viewing a single generation, as with --list-generations=42,
we’d only display the full contents of that generation.

Does that make sense?

Sorry for not clarifying this in the first place!  I guess that wasn’t
entirely clear to me either.  ;-)

Thanks,
Ludo’.

Re: Display diffs between generations.

[Roel Janssen]

Ludovic Courtès writes:

> Roel Janssen <roel@gnu.org> skribis:
>
>> Ludovic Courtès writes:
>
> [...]
>
>>> I don’t think so.  The old behavior will always be available by
>>> specifying a single generation anyway:
>>>
>>>   guix package --list-generations=42
>>>
>>> It may be worth mentioning this in the manual.
>>>
>>> My understanding of the discussion referred above is that people were
>>> fine with that (or indifferent ;-)).
>>
>> Then the following patch should be it.  I applied your genius
>> suggestions of using @code{display-entry} instead of
>> @code{display-entries}.  I learned something new again today. :-)
>
> Heh.  :-)
>
>>From 5630a512f16c6677fd5b0b8085e2ef15cb10499b Mon Sep 17 00:00:00 2001
>> From: Roel Janssen <roel@gnu.org>
>> Date: Wed, 19 Oct 2016 23:38:11 +0200
>> Subject: [PATCH] guix package: Display generation diffs.
>>
>> * guix/ui.scm (display-profile-content-diff): New variable.
>> * guix/scripts/package.scm (process-query): Use display-profile-content-diff.
>
> The code LGTM, but it doesn’t implement the behavior I suggested above
> for --list-generations=42, which I think I didn’t explain correctly.
>
> I think what I’m suggesting is that the first matching generation should
> be displayed in full using ‘display-profile-content’, and subsequent
> generations would be displayed as a diff.
>
> Thus, when viewing a single generation, as with --list-generations=42,
> we’d only display the full contents of that generation.
>
> Does that make sense?

Ah, sorry, I forgot about this.  This makes sense.  But then, what should we
see when we do:

  guix package --list-generations=42,46

Should we see:
  Generation 42 ...:
  <full contents>

  Generation 46 ...:
  <diff with 42>

Or should we then see:
  Generation 42 ...:
  <full contents>

  Generation 46 ...:
  <full contents>

Kind regards,
Roel Janssen

Re: Display diffs between generations.

[Ludovic Courtès]

Roel Janssen <roel@gnu.org> skribis:

> Ludovic Courtès writes:

[...]

>> I think what I’m suggesting is that the first matching generation should
>> be displayed in full using ‘display-profile-content’, and subsequent
>> generations would be displayed as a diff.
>>
>> Thus, when viewing a single generation, as with --list-generations=42,
>> we’d only display the full contents of that generation.
>>
>> Does that make sense?
>
> Ah, sorry, I forgot about this.  This makes sense.  But then, what should we
> see when we do:
>
>   guix package --list-generations=42,46
>
> Should we see:
>   Generation 42 ...:
>   <full contents>
>
>   Generation 46 ...:
>   <diff with 42>

This one, IMO.

Ludo’.

Re: Display diffs between generations.

[Roel Janssen]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-guix-package-Display-generation-diffs.patch --]
[-- Type: text/x-patch, Size: 4123 bytes --]

From 7864ff2443f99fb227e90d0d176a977a30c88faa Mon Sep 17 00:00:00 2001
From: Roel Janssen <roel@gnu.org>
Date: Fri, 21 Oct 2016 11:31:52 +0200
Subject: [PATCH] guix package: Display generation diffs.

* guix/ui.scm (display-profile-content-diff): New variable.
* guix/scripts/package.scm (process-query): Use display-profile-content-diff.
---
 guix/scripts/package.scm | 14 ++++++++++----
 guix/ui.scm              | 28 ++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index b87aee0..8b57f8d 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -667,24 +667,30 @@ processed, #f otherwise."
                      ((head tail ...) head))))
     (match (assoc-ref opts 'query)
       (('list-generations pattern)
-       (define (list-generation number)
+       (define (list-generation display-function number)
          (unless (zero? number)
            (display-generation profile number)
-           (display-profile-content profile number)
+           (display-function profile number)
            (newline)))
 
        (cond ((not (file-exists? profile))      ; XXX: race condition
               (raise (condition (&profile-not-found-error
                                  (profile profile)))))
              ((string-null? pattern)
-              (for-each list-generation (profile-generations profile)))
+              (for-each (cut list-generation
+                             display-profile-content-diff <>)
+                        (profile-generations profile)))
              ((matching-generations pattern profile)
               =>
               (lambda (numbers)
                 (if (null-list? numbers)
                     (exit 1)
                     (leave-on-EPIPE
-                     (for-each list-generation numbers)))))
+                     (list-generation display-profile-content (car numbers))
+                     (unless (null-list? (cdr numbers))
+                       (for-each (cut list-generation
+                                      display-profile-content-diff <>)
+                                 (cdr numbers)))))))
              (else
               (leave (_ "invalid syntax: ~a~%")
                      pattern)))
diff --git a/guix/ui.scm b/guix/ui.scm
index eb85df3..4af8d52 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -87,6 +87,7 @@
             matching-generations
             display-generation
             display-profile-content
+            display-profile-content-diff
             roll-back*
             switch-to-generation*
             delete-generation*
@@ -1070,6 +1071,33 @@ DURATION-RELATION with the current time."
           (format #t (_ "~a\t(current)~%") header)
           (format #t "~a~%" header)))))
 
+(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 entry prefix)
+    (match entry
+      (($ <manifest-entry> name version output location _)
+       (format #t " ~a ~a\t~a\t~a\t~a~%" prefix name version output location))))
+
+  (define (list-entries number)
+    (manifest-entries (profile-manifest (generation-file-name profile number))))
+
+  (define (display-diff profile old new)
+    (if (or (= old 0)
+            #f) ; Or first profile specified as an argument.
+        (display-profile-content profile new)
+        (let ((added (lset-difference
+                      equal-entry? (list-entries new) (list-entries old)))
+              (removed (lset-difference
+                        equal-entry? (list-entries old) (list-entries new))))
+          (for-each (cut display-entry <> "+") added)
+          (for-each (cut display-entry <> "-") removed))))
+
+  (display-diff profile (previous-generation-number profile number) number))
+
 (define (display-profile-content profile number)
   "Display the packages in PROFILE, generation NUMBER, in a human-readable
 way."
-- 
2.10.0


[-- Attachment #2: Type: text/plain, Size: 1116 bytes --]


Ludovic Courtès writes:

> Roel Janssen <roel@gnu.org> skribis:
>
>> Ludovic Courtès writes:
>
> [...]
>
>>> I think what I’m suggesting is that the first matching generation should
>>> be displayed in full using ‘display-profile-content’, and subsequent
>>> generations would be displayed as a diff.
>>>
>>> Thus, when viewing a single generation, as with --list-generations=42,
>>> we’d only display the full contents of that generation.
>>>
>>> Does that make sense?
>>
>> Ah, sorry, I forgot about this.  This makes sense.  But then, what should we
>> see when we do:
>>
>>   guix package --list-generations=42,46
>>
>> Should we see:
>>   Generation 42 ...:
>>   <full contents>
>>
>>   Generation 46 ...:
>>   <diff with 42>
>
> This one, IMO.

The attached patch implements this behavior.  However, because I use
@code{previous-generation-number} to determine which generation to diff
with, the following will not provide a diff as expected:

  guix package --list-generations=42,41,40

Any ideas on how to fix this?  Otherwise I'll give it another go over
the weekend.

Kind regards,
Roel Janssen

Re: Display diffs between generations.

[Benz Schenk]

[-- Attachment #1: Type: text/plain, Size: 1109 bytes --]

On Fri, 21 Oct 2016 11:37:00 +0200
Roel Janssen <roel@gnu.org> wrote:

> Ludovic Courtès writes:
> 
> > Roel Janssen <roel@gnu.org> skribis:
> >  
> >> Ludovic Courtès writes:  
> >
> > [...]
> >  
>  [...]  
> >>
> >> Ah, sorry, I forgot about this.  This makes sense.  But then, what should we
> >> see when we do:
> >>
> >>   guix package --list-generations=42,46
> >>
> >> Should we see:
> >>   Generation 42 ...:
> >>   <full contents>
> >>
> >>   Generation 46 ...:
> >>   <diff with 42>  
> >
> > This one, IMO.  
> 
> The attached patch implements this behavior.  However, because I use
> @code{previous-generation-number} to determine which generation to diff
> with, the following will not provide a diff as expected:
> 
>   guix package --list-generations=42,41,40
> 
> Any ideas on how to fix this?  Otherwise I'll give it another go over
> the weekend.
> 
> Kind regards,
> Roel Janssen

I adapted your patch to hopefully implement the desired behaviour, but
it might need some cleaning up as I'm just getting started learning
scheme.

HANWE

Benz

[-- Attachment #2: 0001-guix-package-Display-generation-diffs.patch --]
[-- Type: text/x-patch, Size: 4306 bytes --]

From f920c91e0572539790952d9c28aec547d7bc0000 Mon Sep 17 00:00:00 2001
From: Benz Schenk <benz.schenk@uzh.ch>
Date: Fri, 21 Oct 2016 16:20:30 +0200
Subject: [PATCH] guix package: Display generation diffs.

* guix/ui.scm (display-profile-content-diff): New variable.
* guix/scripts/package.scm (process-query): Use display-profile-content-diff.

---
 guix/scripts/package.scm | 16 ++++++++++++----
 guix/ui.scm              | 30 ++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index b87aee0..8d148d7 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -667,24 +667,32 @@ processed, #f otherwise."
                      ((head tail ...) head))))
     (match (assoc-ref opts 'query)
       (('list-generations pattern)
-       (define (list-generation number)
+       (define (list-generation display-function number)
          (unless (zero? number)
            (display-generation profile number)
-           (display-profile-content profile number)
+           (display-function profile number)
            (newline)))
+       (define (diff-profiles profile numbers)
+         (unless (null-list? (cdr numbers))
+           (display-profile-content-diff profile (car numbers) (cadr numbers)))
+         (unless (null-list? (cdr numbers))
+           (diff-profiles profile (cdr numbers))))
 
        (cond ((not (file-exists? profile))      ; XXX: race condition
               (raise (condition (&profile-not-found-error
                                  (profile profile)))))
              ((string-null? pattern)
-              (for-each list-generation (profile-generations profile)))
+              (list-generation display-profile-content
+                               (car (profile-generations profile)))
+              (diff-profiles profile (profile-generations profile)))
              ((matching-generations pattern profile)
               =>
               (lambda (numbers)
                 (if (null-list? numbers)
                     (exit 1)
                     (leave-on-EPIPE
-                     (for-each list-generation numbers)))))
+                     (list-generation display-profile-content (car numbers))
+                     (diff-profiles profile numbers)))))
              (else
               (leave (_ "invalid syntax: ~a~%")
                      pattern)))
diff --git a/guix/ui.scm b/guix/ui.scm
index eb85df3..c93b44f 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -87,6 +87,7 @@
             matching-generations
             display-generation
             display-profile-content
+            display-profile-content-diff
             roll-back*
             switch-to-generation*
             delete-generation*
@@ -1070,6 +1071,35 @@ DURATION-RELATION with the current time."
           (format #t (_ "~a\t(current)~%") header)
           (format #t "~a~%" header)))))
 
+(define (display-profile-content-diff profile number1 number2)
+  "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 entry prefix)
+    (match entry
+      (($ <manifest-entry> name version output location _)
+       (format #t " ~a ~a\t~a\t~a\t~a~%" prefix name version output location))))
+
+  (define (list-entries number)
+    (manifest-entries (profile-manifest (generation-file-name profile number))))
+
+  (define (display-diff profile old new)
+    (format #t (_ "Generation ~a\t~a ~%") new
+                          (date->string
+                           (time-utc->date
+                            (generation-time profile new))
+                           "~b ~d ~Y ~T"))
+    (let ((added (lset-difference
+                  equal-entry? (list-entries new) (list-entries old)))
+          (removed (lset-difference
+                    equal-entry? (list-entries old) (list-entries new))))
+      (for-each (cut display-entry <> "+") added)
+      (for-each (cut display-entry <> "-") removed)))
+
+  (display-diff profile number1 number2))
+
 (define (display-profile-content profile number)
   "Display the packages in PROFILE, generation NUMBER, in a human-readable
 way."
-- 
2.10.1

Re: Display diffs between generations.

[Ludovic Courtès]

Hi!

Benz Schenk <benz.schenk@uzh.ch> skribis:

> On Fri, 21 Oct 2016 11:37:00 +0200
> Roel Janssen <roel@gnu.org> wrote:

[...]

>> The attached patch implements this behavior.  However, because I use
>> @code{previous-generation-number} to determine which generation to diff
>> with, the following will not provide a diff as expected:
>> 
>>   guix package --list-generations=42,41,40
>> 
>> Any ideas on how to fix this?  Otherwise I'll give it another go over
>> the weekend.
>> 
>> Kind regards,
>> Roel Janssen
>
> I adapted your patch to hopefully implement the desired behaviour, but
> it might need some cleaning up as I'm just getting started learning
> scheme.

From what I can see that Benz’ patch does indeed work as expected (but
really, the example above is a corner case that we shouldn’t worry too
much about.)

Roel, if that’s fine with you, please commit with proper commit log and
acknowledgment.

Thanks to both of you.  :-)

Ludo’.

Re: Display diffs between generations.

[Roel Janssen]

Ludovic Courtès writes:

> Hi!
>
> Benz Schenk <benz.schenk@uzh.ch> skribis:
>
>> On Fri, 21 Oct 2016 11:37:00 +0200
>> Roel Janssen <roel@gnu.org> wrote:
>
> [...]
>
>>> The attached patch implements this behavior.  However, because I use
>>> @code{previous-generation-number} to determine which generation to diff
>>> with, the following will not provide a diff as expected:
>>> 
>>>   guix package --list-generations=42,41,40
>>> 
>>> Any ideas on how to fix this?  Otherwise I'll give it another go over
>>> the weekend.
>>> 
>>> Kind regards,
>>> Roel Janssen
>>
>> I adapted your patch to hopefully implement the desired behaviour, but
>> it might need some cleaning up as I'm just getting started learning
>> scheme.
>
>From what I can see that Benz’ patch does indeed work as expected (but
> really, the example above is a corner case that we shouldn’t worry too
> much about.)
>
> Roel, if that’s fine with you, please commit with proper commit log and
> acknowledgment.
>
> Thanks to both of you.  :-)

Thanks a lot Benz!

There's only one thing:
Would it make more sense to stick to the chronology of the generations
(sorting them before displaying them)?


If you think Benz's patch is good, then I will push that one.  Otherwise
I'll adapt it to sort the generations.

@Benz, what's the copyright line you want to have in the patch?

Kind regards,
Roel Janssen

Re: Display diffs between generations.

[Benz Schenk]

[-- Attachment #1: Type: text/plain, Size: 2132 bytes --]

On Tue, 25 Oct 2016 18:01:23 +0200
Roel Janssen <roel@gnu.org> wrote:

> Ludovic Courtès writes:
> 
> > Hi!
> >
> > Benz Schenk <benz.schenk@uzh.ch> skribis:
> >  
> >> On Fri, 21 Oct 2016 11:37:00 +0200
> >> Roel Janssen <roel@gnu.org> wrote:  
> >
> > [...]
> >  
>  [...]  
> >>
> >> I adapted your patch to hopefully implement the desired behaviour, but
> >> it might need some cleaning up as I'm just getting started learning
> >> scheme.  
> >  
> >>From what I can see that Benz’ patch does indeed work as expected (but  
> > really, the example above is a corner case that we shouldn’t worry too
> > much about.)
> >
> > Roel, if that’s fine with you, please commit with proper commit log and
> > acknowledgment.
> >
> > Thanks to both of you.  :-)  
> 
> Thanks a lot Benz!
> 
> There's only one thing:
> Would it make more sense to stick to the chronology of the generations
> (sorting them before displaying them)?

IMO it's useful to see the diffs in reverse when before switching
to some previous generation, although you can easily see the changes
no matter how you order the generations, so I don't really have a strong
opinion either way.

>
> 
> If you think Benz's patch is good, then I will push that one.  Otherwise
> I'll adapt it to sort the generations.
> 
> @Benz, what's the copyright line you want to have in the patch?

I guess
Copyright © 2016 Benz Schenk <benz.schenk@uzh.ch>

> 
> Kind regards,
> Roel Janssen

Kind regards,
Benz Schenk

PS:

@Roel Janssen, sorry for double-posting I forgot to cc guix-devel

@everyone
on the bright side, I updated the patch to use display-generation
instead of the copy+pasted mess I created in the last patch and added
my copyright lines.

I also realized that with this patch, list-generations with
generations that do not differ, will simply display the generation number 
and date like

>Generation 54	Oct 19 2016 13:42:16
>[... <packages>]
>Generation 56	Oct 21 2016 15:12:24
>Generation 57	Oct 23 2016 18:15:03
>[... <package-diff>]

I'm not sure if that might be confusing.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-guix-package-Display-generation-diffs.patch --]
[-- Type: text/x-patch, Size: 4824 bytes --]

From 880d95a3779341cb2305a638fbad9364455a02eb Mon Sep 17 00:00:00 2001
From: Benz Schenk <benz.schenk@uzh.ch>
Date: Wed, 26 Oct 2016 12:31:03 +0200
Subject: [PATCH] guix package: Display generation diffs.

* guix/ui.scm (display-profile-content-diff): New variable.
* guix/scripts/package.scm (process-query): Use display-profile-content-diff.

---
 guix/scripts/package.scm | 17 +++++++++++++----
 guix/ui.scm              | 27 +++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index b87aee0..e85f3fb 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
 ;;; Copyright © 2013, 2015 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2014, 2016 Alex Kost <alezost@gmail.com>
+;;; Copyright © 2016 Benz Schenk <benz.schenk@uzh.ch>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -667,24 +668,32 @@ processed, #f otherwise."
                      ((head tail ...) head))))
     (match (assoc-ref opts 'query)
       (('list-generations pattern)
-       (define (list-generation number)
+       (define (list-generation display-function number)
          (unless (zero? number)
            (display-generation profile number)
-           (display-profile-content profile number)
+           (display-function profile number)
            (newline)))
+       (define (diff-profiles profile numbers)
+         (unless (null-list? (cdr numbers))
+           (display-profile-content-diff profile (car numbers) (cadr numbers)))
+         (unless (null-list? (cdr numbers))
+           (diff-profiles profile (cdr numbers))))
 
        (cond ((not (file-exists? profile))      ; XXX: race condition
               (raise (condition (&profile-not-found-error
                                  (profile profile)))))
              ((string-null? pattern)
-              (for-each list-generation (profile-generations profile)))
+              (list-generation display-profile-content
+                               (car (profile-generations profile)))
+              (diff-profiles profile (profile-generations profile)))
              ((matching-generations pattern profile)
               =>
               (lambda (numbers)
                 (if (null-list? numbers)
                     (exit 1)
                     (leave-on-EPIPE
-                     (for-each list-generation numbers)))))
+                     (list-generation display-profile-content (car numbers))
+                     (diff-profiles profile numbers)))))
              (else
               (leave (_ "invalid syntax: ~a~%")
                      pattern)))
diff --git a/guix/ui.scm b/guix/ui.scm
index eb85df3..306c14c 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -7,6 +7,7 @@
 ;;; Copyright © 2014, 2015 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2015 David Thompson <davet@gnu.org>
 ;;; Copyright © 2015, 2016 Mathieu Lirzin <mthl@gnu.org>
+;;; Copyright © 2016 Benz Schenk <benz.schenk@uzh.ch>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -87,6 +88,7 @@
             matching-generations
             display-generation
             display-profile-content
+            display-profile-content-diff
             roll-back*
             switch-to-generation*
             delete-generation*
@@ -1070,6 +1072,31 @@ DURATION-RELATION with the current time."
           (format #t (_ "~a\t(current)~%") header)
           (format #t "~a~%" header)))))
 
+(define (display-profile-content-diff profile number1 number2)
+  "Display the changed packages in PROFILE NUMBER2 compared to generation NUMBER2."
+
+  (define (equal-entry? first second)
+    (string= (manifest-entry-item first) (manifest-entry-item second)))
+
+  (define (display-entry entry prefix)
+    (match entry
+      (($ <manifest-entry> name version output location _)
+       (format #t " ~a ~a\t~a\t~a\t~a~%" prefix name version output location))))
+
+  (define (list-entries number)
+    (manifest-entries (profile-manifest (generation-file-name profile number))))
+
+  (define (display-diff profile old new)
+    (display-generation profile new)
+    (let ((added (lset-difference
+                  equal-entry? (list-entries new) (list-entries old)))
+          (removed (lset-difference
+                    equal-entry? (list-entries old) (list-entries new))))
+      (for-each (cut display-entry <> "+") added)
+      (for-each (cut display-entry <> "-") removed)))
+
+  (display-diff profile number1 number2))
+
 (define (display-profile-content profile number)
   "Display the packages in PROFILE, generation NUMBER, in a human-readable
 way."
-- 
2.10.1

Re: Display diffs between generations.

[Ludovic Courtès]

Benz Schenk <benz.schenk@uzh.ch> skribis:

> On Tue, 25 Oct 2016 18:01:23 +0200
> Roel Janssen <roel@gnu.org> wrote:

[...]

>> There's only one thing:
>> Would it make more sense to stick to the chronology of the generations
>> (sorting them before displaying them)?
>
> IMO it's useful to see the diffs in reverse when before switching
> to some previous generation, although you can easily see the changes
> no matter how you order the generations, so I don't really have a strong
> opinion either way.

I’m in favor of not sorting: if the user specifies a reverse order, it’s
their choice.

So I think all is good, Roel!

Ludo’.

Re: Display diffs between generations.

[Roel Janssen]

Benz Schenk writes:

> On Tue, 25 Oct 2016 18:01:23 +0200
> Roel Janssen <roel@gnu.org> wrote:
>
>> Ludovic Courtès writes:
>> 
>> > Hi!
>> >
>> > Benz Schenk <benz.schenk@uzh.ch> skribis:
>> >  
>> >> On Fri, 21 Oct 2016 11:37:00 +0200
>> >> Roel Janssen <roel@gnu.org> wrote:  
>> >
>> > [...]
>> >  
>>  [...]  
>> >>
>> >> I adapted your patch to hopefully implement the desired behaviour, but
>> >> it might need some cleaning up as I'm just getting started learning
>> >> scheme.  
>> >  
>> >>From what I can see that Benz’ patch does indeed work as expected (but  
>> > really, the example above is a corner case that we shouldn’t worry too
>> > much about.)
>> >
>> > Roel, if that’s fine with you, please commit with proper commit log and
>> > acknowledgment.
>> >
>> > Thanks to both of you.  :-)  
>> 
>> Thanks a lot Benz!
>> 
>> There's only one thing:
>> Would it make more sense to stick to the chronology of the generations
>> (sorting them before displaying them)?
>
> IMO it's useful to see the diffs in reverse when before switching
> to some previous generation, although you can easily see the changes
> no matter how you order the generations, so I don't really have a strong
> opinion either way.
>
>>
>> 
>> If you think Benz's patch is good, then I will push that one.  Otherwise
>> I'll adapt it to sort the generations.
>> 
>> @Benz, what's the copyright line you want to have in the patch?
>
> I guess
> Copyright © 2016 Benz Schenk <benz.schenk@uzh.ch>
>
>> 
>> Kind regards,
>> Roel Janssen
>
> Kind regards,
> Benz Schenk
>
> PS:
>
> @Roel Janssen, sorry for double-posting I forgot to cc guix-devel
>
> @everyone
> on the bright side, I updated the patch to use display-generation
> instead of the copy+pasted mess I created in the last patch and added
> my copyright lines.

Thanks!

I pushed the patch with some more minor clean-ups.

Kind regards,
Roel Janssen