Re: VALUES patch

[Marco B. <em...@be...>]

Sam Steingold <sd...@gn...> writes:

> 1. you replaced eq() with eql() in the nullp() and friends.
>    this is a grave mistake since eq() is a simple macro while eql() is
>    a function call.  please revert it!

my bad, i caught that after putting the patch up and forgot to upload
it again.
 
> 3. unboundp() should be removed!  Conditionals like
>         if (!unboundp()) ...
>    are an abomination.

that's why i introduced boundp. or do you mean unboundp(foo) should become
! boundp(foo) ?

i just ran "grep -e '! *unboundp' *.d" and got nothing.

> 4. VALUES* do not require {} around them - do not put them there!

you lost me here.
 
> 5. map_hashtable() and map_hashtable_nogc() are in the patch.  why?

boundp was added to them, i did a few touch ups by hand and reran
C-cC-\.
 
> 6. it would be nice if you translated the comments (use babelfish) in
>    the functions you change.

i think this is a bad idea. it's important that the documentation be
correct, not a literal word for word translation from german to
english. we need to translate the meaning of the docs not just the
words and babelfish can't do this. i have been forced to do some
italian<->english translations and i can assure you that literal word
for word translations can really contort the meaning of the
original. some things (lisparit.d come to mind) can be translated
without really knowing the german as the code speaks fairly clearly,
but i'd be hestitant to do even this.
 
> 8. #define missingp(x) (unboundp(x) || nullp(x))
>    appears to be common

agreed, but there's already an awful lot to this patch.
 
> 9. VALUES(foo); # Wert foo
>    should read just
>    VALUES1(foo);

can #8 and #9 be done in a second patch? this second patch would also
include the elimination of K&R style declarations (it'll probably end
up being as big as this one).

-- 
-Marco
Ring the bells that still can ring.
Forget your perfect offering.
There is a crack in everything.
That's how the light gets in.
     -Leonard Cohen