unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Merging guix.el
@ 2014-08-23 12:17 Ludovic Courtès
  2014-08-24  6:59 ` Alex Kost
  2014-08-27 13:11 ` Alex Kost
  0 siblings, 2 replies; 22+ messages in thread
From: Ludovic Courtès @ 2014-08-23 12:17 UTC (permalink / raw)
  To: Alex Kost; +Cc: Guix-devel

Hello!

I think we can merge guix.el in the repo anytime now.  Here are some
notes on how this could be done:

  • Put all the .el and .scm files of the guix.el repo under an emacs/
    directory.

  • Have them appropriately listed in the top-level Makefile.am (I can
    help with that, if you’re not familiar.)

  • Add a section in the manual, probably under “Package Management”,
    describing the interface (probably as a separate .texi file
    @included from the main one.)  It can come later.

I think you would effectively be the maintainer of everything in that
directory, so you could commit whatever needs to be done.  However I
think it’s important to keep sending notes here occasionally about
things that are going on, and possibly discuss user interface issues.

WDYT?

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-08-23 12:17 Merging guix.el Ludovic Courtès
@ 2014-08-24  6:59 ` Alex Kost
  2014-08-25  8:23   ` Ludovic Courtès
  2014-08-27 13:11 ` Alex Kost
  1 sibling, 1 reply; 22+ messages in thread
From: Alex Kost @ 2014-08-24  6:59 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Guix-devel

Ludovic Courtès (2014-08-23 16:17 +0400) wrote:

> Hello!
>
> I think we can merge guix.el in the repo anytime now.  Here are some
> notes on how this could be done:
>
>   • Put all the .el and .scm files of the guix.el repo under an emacs/
>     directory.
>
>   • Have them appropriately listed in the top-level Makefile.am (I can
>     help with that, if you’re not familiar.)

I'm not familiar, but I'll try to figure it out.

>   • Add a section in the manual, probably under “Package Management”,
>     describing the interface (probably as a separate .texi file
>     @included from the main one.)  It can come later.
>
> I think you would effectively be the maintainer of everything in that
> directory, so you could commit whatever needs to be done.  However I
> think it’s important to keep sending notes here occasionally about
> things that are going on, and possibly discuss user interface issues.
>
> WDYT?

Great, thanks!  I totally agree with this.

--
Alex

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-08-24  6:59 ` Alex Kost
@ 2014-08-25  8:23   ` Ludovic Courtès
  2014-08-25 12:31     ` Alex Kost
  0 siblings, 1 reply; 22+ messages in thread
From: Ludovic Courtès @ 2014-08-25  8:23 UTC (permalink / raw)
  To: Alex Kost; +Cc: Guix-devel

Hello!

I’ve added you to the ‘guix’ group on Savannah, so you now have commit
access, congrats.  ;-)

Please make sure to read the “Commit Access” section in Savannah.

Ludo’.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-08-25  8:23   ` Ludovic Courtès
@ 2014-08-25 12:31     ` Alex Kost
  2014-08-25 22:03       ` Ludovic Courtès
  0 siblings, 1 reply; 22+ messages in thread
From: Alex Kost @ 2014-08-25 12:31 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Guix-devel

Ludovic Courtès (2014-08-25 12:23 +0400) wrote:

> Hello!
>
> I’ve added you to the ‘guix’ group on Savannah, so you now have commit
> access, congrats.  ;-)
>
> Please make sure to read the “Commit Access” section in Savannah.

Thanks, I had already pushed a “Fix typo” commit, so I think I dealt
with “commit access” properly.

--
Alex

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-08-25 12:31     ` Alex Kost
@ 2014-08-25 22:03       ` Ludovic Courtès
  2014-08-26  4:14         ` Alex Kost
  0 siblings, 1 reply; 22+ messages in thread
From: Ludovic Courtès @ 2014-08-25 22:03 UTC (permalink / raw)
  To: Alex Kost; +Cc: Guix-devel

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2014-08-25 12:23 +0400) wrote:
>
>> Hello!
>>
>> I’ve added you to the ‘guix’ group on Savannah, so you now have commit
>> access, congrats.  ;-)
>>
>> Please make sure to read the “Commit Access” section in Savannah.
>
> Thanks, I had already pushed a “Fix typo” commit, so I think I dealt
> with “commit access” properly.

Indeed.  :-)

But I actually meant the “Commit Access” section in the ‘HACKING’ file,
sorry for the confusion.

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-08-25 22:03       ` Ludovic Courtès
@ 2014-08-26  4:14         ` Alex Kost
  0 siblings, 0 replies; 22+ messages in thread
From: Alex Kost @ 2014-08-26  4:14 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Guix-devel

Ludovic Courtès (2014-08-26 02:03 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> Ludovic Courtès (2014-08-25 12:23 +0400) wrote:
>>
>>> Hello!
>>>
>>> I’ve added you to the ‘guix’ group on Savannah, so you now have commit
>>> access, congrats.  ;-)
>>>
>>> Please make sure to read the “Commit Access” section in Savannah.
>>
>> Thanks, I had already pushed a “Fix typo” commit, so I think I dealt
>> with “commit access” properly.
>
> Indeed.  :-)
>
> But I actually meant the “Commit Access” section in the ‘HACKING’ file,
> sorry for the confusion.

Ah, that's why I couldn't find this section in Savannah :-)

I read "HACKING" file some time ago and I'm reading it now again to be
sure I missed nothing, thanks.

--
Alex

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-08-23 12:17 Merging guix.el Ludovic Courtès
  2014-08-24  6:59 ` Alex Kost
@ 2014-08-27 13:11 ` Alex Kost
  2014-08-28 12:41   ` Ludovic Courtès
  1 sibling, 1 reply; 22+ messages in thread
From: Alex Kost @ 2014-08-27 13:11 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Guix-devel

Hello,

Ludovic Courtès (2014-08-23 16:17 +0400) wrote:

> Hello!
>
> I think we can merge guix.el in the repo anytime now.  Here are some
> notes on how this could be done:
>
>   • Put all the .el and .scm files of the guix.el repo under an emacs/
>     directory.

Done.  I have made “emacs-ui” branch with what I did so far.  But as I
have no experience with autotools, I'm afraid all that stuff need to be
reviewed carefully.

>   • Have them appropriately listed in the top-level Makefile.am (I can
>     help with that, if you’re not familiar.)

Along with the small changes to top-level "Makefile.am", I made
"Makefile.am" in "emacs" dir and...

I imagine there may be... for example vi users, who wouldn't want to
install this feature, so I made some changes in "configure.ac" to add
“--disable-emacs-ui” option.

Also I use almost the same code in "guix-helper.scm.in" that is used in
"scripts/guix.in", so I think it will be good to have some little
additional module with ‘config-lookup’ function.  WDYT?

>   • Add a section in the manual, probably under “Package Management”,
>     describing the interface (probably as a separate .texi file
>     @included from the main one.)  It can come later.

This is new subject for me as well; I'll write it as soon as possible.

For now the most important part of the documentation is that after
installing guix with emacs UI, it may be used by putting:

  (require 'guix-init)

into ".emacs".  After that autoloaded "guix-..." commands should be
available.

-- 
Alex Kost

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-08-27 13:11 ` Alex Kost
@ 2014-08-28 12:41   ` Ludovic Courtès
  2014-08-28 18:22     ` Alex Kost
  0 siblings, 1 reply; 22+ messages in thread
