From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: =?UTF-8?Q?Jorgen_Sch=C3=A4fer?= Newsgroups: gmane.emacs.devel Subject: Re: [Emacs-diffs] master b689b90: Package archives now have priorities. Date: Sat, 17 Jan 2015 13:27:26 +0100 Message-ID: References: <20150116102411.11014.8945@vcs.savannah.gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Trace: ger.gmane.org 1421498833 25960 80.91.229.3 (17 Jan 2015 12:47:13 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 17 Jan 2015 12:47:13 +0000 (UTC) Cc: emacs-devel To: Artur Malabarba Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Jan 17 13:47:13 2015 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1YCSmV-0008LI-HW for ged-emacs-devel@m.gmane.org; Sat, 17 Jan 2015 13:47:11 +0100 Original-Received: from localhost ([::1]:59007 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YCSmU-0007kl-Pw for ged-emacs-devel@m.gmane.org; Sat, 17 Jan 2015 07:47:10 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:47434) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YCSTV-0004Yp-9W for emacs-devel@gnu.org; Sat, 17 Jan 2015 07:27:34 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YCSTQ-0005Gr-Bs for emacs-devel@gnu.org; Sat, 17 Jan 2015 07:27:33 -0500 Original-Received: from loki.jorgenschaefer.de ([87.230.15.51]:43316) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YCSTQ-0005GU-6o for emacs-devel@gnu.org; Sat, 17 Jan 2015 07:27:28 -0500 Original-Received: by loki.jorgenschaefer.de (Postfix, from userid 998) id D536E200F03; Sat, 17 Jan 2015 13:27:26 +0100 (CET) Original-Received: from mail-we0-f175.google.com (mail-we0-f175.google.com [74.125.82.175]) by loki.jorgenschaefer.de (Postfix) with ESMTPSA id 59DA0200EFF for ; Sat, 17 Jan 2015 13:27:26 +0100 (CET) Original-Received: by mail-we0-f175.google.com with SMTP id k11so24336333wes.6 for ; Sat, 17 Jan 2015 04:27:26 -0800 (PST) X-Received: by 10.194.191.227 with SMTP id hb3mr39783824wjc.79.1421497646130; Sat, 17 Jan 2015 04:27:26 -0800 (PST) Original-Received: by 10.180.102.202 with HTTP; Sat, 17 Jan 2015 04:27:26 -0800 (PST) In-Reply-To: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 87.230.15.51 X-Mailman-Approved-At: Sat, 17 Jan 2015 07:47:08 -0500 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:181373 Archived-At: On Sat, Jan 17, 2015 at 1:12 PM, Artur Malabarba 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 =E2=80=93 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 =E2=80=93 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