Re: [Emacs-diffs] master b689b90: Package archives now have priorities.

[Jorgen Schäfer]

On Sat, Jan 17, 2015 at 1:12 PM, Artur Malabarba
<address@hidden> wrote:
>>>> +(defun package--add-to-alist (pkg-desc alist)
>>>> +  "Add PKG-DESC to ALIST.
>>>> +
>>>> +Packages are grouped by name. The package descriptions are sorted
>>>> +by version number."
>>>> +  (let* ((name (package-desc-name pkg-desc))
>>>> +         (priority-version (package-desc-priority-version pkg-desc))
>>>> +         (existing-packages (assq name alist)))
>>>> +    (if (not existing-packages)
>>>> +        (cons (list name pkg-desc)
>>> This list should be a cons, probably why the test is failing.
>>
>> Why should this be a cons? The alist maps package names to ordered
>> package descriptors – I guess (cons name (list pkg-desc)) would be
>> clearer in intent.
>
> Reading your code again, I see there are places where this `list' is
> correct, but there's also one point where I think it's wrong.
> Note the following diff section. It used to push `(cons
> (package-desc-name pkg-desc) pkg-desc)', and now it uses
> `package--add-to-alist' which pushes a list instead of a cons. Do you
> see what I mean?

Yes – the code used to keep only one version around, which made
finding the correct version by archive priority more tricky, so I
changed it to keep a full list. That was also what another part of the
code already did, so I could unify the two into a single function. All
accesses to that list should be updated appropriately.

(The whole logic in package.el is quite convoluted and has a lot of
duplication with subtly different semantics, so I would not be too
surprised if there are some edge cases I missed, though.)

> I also have a very tiny peeve here. Could we name this function
> `package--alist-append' or something like this?
>
> It's just that `add-to-alist' really reminds me of `add-to-list',
> which has a very different effect (the list variable is the first
> argument, it's referenced by name instead of value, and it's changed
> destructively).
> If you don't agree I won't push it. :-)

Go ahead, I do not have any strong feelings about the names.

Regards,
Jorgen