From: Ludovic Courtès @ 2014-08-28 12:41 UTC (permalink / raw)
  To: Alex Kost; +Cc: Guix-devel

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

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2014-08-23 16:17 +0400) wrote:

[...]

>>   • Have them appropriately listed in the top-level Makefile.am (I can
>>     help with that, if you’re not familiar.)
>
> Along with the small changes to top-level "Makefile.am", I made
> "Makefile.am" in "emacs" dir and...

I think it’s better to avoid recursive makefiles, which is why I
suggested adding changes to the top-level Makefile.am.

Could you make it this way?

More precisely, the Emacs-specific things could be kept in emacs.am, and
that file would be included from the top-level Makefile.am.

> I imagine there may be... for example vi users, who wouldn't want to
> install this feature, so I made some changes in "configure.ac" to add
> “--disable-emacs-ui” option.

It seems that the only things that cannot be done when Emacs is not
available is the generation of the autoloads file, right?

Then, what about adding $(AUTOLOADS) to the distribution?  It would just
need to be appended to dist_lisp_DATA, and not added to CLEANFILES.

Nitpick: could you use makefile-backslash-region for the $(AUTOLOADS)
recipe?

> Also I use almost the same code in "guix-helper.scm.in" that is used in
> "scripts/guix.in", so I think it will be good to have some little
> additional module with ‘config-lookup’ function.  WDYT?

It cannot be in a module, because at this point the module location
isn’t known yet.  I don’t really know how to factorize it, so I propose
to leave it for later, with a FIXME.  Maybe Mark has an idea?

+(define %current-manifest)
+(define current-manifest-entries-table)
+(define packages)
+(define packages-table)

I didn’t know this was possible, but we shouldn’t rely on it.

+(define name+version->key cons)
+(define (key->name+version key)
+  (values (car key) (cdr key)))

I would find it easier to read if it ‘cons’ and ‘car+cdr’ (from SRFI-1)
were used directly.

+(define* (set-current-manifest-maybe! #:optional manifest)
+  (define (manifest-entries->hash-table entries)
+    (let ((entries-table (make-hash-table (length entries))))
+      (map (lambda (entry)
+             (let* ((key (name+version->key
+                          (manifest-entry-name entry)
+                          (manifest-entry-version entry)))
+                    (ref (hash-ref entries-table key)))
+               (hash-set! entries-table key
+                          (if ref (cons entry ref) (list entry)))))
+           entries)
+      entries-table))
+
+  (let ((manifest (or manifest (profile-manifest %user-profile))))
+    (unless (and (manifest? %current-manifest)
+                 (equal? manifest %current-manifest))
+      (set! %current-manifest manifest)
+      (set! current-manifest-entries-table
+            (manifest-entries->hash-table
+             (manifest-entries manifest))))))

Wouldn’t it be enough to pass the current manifest as an argument to the
various functions, instead of defining a global variable?

Also, my understanding is that ‘current-manifest-entries-table’ is here
to speed up lookups in ‘manifest-entries-by-name+version’, right?

Then, I think this optimization should go into (guix profiles):
<manifest> objects would carry that vhash, and ‘manifest-installed?’
etc. would make use of it.  The constructor would be changed along these
lines:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 767 bytes --]

diff --git a/guix/profiles.scm b/guix/profiles.scm
index a2c73fd..98eb814 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -84,9 +84,17 @@
 ;;;
 
 (define-record-type <manifest>
-  (manifest entries)
+  (%manifest entries name->entry)
   manifest?
-  (entries manifest-entries))                     ; list of <manifest-entry>
+  (entries     manifest-entries)         ; list of <manifest-entry>
+  (name->entry manifest-name->entry))    ; vhash [string -> <manifest-entry>]
+
+(define (manifest entries)
+  (%manifest entries
+             (fold (lambda (entry result)
+                     (vhash-cons ... result))
+                   vlist-null
+                   entries)))
 
 ;; Convenient alias, to avoid name clashes.
 (define make-manifest manifest)

[-- Attachment #3: Type: text/plain, Size: 1974 bytes --]


WDYT?

+(define (set-packages!)
+  (let ((count 0))
+    (set! packages
+          (fold-packages (lambda (pkg res)
+                           (set! count (+ 1 count))
+                           (vhash-consq (object-address pkg) pkg res))
+                         vlist-null))
+    (set! packages-table (make-hash-table count))
+    (vlist-for-each (lambda (elem)
+                      (let* ((pkg (cdr elem))
+                             (key (name+version->key
+                                   (package-name pkg)
+                                   (package-version pkg)))
+                             (ref (hash-ref packages-table key)))
+                        (hash-set! packages-table key
+                                   (if ref (cons pkg ref) (list pkg)))))
+                    packages)))
+
+(set-packages!)

Given that ‘set-packages!’ has only on call site, what about removing
it, and instead writing directly:

  (define %packages
    (fold-packages ... vlist-null))

  (define %package-count
    (length %packages))

  (define %package-table
    (vlist-fold ...))

It’s also best to prefix global variable names with ‘%’.

I should point out that we try to stick to functional style (in
host-side code at least.)  Thus any identifier with an exclamation mark
gives you a -1 during review.  ;-)  See “Coding Style” in HACKING.

