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

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

[Michael Heerdegen]

Hello Emacs,

assoc.el, a library providing functions on association lists, is part
of GNU Emacs.

My requests:


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".


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.


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.

Even worse, `amake' uses `aput', thus sorts the alist built so far in
recursion.

There is a package in apel called "alist.el" which provides very
similar functions.  You can use it as a reference.


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

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


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


Thanks,

Michael.



In GNU Emacs 23.2.1 (i486-pc-linux-gnu, GTK+ Version 2.20.0)
 of 2010-12-11 on raven, modified by Debian
Windowing system distributor `The X.Org Foundation', version 11.0.10707000
configured using `configure  '--build' 'i486-linux-gnu' '--build' 'i486-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var/lib' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--enable-locallisppath=/etc/emacs23:/etc/emacs:/usr/local/share/emacs/23.2/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/23.2/site-lisp:/usr/share/emacs/site-lisp:/usr/share/emacs/23.2/leim' '--with-x=yes' '--with-x-toolkit=gtk' '--with-toolkit-scroll-bars' 'build_alias=i486-linux-gnu' 'CFLAGS=-DDEBIAN -g -O2' 'LDFLAGS=-g' 'CPPFLAGS=''

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)
 

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

[Glenn Morris]

tags 8126 wontfix
close 8126
stop

In Emacs trunk, assoc.el has been moved to lisp/obsolete.
So it won't be getting any further documentation or improvements.