Re: lily-guile updates and CG: "Scheme->C interface" section. (issue 4917044)

[bordage . bertrand]

On 2011/08/19 18:04:38, Carl wrote:
> THanks for doing this!

And thanks for this excellent review!

> I have some comments about the docs.  I think they're too tutorial,
and I think
> the exhaustive lists are unwieldy and should be eliminated.  THe
source should
> be the reference.

You're right.  Specifying the meaning of the prefixes "ly_", "robust_",
... should be enough, but requires to unify these functions (name and
default value).  As you can see, I made a few changes in this direction.
 If you think this is a good idea, I can unify the whole interface.  Of
course, there's a lot of downstream name changes to do.  And maybe make
a small changelog somewhere for the developpers.

> I think the code changes should be separated from the doc changes.

This is two commits I uploaded at once.  Since I wrote the small
references according to the changes made to the C files, I thought this
would be easier to understand.

> Also, the Guile API says :

> The type of the return value of a C function that corresponds to a
Scheme
> function is always SCM. In the descriptions below, types are therefore
often
> omitted but for the return value and for the arguments.

> (see

<http://www.gnu.org/software/guile/manual/html_node/API-Overview.html#API-Overview>
> )

Ok, so there's no logical error.  Forgive me.


http://codereview.appspot.com/4917044/diff/1/lily/general-scheme.cc#newcode110
> The code expresses that any number between -1 and 1 is a valid
direction, not
> just the integers -1, 0 and 1.

> I think this change should not be pushed.

So I guess the scm_to_int has to be changed for scm_to_double in these
files...
http://codereview.appspot.com/4917044/diff/1/lily/lily-guile.cc#newcode217
http://codereview.appspot.com/4917044/diff/1/lily/general-scheme.cc#newcode112


http://codereview.appspot.com/4917044/diff/1/lily/lily-guile.cc#newcode181
> lily/lily-guile.cc:181: if (scm_is_integer (s))
> I think integer is correct here.  We don't have axes that are halfway
between
> Xand Y.

This could be very nice!


I totally agree with anything else.
I'll make a new patch set before Tuesday.

Thanks,
Bertrand

http://codereview.appspot.com/4917044/