>>   • Add a section in the manual, probably under “Package Management”,
>>     describing the interface (probably as a separate .texi file
>>     @included from the main one.)  It can come later.
>
> This is new subject for me as well; I'll write it as soon as possible.
>
> For now the most important part of the documentation is that after
> installing guix with emacs UI, it may be used by putting:
>
>   (require 'guix-init)
>
> into ".emacs".  After that autoloaded "guix-..." commands should be
> available.

Cool!

Thanks,
Ludo’.

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-08-28 12:41   ` Ludovic Courtès
@ 2014-08-28 18:22     ` Alex Kost
  2014-08-28 20:09       ` Ludovic Courtès
  0 siblings, 1 reply; 22+ messages in thread
From: Alex Kost @ 2014-08-28 18:22 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Guix-devel

Ludovic Courtès (2014-08-28 16:41 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> Ludovic Courtès (2014-08-23 16:17 +0400) wrote:
>
> [...]
>
>>>   • Have them appropriately listed in the top-level Makefile.am (I can
>>>     help with that, if you’re not familiar.)
>>
>> Along with the small changes to top-level "Makefile.am", I made
>> "Makefile.am" in "emacs" dir and...
>
> I think it’s better to avoid recursive makefiles, which is why I
> suggested adding changes to the top-level Makefile.am.
>
> Could you make it this way?

No problem.

> More precisely, the Emacs-specific things could be kept in emacs.am, and
> that file would be included from the top-level Makefile.am.

OK, I think it would be good to make "emacs-ui" branch temporary, so
that after I'll fix everything that needs to be fixed, it may be
recreated with a nice and clean commit(s) for merging “guix.el”.  This
way I could push commits there without a fear that I mess it all up.
WDYT?

>> I imagine there may be... for example vi users, who wouldn't want to
>> install this feature, so I made some changes in "configure.ac" to add
>> “--disable-emacs-ui” option.
>
> It seems that the only things that cannot be done when Emacs is not
> available is the generation of the autoloads file, right?

Yes.

> Then, what about adding $(AUTOLOADS) to the distribution?  It would just
> need to be appended to dist_lisp_DATA, and not added to CLEANFILES.

OK, will be done.  And what about "configure.ac"?  I thought a new
configure option is good.  Should I delete it?

> Nitpick: could you use makefile-backslash-region for the $(AUTOLOADS)
> recipe?

Yes, sorry.

>> Also I use almost the same code in "guix-helper.scm.in" that is used in
>> "scripts/guix.in", so I think it will be good to have some little
>> additional module with ‘config-lookup’ function.  WDYT?
>
> It cannot be in a module, because at this point the module location
> isn’t known yet.  I don’t really know how to factorize it, so I propose
> to leave it for later, with a FIXME.  Maybe Mark has an idea?
>
> +(define %current-manifest)
> +(define current-manifest-entries-table)
> +(define packages)
> +(define packages-table)
>
> I didn’t know this was possible, but we shouldn’t rely on it.

Do you mean definitions without initial values?

> +(define name+version->key cons)
> +(define (key->name+version key)
> +  (values (car key) (cdr key)))
>
> I would find it easier to read if it ‘cons’ and ‘car+cdr’ (from SRFI-1)
> were used directly.

Thanks, I didn't know about that.

> +(define* (set-current-manifest-maybe! #:optional manifest)
> +  (define (manifest-entries->hash-table entries)
> +    (let ((entries-table (make-hash-table (length entries))))
> +      (map (lambda (entry)
> +             (let* ((key (name+version->key
> +                          (manifest-entry-name entry)
> +                          (manifest-entry-version entry)))
> +                    (ref (hash-ref entries-table key)))
> +               (hash-set! entries-table key
> +                          (if ref (cons entry ref) (list entry)))))
> +           entries)
> +      entries-table))
> +
> +  (let ((manifest (or manifest (profile-manifest %user-profile))))
> +    (unless (and (manifest? %current-manifest)
> +                 (equal? manifest %current-manifest))
> +      (set! %current-manifest manifest)
> +      (set! current-manifest-entries-table
> +            (manifest-entries->hash-table
> +             (manifest-entries manifest))))))
>
> Wouldn’t it be enough to pass the current manifest as an argument to the
> various functions, instead of defining a global variable?

Most likely, I'll fix it.

> Also, my understanding is that ‘current-manifest-entries-table’ is here
> to speed up lookups in ‘manifest-entries-by-name+version’, right?

Yes, ‘current-manifest-entries-table’ and ‘packages-table’ are there to
speed up the process of finding “manifest entries”/“packages” by
name+version (it is a very general need).  Also
‘current-manifest-entries-table’ is used in ‘fold-manifest-entries’.

> Then, I think this optimization should go into (guix profiles):
> <manifest> objects would carry that vhash, and ‘manifest-installed?’
> etc. would make use of it.  The constructor would be changed along these
> lines:
>
> diff --git a/guix/profiles.scm b/guix/profiles.scm
> index a2c73fd..98eb814 100644
> --- a/guix/profiles.scm
> +++ b/guix/profiles.scm
> @@ -84,9 +84,17 @@
>  ;;;
>  
>  (define-record-type <manifest>
> -  (manifest entries)
> +  (%manifest entries name->entry)
>    manifest?
> -  (entries manifest-entries))                     ; list of <manifest-entry>
> +  (entries     manifest-entries)         ; list of <manifest-entry>
> +  (name->entry manifest-name->entry))    ; vhash [string -> <manifest-entry>]
> +
> +(define (manifest entries)
> +  (%manifest entries
> +             (fold (lambda (entry result)
> +                     (vhash-cons ... result))
> +                   vlist-null
> +                   entries)))
>  
>  ;; Convenient alias, to avoid name clashes.
>  (define make-manifest manifest)
>
> WDYT?

I think it's a good idea, but if that "name" is just a package name,
I can't use this optimization: I need to define entries by
"name+version".  Also I think it should be ‘name->entries’ as there can
be several manifest entries with the same name (or name/version).

> +(define (set-packages!)
> +  (let ((count 0))
> +    (set! packages
> +          (fold-packages (lambda (pkg res)
> +                           (set! count (+ 1 count))
> +                           (vhash-consq (object-address pkg) pkg res))
> +                         vlist-null))
> +    (set! packages-table (make-hash-table count))
> +    (vlist-for-each (lambda (elem)
> +                      (let* ((pkg (cdr elem))
> +                             (key (name+version->key
> +                                   (package-name pkg)
> +                                   (package-version pkg)))
> +                             (ref (hash-ref packages-table key)))
> +                        (hash-set! packages-table key
> +                                   (if ref (cons pkg ref) (list pkg)))))
> +                    packages)))
> +
> +(set-packages!)
>
> Given that ‘set-packages!’ has only on call site, what about removing
> it, and instead writing directly:
>
>   (define %packages
>     (fold-packages ... vlist-null))
>
>   (define %package-count
>     (length %packages))
>
>   (define %package-table
>     (vlist-fold ...))
>
> It’s also best to prefix global variable names with ‘%’.

Yes, it's definitely better, except I don't know how to fill a table
with ‘vlist-fold’ and I don't see a better variant than this:

(define %package-table
  (let ((table (make-hash-table %package-count)))
    (vlist-for-each (lambda (elem)
                      (let* ((pkg (cdr elem))
                             (key (name+version->key
                                   (package-name pkg)
                                   (package-version pkg)))
                             (ref (hash-ref table key)))
                        (hash-set! table key
                                   (if ref (cons pkg ref) (list pkg)))))
                    packages)
    table))

> I should point out that we try to stick to functional style (in
> host-side code at least.)  Thus any identifier with an exclamation mark
> gives you a -1 during review.  ;-)  See “Coding Style” in HACKING.

My bad, I'll fix it, thanks.


P.S.  OMG!  Thank you very much for reviewing all that crap.  Elisp
code is much better (I hope :-)).

--
Alex

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-08-28 18:22     ` Alex Kost
@ 2014-08-28 20:09       ` Ludovic Courtès
  2014-08-29 18:24         ` Alex Kost
  0 siblings, 1 reply; 22+ messages in thread
From: Ludovic Courtès @ 2014-08-28 20:09 UTC (permalink / raw)
  To: Alex Kost; +Cc: Guix-devel

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2014-08-28 16:41 +0400) wrote:
>
>> Alex Kost <alezost@gmail.com> skribis:
>>
>>> Ludovic Courtès (2014-08-23 16:17 +0400) wrote:

[...]

> OK, I think it would be good to make "emacs-ui" branch temporary, so
> that after I'll fix everything that needs to be fixed, it may be
> recreated with a nice and clean commit(s) for merging “guix.el”.  This
> way I could push commits there without a fear that I mess it all up.
> WDYT?

