[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Issue 3720: Built-in templates for SATB vocal scores (issue 41990043
From: |
dak |
Subject: |
Re: Issue 3720: Built-in templates for SATB vocal scores (issue 41990043) |
Date: |
Sun, 05 Jan 2014 01:42:56 +0000 |
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.
https://codereview.appspot.com/41990043/
- Re: Issue 3720: Built-in templates for SATB vocal scores (issue 41990043),
dak <=