Thanks Neil
Sorry about the trailing spaces. I was used to my Windows editor
removing
them automatically; gedit can't, AFAICS. However, I've just
discovered that
git rebase --warning=fix will remove them, so as long as I remember
to do
that they should be gone.
I think I've corrected the formatting errors as you suggested.
Could you please
explain when I should use a 1-space indent and when 2-space, please,
otherwise
this seems just arbitrary. I do find a 1-space indent makes it more
difficult to
see the alignments when the lines are some distance apart.
So is http://codereview.appspot.com/164063 now ready to go?
I'll amend the docs after pushing.
Trevor
----- Original Message -----
From: <n.putt...@gmail.com>
To: <tdanielsmu...@googlemail.com>; <carl.d.soren...@gmail.com>
Cc: <re...@codereview.appspotmail.com>; <lilypond-devel@gnu.org>
Sent: Sunday, December 13, 2009 7:54 PM
Subject: Re: Add option to indicate frets by letters in tablature
(issue164063)
Hi Trevor,
Here are a few comments as promised earlier.
Cheers,
Neil
http://codereview.appspot.com/164063/diff/5001/5002
File Documentation/changes.tely (right):
http://codereview.appspot.com/164063/diff/5001/5002#newcode73
Documentation/changes.tely:73: \new TabStaff
trailing space
http://codereview.appspot.com/164063/diff/5001/5002#newcode79
Documentation/changes.tely:79: \set fretLabels = #`(,(markup
#:with-color red "a")
trailing space
http://codereview.appspot.com/164063/diff/5001/5002#newcode80
Documentation/changes.tely:80: "b"
trailing space
http://codereview.appspot.com/164063/diff/5001/5003
File input/regression/tablature-letter.ly (right):
http://codereview.appspot.com/164063/diff/5001/5003#newcode23
input/regression/tablature-letter.ly:23: \set fretLabels = #`("α"
"β"
"γ")
#'
http://codereview.appspot.com/164063/diff/5001/5005
File lily/grob.cc (right):
http://codereview.appspot.com/164063/diff/5001/5005#newcode160
lily/grob.cc:160: /* Call the scheme procedure stencil-whiteout in
scm/stencils.scm */
I'd use { } here if you want this comment placed before the code.
http://codereview.appspot.com/164063/diff/5001/5005#newcode162
lily/grob.cc:162: retval = *unsmob_stencil (scm_call_1 (
If you newline at the parenthesis, the following lines should be
indented accordingly, but this will make the line quite long so
it's
probably better to start on the following line:
retval
= *unsmob_stencil (scm_call_1 (ly_lily_module_constant
("stencil-whiteout"),
retval.smobbed_copy()));
http://codereview.appspot.com/164063/diff/5001/5010
File scm/translation-functions.scm (right):
http://codereview.appspot.com/164063/diff/5001/5010#newcode398
scm/translation-functions.scm:398: (make-vcenter-markup
(make-vcenter-markup
(cond
((= 0 (length labels))
(string (integer->char (+ fret (char->integer #\a)))))
((and (<= 0 fret) (< fret (length labels)))
(list-ref labels fret))
(else
(ly:warning "No label for fret ~a (~a on string ~a);
only ~a fret labels provided"
fret pitch string-number (length labels))
".")))))
http://codereview.appspot.com/164063/diff/5001/5010#newcode414
scm/translation-functions.scm:414: (make-vcenter-markup
(make-vcenter-markup
(format
"~a"
(- (ly:pitch-semitones pitch)
(list-ref tuning
;; remove 1 because list index starts at 0
;;and guitar string at 1.
(1- string-number))))))
http://codereview.appspot.com/164063/diff/5001/5010#newcode432
scm/translation-functions.scm:432: (make-vcenter-markup
(make-vcenter-markup
(let ((fret (- (ly:pitch-semitones pitch)
(list-ref tuning (1- string-number)))))
(number->string (cond
((and (> fret 0) (= string-number 5))
(+ fret 5))
(else fret)))))))
http://codereview.appspot.com/164063
--------------------------------------------------------------------------------
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