Definitely.  What’s been done before for such work-in-progress branches
is to rewrite them until they’re ready, and then merge them.  So if you
agree to adjust that commit and then delete+push the branch, it’s
perfect.  (By convention, we normally call these branches wip-* so that
people understand that they may be rebased.)

Now, I think we’re almost there anyway, so hopefully you’ll merge real
soon.  :-)

>>> I imagine there may be... for example vi users, who wouldn't want to
>>> install this feature, so I made some changes in "configure.ac" to add
>>> “--disable-emacs-ui” option.
>>
>> It seems that the only things that cannot be done when Emacs is not
>> available is the generation of the autoloads file, right?
>
> Yes.
>
>> Then, what about adding $(AUTOLOADS) to the distribution?  It would just
>> need to be appended to dist_lisp_DATA, and not added to CLEANFILES.
>
> OK, will be done.  And what about "configure.ac"?  I thought a new
> configure option is good.  Should I delete it?

I think so, yes, because it wouldn’t make any difference if the
autoloads are already generated.  Non-Emacs users would end up
installing a few files that they would not use, but that’s not big deal
IMO.

>>> Also I use almost the same code in "guix-helper.scm.in" that is used in
>>> "scripts/guix.in", so I think it will be good to have some little
>>> additional module with ‘config-lookup’ function.  WDYT?
>>
>> It cannot be in a module, because at this point the module location
>> isn’t known yet.  I don’t really know how to factorize it, so I propose
>> to leave it for later, with a FIXME.  Maybe Mark has an idea?
>>
>> +(define %current-manifest)
>> +(define current-manifest-entries-table)
>> +(define packages)
>> +(define packages-table)
>>
>> I didn’t know this was possible, but we shouldn’t rely on it.
>
> Do you mean definitions without initial values?

Yes.

[...]

>> Also, my understanding is that ‘current-manifest-entries-table’ is here
>> to speed up lookups in ‘manifest-entries-by-name+version’, right?
>
> Yes, ‘current-manifest-entries-table’ and ‘packages-table’ are there to
> speed up the process of finding “manifest entries”/“packages” by
> name+version (it is a very general need).  Also
> ‘current-manifest-entries-table’ is used in ‘fold-manifest-entries’.

OK.

>> Then, I think this optimization should go into (guix profiles):
>> <manifest> objects would carry that vhash, and ‘manifest-installed?’
>> etc. would make use of it.  The constructor would be changed along these
>> lines:

[...]

> I think it's a good idea, but if that "name" is just a package name,
> I can't use this optimization: I need to define entries by
> "name+version".

Yes, makes sense.  So that ‘name->entries’ field would map a package
name to a list of entries; in the most common case, there’ll be just one
entry anyway.  How does that sound?

> Also I think it should be ‘name->entries’ as there can be several
> manifest entries with the same name (or name/version).

Yes, sure.

>> Given that ‘set-packages!’ has only on call site, what about removing
>> it, and instead writing directly:
>>
>>   (define %packages
>>     (fold-packages ... vlist-null))
>>
>>   (define %package-count
>>     (length %packages))
>>
>>   (define %package-table
>>     (vlist-fold ...))
>>
>> It’s also best to prefix global variable names with ‘%’.
>
> Yes, it's definitely better, except I don't know how to fill a table
> with ‘vlist-fold’ and I don't see a better variant than this:
>
> (define %package-table
>   (let ((table (make-hash-table %package-count)))
>     (vlist-for-each (lambda (elem)
>                       (let* ((pkg (cdr elem))
>                              (key (name+version->key
>                                    (package-name pkg)
>                                    (package-version pkg)))
>                              (ref (hash-ref table key)))
>                         (hash-set! table key
>                                    (if ref (cons pkg ref) (list pkg)))))
>                     packages)
>     table))

I would make %package-table a vhash instead of a hash table:

  (define %package-table
    (vlist-fold (lambda (elem result)
                  (match elem
                    ((name . package)
                     (vhash-cons (cons (package-name package)
                                       (package-version package))
                                 package
                                 (if (vhash-assq name result)
                                     ...)))))
                vlist-null
                %packages))

> P.S.  OMG!  Thank you very much for reviewing all that crap.  Elisp
> code is much better (I hope :-)).

Np, I trust you on the elisp code.  :-)

