Re: [PATCH] emacs: Rewrite scheme side in a functional manner.

[Alex Kost]

Ludovic Courtès (2014-09-21 23:27 +0400) wrote:

> Alex Kost <address@hidden> skribis:
>
>> Ludovic Courtès (2014-09-20 18:11 +0400) wrote:

[...]

>>> Overall it looks OK.  I’m concerned with code duplication between emacs/
>>> and guix/, though, and there are also a few stylistic issues, and
>>> missing or terse docstrings.
>>
>> I don't understand what duplication you mean.
>
> I was thinking about things generic enough to be in (guix profiles),
> things that ‘guix package’ or guix-web could use.
>
> That said, it’s also fine do to things incrementally: write things
> specifically for guix.el’s need, and generalize them later when we have
> a clearer understanding of the situation.  I just thought it’s worth
> keeping in mind.

Thanks, I keep it in mind.  Such incremental approach is the only way I
can write code.  I'm not able to construct a proper thing from the very
beginning: I always make big changes when something new is added and I
notice that a new wave of generalization is required.

[...]

>> We discussed using vhash, but I don't see how it can replace hash
>> table.  Here is the excerpt from the earlier message:
>
> Right, I remember that.  My point was just that either this optimization
> is not strictly needed, or it could go in (guix profiles), whether it
> uses a vhash or a hash table actually.
>
> But I think we actually agree on that, so that can still be addressed at
> a later stage.

Thanks.

[...]

>> To get information about packages/outputs, different “search-types” are
>> used (like ‘name’, ‘all-available’, ‘installed’).  These “search-types +
>> search-vals” are transformed into a list of package/output patterns.
>>
>> Package patterns are:
>>
>> - package record;
>> - list of name, version;
>> - list of name, version, manifest entries;
>> - list of name, version, manifest entries, packages.
>
> Oh, OK.  Do remove any ambiguity, an option would be to call them
> “matches”, “search results”, “descriptors”, or “specifications”,
> perhaps.  WDYT?
>
> (Maybe this is just bikeshedding, so your call.)

I leave a “pattern” name for now.

>> Output patterns are:
>>
>> - package record;
>> - list of package record, output;
>> - manifest entry record;
>> - list of manifest entry record, packages;
>> - list of name, version, output.
>>
>> Then these patterns are transformed into entries (alists with
>> parameters/values) using “pattern->entries” functions created by
>> ‘package-pattern-transformer’ and ‘output-pattern-transformer’.
>
> What about s/entries/sexps/?  That would make it clearer that the intent
> is to serialize things, not to manipulate them internally.

Yes, it is better, thanks.  I settled to “sexp” for this thing.

>>>> +(define (get-package/output-entries profile params entry-type
>>>> +                                    search-type search-vals)
>>>> +  "Return list of package or output entries."
>>>
>>> Never ‘get’.
>>
>> Do you mean I need to remove "get-" from the procedures' names?
>
> Yes.

Done.

[...]

>>> The docstring is too terse.
>>
>> The concept of "entry" is too common for the code in “guix-main.scm” to
>> write about it in every docstring.
>
> Yes, but still: there are 5 parameters.  I can tell what ‘profile’ is,
> but I have no clear idea about the type and meaning of the others at
> first glance.  Presumably ‘search-type’ and ‘entry-type’ are symbols,
> but then that still leaves a lot to be found out.

I improved some docstrings.

[...]

Thanks for all your recommendations, I tried to follow them.
I'm attaching the improved patch.  Is it good enough now?

0001-emacs-Rewrite-scheme-side-in-a-functional-manner.patch
Description: Text Data