[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Carl . D . Sorensen
Subject: Re: lily-guile updates and CG: "Scheme->C interface" section. (issue 4917044)
Date: Fri, 19 Aug 2011 18:04:38 +0000

THanks for doing this!

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.

I think the code changes should be separated from the doc changes.  I
disagree with your changes on directions; they don't need to be


File Documentation/contributor/programming-work.itexi (right):
Documentation/contributor/programming-work.itexi:1811: (see
@ref{LilyPond programming languages}).
I'd prefer to see a URL to the appropriate Guile page here.
Documentation/contributor/programming-work.itexi:1830: or warning when
compiling but must be avoided:
I think that you should reference the paragraph that David pointed out
in his email:


The code in your example *will work*, but it is not following the API
guidelines.  So we should be clear about why we're doing this.  It's for
standards compliance, not because of lack of functionality.
Documentation/contributor/programming-work.itexi:1833: scm_string_p
([any_SCM_you_want]) == SCM_BOOL_T
I think it would be better to say:

if (scm_string_p (scm_value) == SCM_BOOL_T)

i.e. put in a real variable name, instead of a place holder)
Documentation/contributor/programming-work.itexi:1840: quicker in terms
of execution time.
Are you sure of the execution time argument?  I'm not.
Documentation/contributor/programming-work.itexi:1845: a misnomer in the
reference: it says @code{scm_is_string} returns @code{#t}
Are you sure this is correct?

The current page of the API reference says:

— Scheme Procedure: string? obj
— C Function: scm_string_p (obj)
Return #t if obj is a string, else #f.

— C Function: int scm_is_string (SCM obj)
Returns 1 if obj is a string, 0 otherwise.


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.


So scm_is_string returns a C int, with a value of 0 or 1.

scm_string_p returns a C SCM, with a value of #t or #f (but we can't
access those values directly from C).

I think the key point that we should get across (which I didn't
understand until I started this review) is that we should use generally
use C functions and macros that have no corresponding Scheme procedures.
 The C functions with corresponding Scheme procedures return SCM values,
and we can't use them for anything but assignment in C.

Or perhaps it's clearer to say that for anything but assignment in C, we
should use only C Functions and Macros with non-SCM return values.
Documentation/contributor/programming-work.itexi:1876: Here is a list of
these functions:
Rather than explaining these functions, I think we should only explain
cases where the usage is non-obvious (i.e. using to_boolean on property
returns, since unset properties often aren't boolean values.
Documentation/contributor/programming-work.itexi:1878: TODO: complete
this list.
See my comment below.  I don't think we should have exhaustive lists,
unless they are automatically generated.
Documentation/contributor/programming-work.itexi:1882: Convert a Scheme
boolean @var{b} to a C boolean, else return false.
If b is a Scheme boolean value, return the corresponding C boolean
value, else return false.


Return @code{true} if @var{b} is @code{SCM_BOOL_T}, otherwise return
Documentation/contributor/programming-work.itexi:1918: TODO: complete
this list.
I'm not in favor of having a complete list here.  Unless we can identify
a means of automatically creating the list from the source code, this
will invariably get out of date.  Better not to have it than to have it
be out of date.
Documentation/contributor/programming-work.itexi:1927: @subsubheading
Interval ly_scm2interval (SCM p)
Rather, I should say it converts a Scheme interval (which happens to be
a pair of numbers) to a C Interval.
Documentation/contributor/programming-work.itexi:1934: Convert a pair of
floating point numbers @var{p} to an offset,
Similarly, it converts a Scheme offset to a C offset.
File lily/ (right):
lily/ if (scm_is_integer (s))
Is this correct?  Should directions be limited to integers?

For example, the backend property align-dir is a direction.  It can be
set anywhere between -1 and 1, so that I can center two grobs, or left
align them, or right align them, or align them with a bias to the left
or to the right.  0.5 and -0.5 are valid values for align-dir.

I think the old code here is correct, and the doc-string is wrong.

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.
File lily/ (right):
lily/ if (scm_is_integer (s))
I think integer is correct here.  We don't have axes that are halfway
between Xand Y.
lily/ if (scm_is_integer (s))
I think number was correct.  Directions are not limited to -=1, 0, 1.
THey can be any number between -1 and 1.  See earlier comment.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]