Ludo’.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-08-28 20:09       ` Ludovic Courtès
@ 2014-08-29 18:24         ` Alex Kost
  2014-08-31 14:59           ` Ludovic Courtès
  0 siblings, 1 reply; 22+ messages in thread
From: Alex Kost @ 2014-08-29 18:24 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Guix-devel

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

Hello,

Ludovic Courtès (2014-08-29 00:09 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> Ludovic Courtès (2014-08-28 16:41 +0400) wrote:
>>
>>> Alex Kost <alezost@gmail.com> skribis:

[...]

>>>> I imagine there may be... for example vi users, who wouldn't want to
>>>> install this feature, so I made some changes in "configure.ac" to add
>>>> “--disable-emacs-ui” option.
>>>
>>> It seems that the only things that cannot be done when Emacs is not
>>> available is the generation of the autoloads file, right?
>>
>> Yes.
>>
>>> Then, what about adding $(AUTOLOADS) to the distribution?  It would just
>>> need to be appended to dist_lisp_DATA, and not added to CLEANFILES.
>>
>> OK, will be done.  And what about "configure.ac"?  I thought a new
>> configure option is good.  Should I delete it?
>
> I think so, yes, because it wouldn’t make any difference if the
> autoloads are already generated.  Non-Emacs users would end up
> installing a few files that they would not use, but that’s not big deal
> IMO.

OK, I made "emacs.am" and modified "configure.ac" and "Makefile.am" (I
pushed 2 new commits to “emacs-ui” branch).  Is there anything else to
be done in a build part?

>
> [...]
>
>>> Also, my understanding is that ‘current-manifest-entries-table’ is here
>>> to speed up lookups in ‘manifest-entries-by-name+version’, right?
>>
>> Yes, ‘current-manifest-entries-table’ and ‘packages-table’ are there to
>> speed up the process of finding “manifest entries”/“packages” by
>> name+version (it is a very general need).  Also
>> ‘current-manifest-entries-table’ is used in ‘fold-manifest-entries’.
>
> OK.
>
>>> Then, I think this optimization should go into (guix profiles):
>>> <manifest> objects would carry that vhash, and ‘manifest-installed?’
>>> etc. would make use of it.  The constructor would be changed along these
>>> lines:
>
> [...]
>
>> I think it's a good idea, but if that "name" is just a package name,
>> I can't use this optimization: I need to define entries by
>> "name+version".
>
> Yes, makes sense.  So that ‘name->entries’ field would map a package
> name to a list of entries; in the most common case, there’ll be just one
> entry anyway.  How does that sound?

Yes, I think I could use it; I have a problem however.  I don't know how
to use vhash for that (see a comment for %package-table below); if
hash-table is OK then I can make it.

Also I'm still not sure about the name ‘manifest-name->entries’ (or
‘manifest-name->entry’), as this function returns a vhash, it does not
transform a name into a list of entries.  It may be confusing, no?

>
> [...]
>
>>> Given that ‘set-packages!’ has only on call site, what about removing
>>> it, and instead writing directly:
>>>
>>>   (define %packages
>>>     (fold-packages ... vlist-null))
>>>
>>>   (define %package-count
>>>     (length %packages))
>>>
>>>   (define %package-table
>>>     (vlist-fold ...))
>>>
>>> It’s also best to prefix global variable names with ‘%’.

I recalled why I used that ugly ‘set-packages!’: I just didn't want to
count packages again (i.e. to use ‘length’) for hash-table, so I
combined setting ‘packages’ variable with counting.  For now I got rid
of ‘set-packages!’, as you suggested with one exception...

>> Yes, it's definitely better, except I don't know how to fill a table
>> with ‘vlist-fold’ and I don't see a better variant than this:
>>
>> (define %package-table
>>   (let ((table (make-hash-table %package-count)))
>>     (vlist-for-each (lambda (elem)
>>                       (let* ((pkg (cdr elem))
>>                              (key (name+version->key
>>                                    (package-name pkg)
>>                                    (package-version pkg)))
>>                              (ref (hash-ref table key)))
>>                         (hash-set! table key
>>                                    (if ref (cons pkg ref) (list pkg)))))
>>                     packages)
>>     table))
>
> I would make %package-table a vhash instead of a hash table:
>
>   (define %package-table
>     (vlist-fold (lambda (elem result)
>                   (match elem
>                     ((name . package)
>                      (vhash-cons (cons (package-name package)
>                                        (package-version package))
>                                  package
>                                  (if (vhash-assq name result)
>                                      ...)))))
>                 vlist-null
>                 %packages))

... I left hash table, because I still don't understand how to use vhash
for that: a name+version key should give a list of all matching
packages, but AFAIU your variant would just replace one package with
another (with the same name+version).

The same thing with modifications in <manifest> that you proposed:


[-- Attachment #2: sample.scm --]
[-- Type: text/plain, Size: 432 bytes --]

(define-record-type <manifest>
  (%manifest entries name->entries)
  manifest?
  (entries       manifest-entries)        ; list of <manifest-entry>
  (name->entries manifest-name->entries)) ; vhash [string -> list of <manifest-entry>]

(define (manifest entries)
  (%manifest entries
             (fold (lambda (entry result)
                     (vhash-cons ... result))
                   vlist-null
                   entries)))

[-- Attachment #3: Type: text/plain, Size: 434 bytes --]


I can't see how to build an appropriate vhash, sorry.  Need help :)

As for ‘set-current-manifest-maybe!’, I'm afraid it can't be deleted.  I
found that I put it in some places where it shouldn't appear and I fixed
that, but it still stays in functions returning “package information”
for the elisp side (for info/list buffers).  Currently I can't invent a
way how to get rid of this function completely.

--
Alex

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-08-29 18:24         ` Alex Kost
@ 2014-08-31 14:59           ` Ludovic Courtès
  2014-09-01  8:26             ` Alex Kost
  2014-09-02 20:22             ` Alex Kost
  0 siblings, 2 replies; 22+ messages in thread
From: Ludovic Courtès @ 2014-08-31 14:59 UTC (permalink / raw)
  To: Alex Kost; +Cc: Guix-devel

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2014-08-29 00:09 +0400) wrote:

[...]

> OK, I made "emacs.am" and modified "configure.ac" and "Makefile.am" (I
> pushed 2 new commits to “emacs-ui” branch).  Is there anything else to
> be done in a build part?

No, that looks good to me.

[...]

>>> I think it's a good idea, but if that "name" is just a package name,
>>> I can't use this optimization: I need to define entries by
>>> "name+version".
>>
>> Yes, makes sense.  So that ‘name->entries’ field would map a package
>> name to a list of entries; in the most common case, there’ll be just one
>> entry anyway.  How does that sound?

[...]

> Also I'm still not sure about the name ‘manifest-name->entries’ (or
> ‘manifest-name->entry’), as this function returns a vhash, it does not
> transform a name into a list of entries.  It may be confusing, no?

That would be only for internal use anyway, but if you prefer
‘manifest-table’ would do as well.

[...]

>>>> Given that ‘set-packages!’ has only on call site, what about removing
>>>> it, and instead writing directly:
>>>>
>>>>   (define %packages
>>>>     (fold-packages ... vlist-null))
>>>>
>>>>   (define %package-count
>>>>     (length %packages))
>>>>
>>>>   (define %package-table
>>>>     (vlist-fold ...))
>>>>
>>>> It’s also best to prefix global variable names with ‘%’.
>
> I recalled why I used that ugly ‘set-packages!’: I just didn't want to
> count packages again (i.e. to use ‘length’) for hash-table, so I
> combined setting ‘packages’ variable with counting.

OK, but even when there’s 1M packages, ‘length’ is still going to be
almost instantaneous, so no worries IMO.  ;-)

>> I would make %package-table a vhash instead of a hash table:
>>
>>   (define %package-table
>>     (vlist-fold (lambda (elem result)
>>                   (match elem
>>                     ((name . package)
>>                      (vhash-cons (cons (package-name package)
>>                                        (package-version package))
>>                                  package
>>                                  (if (vhash-assq name result)
>>                                      ...)))))
>>                 vlist-null
>>                 %packages))
>
> ... I left hash table, because I still don't understand how to use vhash
> for that: a name+version key should give a list of all matching
> packages, but AFAIU your variant would just replace one package with
> another (with the same name+version).

The key is ‘vhash-fold*’ (info "(guile) VHashes").  It allows you to
traverse all the entries associated with a given key:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> (vhash-cons '("guile" "2.0") 'foo
				 (vhash-cons '("guile" "2.0") 'bar
					     vlist-null))
$12 = #<vhash 39240a0 2 pairs>
scheme@(guile-user)> (vhash-fold* cons '() '("guile" "2.0") $12)
$13 = (bar foo)
--8<---------------cut here---------------end--------------->8---

I think that answers your question, right?

> As for ‘set-current-manifest-maybe!’, I'm afraid it can't be deleted.  I
> found that I put it in some places where it shouldn't appear and I fixed
> that, but it still stays in functions returning “package information”
> for the elisp side (for info/list buffers).  Currently I can't invent a
> way how to get rid of this function completely.

I think the profile’s file name could be kept on the elisp side, and
passed to the Scheme code, which wouldn’t need to keep it in a global
variable.  That would also allow guix.el to be used on profiles other
than the default one.

That said, we can look into it later if you prefer.  WDYT?

Thanks!

Ludo’.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-08-31 14:59           ` Ludovic Courtès
@ 2014-09-01  8:26             ` Alex Kost
  2014-09-01 12:10               ` Ludovic Courtès
  2014-09-01 22:28               ` Ludovic Courtès
  2014-09-02 20:22             ` Alex Kost
  1 sibling, 2 replies; 22+ messages in thread
From: Alex Kost @ 2014-09-01  8:26 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Guix-devel

Hello,

I've just pushed a manual for “guix.el” (into “emacs-ui” branch).  I
tried my best, but I'm not a native English speaker and I have never
worked with Texinfo before, so there may be... issues :-)

Ludovic Courtès (2014-08-31 18:59 +0400) wrote:

[...]

