Fix key and describe argument order of test

[okamsn]

key should only be applied to the sequences, not the value to accumulate.

;; => (1 . 2)
(cl-find 1 '((1 . 2) (3 . 4)) :key #'car)

;; => ((1 . 2) (3 . 4))
(cl-member 1 '((1 . 2) (3 . 4)) :key #'car)

;; Should not error:
(loopy (with (coll '((1 . 2) (3 . 4))))
       (list i '(1 3))
       (adjoin i :key #'car))

Similarly, we should make sure that test also passes arguments as (VAR, VAL), which is the same order these arguments use in the command call ((adjoin VAR VAL)).

TODO: Check behavior of all functions that use key, such as cl-sort.

[okamsn]

Turns out, the CLHS describes adjoin incorrectly. It describes :key as
being applied only to the sequence, as in functions like member and find,
but it actually applied to both arguments like in pushnew.

See:

• https://stackoverflow.com/questions/22359337/does-adjoin-in-common-lisp-work-as-per-hyperspec-when-used-with-key
• https://www.cliki.net/Issue%20ADJOIN-SPECIFICATION
• http://clhs.lisp.se/Body/f_adjoin.htm#adjoin

Union also applies :key to both arguments.

Therefore, we do not seem to have a bug, but we should still check all uses and
better document the behaviors.

[okamsn]

Now, the questions become:

1. Do we need :key if we already have a defined argument order for
   :test, which cl-lib has and seq lacks?
2. Should the defined argument order should be (NEW-VAL EXISTING-VAL), as in
   things like cl-member and memq, or should it be (EXISTING-VAL NEW-VAL),
   as in things like seq-reduce, seq-union, seq-contains-p, and the loop
   commands?

As Loopy is an Emacs Lisp library and not a Common Lisp library, I think that it
makes sense to follow seq and not cl-lib, mainly because :key is redundant
and following the order of the loop commands is OK because it also follows
seq. Similarly, we chose to make the default test function equal, as done
in seq and some other Emacs Lisp functions, instead of eql, as done in
cl-lib.

[okamsn]

The benefit of key is that we can apply the key function only once to the
tested argument when seeing if it is a member of the list. For example,

;; => ((a . 1) (b . 2) (c . 4))
(loopy (list i '((a . 1) (b . 2) (a . 3) (c . 4)))
       (adjoin i :key #'car))

should work similarly to

;; => ((a . 1) (b . 2) (c . 4))
(loopy (with (test-val))
       (list i '((a . 1) (b . 2) (a . 3) (c . 4)))
       (set test-val (car i))
       (adjoin i :test (lambda (seq-val _)
                         (equal (car seq-val)
                                test-val))))

, the membership test being something like

(cl-with-gensyms (test-val seq-val)
  `(cl-loop with ,test-val = (funcall ,key ,element)
            for ,seq-val in ,list
            thereis (funcall ,test (funcall ,key ,seq-val) ,test-val)))

.

[okamsn]

We have decided to keep key. This issue is fixed by PR #177.

[okamsn]

Done in #177.