bug#8126: 23.2; Improvement requests for assoc.el

[Stefan Monnier]

> 1. Please mention this lib in the Elisp Manual.

> assoc.el provides some essential elementary functions for alists.
> These should be mentioned in the Elisp Manual (at least `aput',
> `adelete') under "Association Lists".

It seems to be very little used.  Maybe it's for lack of exposure, but
maybe it's because the functionality is not often needed.  So I'm not
convinced it needs to be advertised much more (especially given its
current state, as discussed below).

> 2. Please remove the misleading word `sort' from the header
> descriptions.

> This file doesn't include any function for sorting alists.  There is
> `asort', but it actually doesn't sort the alist, it only moves an
> element with a given KEY to the head of the alist.

Another way to fix it would be to provide a sort function, tho I guess
`sort' works just fine.

> 3. Most of the functions in this library use `asort' (same file) as a
> helper.  `asort' uses `sort' only to put a cons cell with a given key
> to the head of the alist.  This is very inefficient.  Thus, `aput',
> `adelete', `aget' and `amake' have a bad run-time behavior.

Patch welcome.

> 4. `eval' -> `symbol-value'

> All occurrences of `eval' can be replaced with `symbol-value'.

Indeed.  Abuse of `eval' is something I really dislike.
Using `symbol-value' is much better, tho I'd even much prefer using
`cdr' instead (i.e. not (ab)use symbols but a cons-cell with
a well-known car value, such as `alist' or somesuch).

> 5. There are currently no autoload cookies in "assoc.el".  Maybe they
> should be added.

`assoc.el' is currently meant to be included with (require 'assoc) and
I think it works well that way.

For now, I've installed the patch below,
Thanks,


        Stefan


=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog      2011-02-24 08:45:25 +0000
+++ lisp/ChangeLog      2011-02-26 21:07:45 +0000
@@ -1,3 +1,9 @@
+2011-02-26  Stefan Monnier  <monnier@iro.umontreal.ca>
+
+       * emacs-lisp/assoc.el: Remove misleading `sort' (bug#8126).
+       (aput, adelete, amake): Replace `eval' -> `symbol-value'.
+       Suggested by Michael Heerdegen <michael_heerdegen@web.de>.
+
 2011-02-24  Glenn Morris  <rgm@gnu.org>
 
        * files-x.el (modify-dir-local-variable): Handle dir-locals from

=== modified file 'lisp/emacs-lisp/assoc.el'
--- lisp/emacs-lisp/assoc.el    2011-01-25 04:08:28 +0000
+++ lisp/emacs-lisp/assoc.el    2011-02-26 21:02:38 +0000
@@ -1,4 +1,4 @@
-;;; assoc.el --- insert/delete/sort functions on association lists
+;;; assoc.el --- insert/delete functions on association lists
 
 ;; Copyright (C) 1996, 2001-2011  Free Software Foundation, Inc.
 
@@ -35,7 +35,7 @@
 the order of any other key-value pair.  Side effect sets alist to new
 sorted list."
   (set alist-symbol
-       (sort (copy-alist (eval alist-symbol))
+       (sort (copy-alist (symbol-value alist-symbol))
             (function (lambda (a b) (equal (car a) key))))))
 
 
@@ -75,7 +75,7 @@
   (lexical-let ((elem (aelement key value))
                alist)
     (asort alist-symbol key)
-    (setq alist (eval alist-symbol))
+    (setq alist (symbol-value alist-symbol))
     (cond ((null alist) (set alist-symbol elem))
          ((anot-head-p alist key) (set alist-symbol (nconc elem alist)))
          (value (setcar alist (car elem)))
@@ -87,7 +87,7 @@
 Alist is referenced by ALIST-SYMBOL and the key-value pair to remove
 is pair matching KEY.  Returns the altered alist."
   (asort alist-symbol key)
-  (lexical-let ((alist (eval alist-symbol)))
+  (lexical-let ((alist (symbol-value alist-symbol)))
     (cond ((null alist) nil)
          ((anot-head-p alist key) alist)
          (t (set alist-symbol (cdr alist))))))
@@ -133,7 +133,7 @@
          (t
           (amake alist-symbol keycdr valcdr)
           (aput alist-symbol keycar valcar))))
-  (eval alist-symbol))
+  (symbol-value alist-symbol))
 
 (provide 'assoc)