On 2014/01/05 01:42:55, dak wrote:
https://codereview.appspot.com/41990043/diff/60001/ly/satb.ly
File ly/satb.ly (right):
https://codereview.appspot.com/41990043/diff/60001/ly/satb.ly#newcode1
ly/satb.ly:1: %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
Missing \version number is causing an error when running convert-ly.
https://codereview.appspot.com/41990043/diff/60001/ly/satb.ly#newcode68
ly/satb.ly:68: (if (defined? name) name (if (pair? default) (car
default)
'#{#})))
'#{#} is expensive. I'd rather use *unspecified* here.
https://codereview.appspot.com/41990043/diff/60001/ly/satb.ly#newcode70
ly/satb.ly:70: #(define (sym . strings) (string->symbol (apply
string-append
strings)))
For an include file (rather than a module), this and some other macro
names seem
awfully likely to cause collisions.
https://codereview.appspot.com/41990043/diff/60001/ly/satb.ly#newcode84
ly/satb.ly:84: (let ((above (if (pair? optionals) (car optionals)
#f)))
(if x y #f) is easier to understand and write as (and x y), in
particular if x
takes up considerable space.
https://codereview.appspot.com/41990043/diff/60001/ly/satb.ly#newcode93
ly/satb.ly:93: #{#})))
Why #{#} unquoted? That evaluates at macro placement time. But I'd
use
*unspecified* anyway.
https://codereview.appspot.com/41990043/diff/60001/ly/satb.ly#newcode97
ly/satb.ly:97: \new Staff = #(identity ,name) \with {
Why #(identity ,name) here instead of #,name ?
https://codereview.appspot.com/41990043/diff/60001/ly/satb.ly#newcode104
ly/satb.ly:104: \clef #(identity ,clef)
What's with the identity? It does not make sense.
I'll skip the copious other occurences but they should be fixed.
As this patch has already been pushed I'll open a new
tracker to cover these issues.
Trevor
https://codereview.appspot.com/41990043/
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