>>> I would make %package-table a vhash instead of a hash table:
>>>
>>>   (define %package-table
>>>     (vlist-fold (lambda (elem result)
>>>                   (match elem
>>>                     ((name . package)
>>>                      (vhash-cons (cons (package-name package)
>>>                                        (package-version package))
>>>                                  package
>>>                                  (if (vhash-assq name result)
>>>                                      ...)))))
>>>                 vlist-null
>>>                 %packages))
>>
>> ... I left hash table, because I still don't understand how to use vhash
>> for that: a name+version key should give a list of all matching
>> packages, but AFAIU your variant would just replace one package with
>> another (with the same name+version).
>
> The key is ‘vhash-fold*’ (info "(guile) VHashes").  It allows you to
> traverse all the entries associated with a given key:
>
> scheme@(guile-user)> (vhash-cons '("guile" "2.0") 'foo
> 				 (vhash-cons '("guile" "2.0") 'bar
> 					     vlist-null))
> $12 = #<vhash 39240a0 2 pairs>
> scheme@(guile-user)> (vhash-fold* cons '() '("guile" "2.0") $12)
> $13 = (bar foo)
>
> I think that answers your question, right?

Absolutely; sorry for missing that feature.  But will it be a real
optimization?  If I want to get information for all packages, I have to
perform ‘vhash-fold*’ on ‘manifest-name->entry’ vhash for each package
(to get installed outputs).  With hash-table, I just need to use
‘hash-ref’ for each package.

Also I need to fold over unique names (I use ‘fold-manifest-entries’
from “guix-main.scm” for that) and I have no idea how vhash can help
there.

(Sorry for being intrusive on that subject)

>> As for ‘set-current-manifest-maybe!’, I'm afraid it can't be deleted.  I
>> found that I put it in some places where it shouldn't appear and I fixed
>> that, but it still stays in functions returning “package information”
>> for the elisp side (for info/list buffers).  Currently I can't invent a
>> way how to get rid of this function completely.
>
> I think the profile’s file name could be kept on the elisp side, and
> passed to the Scheme code, which wouldn’t need to keep it in a global
> variable.  That would also allow guix.el to be used on profiles other
> than the default one.

Brilliant idea!  Indeed it would be very useful to “fill” custom
profiles.  This will require some internal changes, but it shouldn't be
too hard.  I'll report when it will be done.

> That said, we can look into it later if you prefer.  WDYT?

Yes, I think dealing with ‘set-current-manifest-maybe!’ is not so
important, it may wait for better times, thanks :)

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-09-01  8:26             ` Alex Kost
@ 2014-09-01 12:10               ` Ludovic Courtès
  2014-09-02 20:22                 ` Alex Kost
  2014-09-01 22:28               ` Ludovic Courtès
  1 sibling, 1 reply; 22+ messages in thread
From: Ludovic Courtès @ 2014-09-01 12:10 UTC (permalink / raw)
  To: Alex Kost; +Cc: Guix-devel

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2014-08-31 18:59 +0400) wrote:

[...]

>> The key is ‘vhash-fold*’ (info "(guile) VHashes").  It allows you to
>> traverse all the entries associated with a given key:
>>
>> scheme@(guile-user)> (vhash-cons '("guile" "2.0") 'foo
>> 				 (vhash-cons '("guile" "2.0") 'bar
>> 					     vlist-null))
>> $12 = #<vhash 39240a0 2 pairs>
>> scheme@(guile-user)> (vhash-fold* cons '() '("guile" "2.0") $12)
>> $13 = (bar foo)
>>
>> I think that answers your question, right?
>
> Absolutely; sorry for missing that feature.  But will it be a real
> optimization?  If I want to get information for all packages, I have to
> perform ‘vhash-fold*’ on ‘manifest-name->entry’ vhash for each package
> (to get installed outputs).  With hash-table, I just need to use
> ‘hash-ref’ for each package.

‘vhash-fold*’ iterates only on the values associated with the given key;
it has time complexity linear in the number of values associated with
that key.  So no worries here (and again, 90% of the time there’ll be
exactly one package corresponding to a name/version pair.)

> Also I need to fold over unique names (I use ‘fold-manifest-entries’
> from “guix-main.scm” for that) and I have no idea how vhash can help
> there.

Would ‘vlist-fold’ work?

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> (vhash-cons 'a 1 (vhash-cons 'b 2 (vhash-cons 'a 3 vlist-null)))
$2 = #<vhash 26fd3a0 3 pairs>
scheme@(guile-user)> (vlist-fold cons '() $2)
$3 = ((a . 3) (b . 2) (a . 1))
--8<---------------cut here---------------end--------------->8---

> Yes, I think dealing with ‘set-current-manifest-maybe!’ is not so
> important, it may wait for better times, thanks :)

Yes.  :-)

I’ll check the doc later today, but it seems this is essentially ready
for merging, no?

When we merge, would you like to rewrite history and make the whole
thing appear as a single “perfect” commit, or just merge ‘emacs-ui’ into
‘master’?  (I often do the former, but I’m fine with the latter here.)

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-09-01  8:26             ` Alex Kost
  2014-09-01 12:10               ` Ludovic Courtès
@ 2014-09-01 22:28               ` Ludovic Courtès
  2014-09-02 20:22                 ` Alex Kost
  1 sibling, 1 reply; 22+ messages in thread
From: Ludovic Courtès @ 2014-09-01 22:28 UTC (permalink / raw)
  To: Alex Kost; +Cc: Guix-devel

Alex Kost <alezost@gmail.com> skribis:

> I've just pushed a manual for “guix.el” (into “emacs-ui” branch).  I
> tried my best, but I'm not a native English speaker and I have never
> worked with Texinfo before, so there may be... issues :-)

Looks good to me!  Here are some minor comments (I’m not a native
speaker either):

+GNU Guix is distributed with a user interface for GNU Emacs (which is
+also known as ``guix.el'').  It can be used for:

To make the goal clearer, what about something like:

  GNU Guix comes with a visual user interface for GNU@tie{}Emacs, known
  as ``guix.el''.  It can be used for routine package management tasks,
  pretty much like the @command{guix package} command (@pxref{Invoking
  guix package}).  Specifically, guix.el makes it easy to:

  @itemize
  @item browse and display ...
  @item search ...
  ...

Also there are a few places that mention “Guix packages”, where it
should be just “packages”, I think.

+@menu
+* Initial Setup::
+* Usage::
+* Configuration::
+@end menu

Please add something to the right-hand side.

Node names are global, so they need to be less generic, to avoid
possible name clashes with the rest of the manual.  For instance:

  guix.el Setup
  guix.el Usage
  guix.el Configuration

The section names can remain unchanged, though.

