Re: [PATCH] emacs: Add support for deleting generations.

[Ludovic Courtès]

Alex Kost <address@hidden> skribis:

> Ludovic Courtès (2014-10-05 00:23 +0400) wrote:

[...]

>> To sum up, I would imagine two followups to this:
>>
>>   1. Move these procedures to (guix profiles).
>>   2. Convert them to monadic style.
>>
>> WDYT?
>
> I like the idea of using monads there, but as I said I'm too week (I
> mean "month" (I mean "weak")) for writing that.

Heh.  :-)  No problem, that can come later.

> Also those followups would make my commit totally redundant, no?
> Wouldn't it be better to make a commit for adding the monadic functions
> to (guix profiles) directly?

I think it’s fine to make changes incrementally.

> However if you still allow me to push this commit,

Sure!

> I think I can also push the attached one with the changes for Emacs UI
> now (if it looks OK for you).
>
>
> From c335cdf17a97d07cc3d4149fa7dc13882d16cc87 Mon Sep 17 00:00:00 2001
> From: Alex Kost <address@hidden>
> Date: Sun, 5 Oct 2014 12:31:23 +0400
> Subject: [PATCH] emacs: Add support for deleting generations.
>
> * doc/emacs.texi (emacs List buffer): Mention new key bindings.
> * emacs/guix-base.el (guix-delete-generations): New procedure.
> * emacs/guix-info.el (guix-generation-info-insert-number): Use it.
> * emacs/guix-list.el (guix-generation-list-mark-delete,
>   guix-generation-list-execute): New procedures.
> * emacs/guix-main.scm (delete-generations*): New procedure.

Looks good!  Nitpicks:

> address@hidden x
> +Execute actions on the marked generations (i.e.@: delete generations).

I would make it:

  Execute actions on the marked generations---i.e., delete generations.

Or possibly a comma before “i.e.”.  Certainly a comma after it.

> --- a/emacs/guix-main.scm
> +++ b/emacs/guix-main.scm
> @@ -815,3 +815,9 @@ OUTPUTS is a list of package outputs (may be an empty 
> list)."
>                                    "~a packages in profile~%"
>                                    count)
>                             count)))))))))
> +
> +(define (delete-generations* profile generations)
> +  "Delete GENERATIONS from PROFILE.
> +GENERATIONS is a list of generation numbers."
> +  (let ((store (open-connection)))
> +    (delete-generations store profile generations)))

Rather:

  (with-store store
    (delete-generations store profile generations))

That will take care of closing ‘store’ when the dynamic extent of the
body is left.

Thanks,
Ludo’.