+@item
+@uref{http://nongnu.org/geiser/, Geiser}, version 0.3 or later: it is
+used for interacting with guile process.

“with the Guile process”

+In rare occasions (for example if you compile your Emacs manually and

“On rare occasions”

+@node Usage
+@subsection Usage
+
+After successful setting up ``guix.el'', you should be able to use

“Once ``guix.el'' has been successfully configured, ...”

+@item C-c C-z
+Go to the Guix REPL.

Add: (@pxref{The REPL,,, geiser, Geiser User Manual})

+@menu
+* Commands::
+* List buffer::
+* Info buffer::
+@end menu

Same remark as the other menu and node names.

+@node Configuration
+@subsection Configuration
+
+There are many variables you can modify to change the appearance or
+behavior of Emacs UI.  Some of these variables are described in this

s/UI/user interface/

+@menu
+* Guile and Build Options::
+* Keymaps::
+* Appearance::
+@end menu

RHS and node names.

--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -584,6 +584,7 @@ management tools it provides.
 * Invoking guix gc::            Running the garbage collector.
 * Invoking guix pull::          Fetching the latest Guix and distribution.
 * Invoking guix archive::       Exporting and importing store files.
+* Emacs Interface::             Package management from Emacs.
 @end menu

What about moving it higher, just after “Invoking guix package”?

Thank you!

Ludo’.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-09-01 22:28               ` Ludovic Courtès
@ 2014-09-02 20:22                 ` Alex Kost
  2014-09-02 21:13                   ` Ludovic Courtès
  0 siblings, 1 reply; 22+ messages in thread
From: Alex Kost @ 2014-09-02 20:22 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Guix-devel

Ludovic Courtès (2014-09-02 02:28 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> I've just pushed a manual for “guix.el” (into “emacs-ui” branch).  I
>> tried my best, but I'm not a native English speaker and I have never
>> worked with Texinfo before, so there may be... issues :-)
>
> Looks good to me!  Here are some minor comments (I’m not a native
> speaker either):

[...]

Thanks for all your comments.  I believe I fixed everything you
mentioned.

> +@menu
> +* Initial Setup::
> +* Usage::
> +* Configuration::
> +@end menu
>
> Please add something to the right-hand side.

Inventing something useful for the right side was the hardest part :-)

> Node names are global, so they need to be less generic, to avoid
> possible name clashes with the rest of the manual.  For instance:
>
>   guix.el Setup
>   guix.el Usage
>   guix.el Configuration
>
> The section names can remain unchanged, though.

I used “emacs” instead of “guix.el” because the dot (in “guix.el”)
breaks menu items.  Is it OK?

--
Alex

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-09-01 12:10               ` Ludovic Courtès
@ 2014-09-02 20:22                 ` Alex Kost
  2014-09-03  7:09                   ` Ludovic Courtès
  0 siblings, 1 reply; 22+ messages in thread
From: Alex Kost @ 2014-09-02 20:22 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Guix-devel

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

Ludovic Courtès (2014-09-01 16:10 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> Ludovic Courtès (2014-08-31 18:59 +0400) wrote:
>
> [...]
>
>>> The key is ‘vhash-fold*’ (info "(guile) VHashes").  It allows you to
>>> traverse all the entries associated with a given key:
>>>
>>> scheme@(guile-user)> (vhash-cons '("guile" "2.0") 'foo
>>> 				 (vhash-cons '("guile" "2.0") 'bar
>>> 					     vlist-null))
>>> $12 = #<vhash 39240a0 2 pairs>
>>> scheme@(guile-user)> (vhash-fold* cons '() '("guile" "2.0") $12)
>>> $13 = (bar foo)
>>>
>>> I think that answers your question, right?
>>
>> Absolutely; sorry for missing that feature.  But will it be a real
>> optimization?  If I want to get information for all packages, I have to
>> perform ‘vhash-fold*’ on ‘manifest-name->entry’ vhash for each package
>> (to get installed outputs).  With hash-table, I just need to use
>> ‘hash-ref’ for each package.
>
> ‘vhash-fold*’ iterates only on the values associated with the given key;
> it has time complexity linear in the number of values associated with
> that key.  So no worries here (and again, 90% of the time there’ll be
> exactly one package corresponding to a name/version pair.)

Ah, OK then.  (I still have some worries but it's just paranoia
apparently).

>> Also I need to fold over unique names (I use ‘fold-manifest-entries’
>> from “guix-main.scm” for that) and I have no idea how vhash can help
>> there.
>
> Would ‘vlist-fold’ work?
>
> scheme@(guile-user)> (vhash-cons 'a 1 (vhash-cons 'b 2 (vhash-cons 'a 3 vlist-null)))
> $2 = #<vhash 26fd3a0 3 pairs>
> scheme@(guile-user)> (vlist-fold cons '() $2)
> $3 = ((a . 3) (b . 2) (a . 1))

Sorry, I don't see how it could work.  Here is an example:


[-- Attachment #2: sample.scm --]
[-- Type: text/plain, Size: 676 bytes --]

;; What I currently have is a hash-table like this one:
(define table (make-hash-table 3))

(hash-set! table 'a '(1 2 3))
(hash-set! table 'b '(4))
(hash-set! table 'c '(5 6))

;; And I can easily fold through unique keys like this:
(hash-fold (lambda (key entries res)
             (cons (cons key (apply + entries)) res))
           '()
           table) ; => ((c . 11) (b . 4) (a . 6))

;; What you suggest is a vhash like this:
(define vhash
  (vhash-cons
   'a 1
   (vhash-cons
    'a 2
    (vhash-cons
     'a 3
     (vhash-cons
      'b 4
      (vhash-cons
       'c 5
       (vhash-cons
        'c 6 vlist-null)))))))

;; But how can I fold through unique keys there?

[-- Attachment #3: Type: text/plain, Size: 1317 bytes --]


[...]

> I’ll check the doc later today, but it seems this is essentially ready
> for merging, no?

Yes, If you don't mind that I'm still using hash-tables, I think it can
be merged.

> When we merge, would you like to rewrite history and make the whole
> thing appear as a single “perfect” commit, or just merge ‘emacs-ui’ into
> ‘master’?  (I often do the former, but I’m fine with the latter here.)

I don't have a preference here.  I can do a single commit if it is more
appropriate.  Would the following commit message be OK?

--8<---------------cut here---------------start------------->8---
Add Emacs user interface.

* configure.ac (emacsuidir): New variable.
  (AC_CONFIG_FILES): Add 'emacs/guix-init.el', 'emacs/guix-helper.scm'.
* Makefile.am: Include 'emacs.am'.
* emacs.am: New file.
* doc/emacs.texi: New file.
* doc/guix.texi: Include 'emacs.texi'.
* emacs/guix-backend.el: New file.
* emacs/guix-base.el: New file.
* emacs/guix-helper.scm.in: New file.
* emacs/guix-history.el: New file.
* emacs/guix-info.el: New file.
* emacs/guix-init.el.in: New file.
* emacs/guix-list.el: New file.
* emacs/guix-main.scm: New file.
* emacs/guix-utils.el: New file.
* emacs/guix.el: New file.
--8<---------------cut here---------------end--------------->8---


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-08-31 14:59           ` Ludovic Courtès
  2014-09-01  8:26             ` Alex Kost
@ 2014-09-02 20:22             ` Alex Kost
  2014-09-02 21:09               ` Ludovic Courtès
  1 sibling, 1 reply; 22+ messages in thread
From: Alex Kost @ 2014-09-02 20:22 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Guix-devel

Ludovic Courtès (2014-08-31 18:59 +0400) wrote:

[...]

> I think the profile’s file name could be kept on the elisp side, and
> passed to the Scheme code, which wouldn’t need to keep it in a global
> variable.  That would also allow guix.el to be used on profiles other
> than the default one.

Done (in the emacs-ui branch).  Thanks again.

--
Alex

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-09-02 20:22             ` Alex Kost
@ 2014-09-02 21:09               ` Ludovic Courtès
  0 siblings, 0 replies; 22+ messages in thread
From: Ludovic Courtès @ 2014-09-02 21:09 UTC (permalink / raw)
  To: Alex Kost; +Cc: Guix-devel

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2014-08-31 18:59 +0400) wrote:
>
> [...]
>
>> I think the profile’s file name could be kept on the elisp side, and
>> passed to the Scheme code, which wouldn’t need to keep it in a global
>> variable.  That would also allow guix.el to be used on profiles other
>> than the default one.
>
> Done (in the emacs-ui branch).  Thanks again.

Excellent!

Ludo'.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-09-02 20:22                 ` Alex Kost
@ 2014-09-02 21:13                   ` Ludovic Courtès
  0 siblings, 0 replies; 22+ messages in thread
From: Ludovic Courtès @ 2014-09-02 21:13 UTC (permalink / raw)
  To: Alex Kost; +Cc: Guix-devel

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2014-09-02 02:28 +0400) wrote:
>
>> Alex Kost <alezost@gmail.com> skribis:
>>
>>> I've just pushed a manual for “guix.el” (into “emacs-ui” branch).  I
>>> tried my best, but I'm not a native English speaker and I have never
>>> worked with Texinfo before, so there may be... issues :-)
>>
>> Looks good to me!  Here are some minor comments (I’m not a native
>> speaker either):
>
> [...]
>
> Thanks for all your comments.  I believe I fixed everything you
> mentioned.

Great, thank you for your patience.

>> +@menu
>> +* Initial Setup::
>> +* Usage::
>> +* Configuration::
>> +@end menu
>>
>> Please add something to the right-hand side.
>
> Inventing something useful for the right side was the hardest part :-)

Heh.  :-)

>> Node names are global, so they need to be less generic, to avoid
>> possible name clashes with the rest of the manual.  For instance:
>>
>>   guix.el Setup
>>   guix.el Usage
>>   guix.el Configuration
>>
>> The section names can remain unchanged, though.
>
> I used “emacs” instead of “guix.el” because the dot (in “guix.el”)
> breaks menu items.  Is it OK?

I would have made it uppercase, but otherwise yes.

Enough nitpicking: when do we merge? :-)

It seems to me the main issues have been addressed, so I’m happy with
continuing that work in ‘master’.  WDYT?

Ludo’.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-09-02 20:22                 ` Alex Kost
@ 2014-09-03  7:09                   ` Ludovic Courtès
  2014-09-03 20:53                     ` Alex Kost
  0 siblings, 1 reply; 22+ messages in thread
From: Ludovic Courtès @ 2014-09-03  7:09 UTC (permalink / raw)
  To: Alex Kost; +Cc: Guix-devel

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2014-09-01 16:10 +0400) wrote:

[...]

>> Would ‘vlist-fold’ work?
>>
>> scheme@(guile-user)> (vhash-cons 'a 1 (vhash-cons 'b 2 (vhash-cons 'a 3 vlist-null)))
>> $2 = #<vhash 26fd3a0 3 pairs>
>> scheme@(guile-user)> (vlist-fold cons '() $2)
>> $3 = ((a . 3) (b . 2) (a . 1))
>
> Sorry, I don't see how it could work.  Here is an example:
>
> ;; What I currently have is a hash-table like this one:
> (define table (make-hash-table 3))
>
> (hash-set! table 'a '(1 2 3))
> (hash-set! table 'b '(4))
> (hash-set! table 'c '(5 6))
>
> ;; And I can easily fold through unique keys like this:
> (hash-fold (lambda (key entries res)
>              (cons (cons key (apply + entries)) res))
>            '()
>            table) ; => ((c . 11) (b . 4) (a . 6))
>
> ;; What you suggest is a vhash like this:
> (define vhash
>   (vhash-cons
>    'a 1
>    (vhash-cons
>     'a 2
>     (vhash-cons
>      'a 3
>      (vhash-cons
>       'b 4
>       (vhash-cons
>        'c 5
>        (vhash-cons
>         'c 6 vlist-null)))))))
>
> ;; But how can I fold through unique keys there?

With a vhash, you can’t really do that efficiently.  ‘vlist-fold’ allows
you to iterate over the list of key/value pairs, but in the order in
which they appear (so you don’t get all the 'a, and then all the 'b.)

If that’s not sufficient, then yes, hash tables may be best.

>> When we merge, would you like to rewrite history and make the whole
>> thing appear as a single “perfect” commit, or just merge ‘emacs-ui’ into
>> ‘master’?  (I often do the former, but I’m fine with the latter here.)
>
> I don't have a preference here.  I can do a single commit if it is more
> appropriate.  Would the following commit message be OK?

Yes, that would be perfect.

Feel free to proceed whenever is convenient for you.

Thanks!

Ludo’.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Merging guix.el
  2014-09-03  7:09                   ` Ludovic Courtès
@ 2014-09-03 20:53                     ` Alex Kost
  0 siblings, 0 replies; 22+ messages in thread
From: Alex Kost @ 2014-09-03 20:53 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Guix-devel

Ludovic Courtès (2014-09-03 11:09 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:

[...]

>> I don't have a preference here.  I can do a single commit if it is more
>> appropriate.  Would the following commit message be OK?
>
> Yes, that would be perfect.
>
> Feel free to proceed whenever is convenient for you.

Merged into master, thanks.

Sorry, all I could take from my brain for the Savannah News is:

--8<---------------cut here---------------start------------->8---
Now GNU Guix has an Emacs interface.

It can be installed along with the
[http://git.savannah.gnu.org/cgit/guix.git latest Guix].  For using and
configuring see documentation.
--8<---------------cut here---------------end--------------->8---

So, I'm afraid you have to do it yourself (if this news is really
needed).

--
Alex

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2014-09-03 20:54 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-23 12:17 Merging guix.el Ludovic Courtès
2014-08-24  6:59 ` Alex Kost
2014-08-25  8:23   ` Ludovic Courtès
2014-08-25 12:31     ` Alex Kost
2014-08-25 22:03       ` Ludovic Courtès
2014-08-26  4:14         ` Alex Kost
2014-08-27 13:11 ` Alex Kost
2014-08-28 12:41   ` Ludovic Courtès
2014-08-28 18:22     ` Alex Kost
2014-08-28 20:09       ` Ludovic Courtès
2014-08-29 18:24         ` Alex Kost
2014-08-31 14:59           ` Ludovic Courtès
2014-09-01  8:26             ` Alex Kost
2014-09-01 12:10               ` Ludovic Courtès
2014-09-02 20:22                 ` Alex Kost
2014-09-03  7:09                   ` Ludovic Courtès
2014-09-03 20:53                     ` Alex Kost
2014-09-01 22:28               ` Ludovic Courtès
2014-09-02 20:22                 ` Alex Kost
2014-09-02 21:13                   ` Ludovic Courtès
2014-09-02 20:22             ` Alex Kost
2014-09-02 21:09               ` Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).