On 2018/01/22 23:52:29, Carl wrote:
Looks good to me, but I have one suggestion.
Thanks,
Carl
https://codereview.appspot.com/339270043/diff/1/scm/fret-diagrams.scm File scm/fret-diagrams.scm (right):
https://codereview.appspot.com/339270043/diff/1/scm/fret-diagrams.scm#newcode365
scm/fret-diagrams.scm:365: ((and (eq? orientation 'landscape)
left-handed)
I would tend to write this as ((eq? orientation 'landscape) (cons fret-coordinate (if left-handed (- (1- string-count) string-coordinate) (- string-coordinate (1- string-count))) (eq? orientation 'opposing-landscape) (cons (- fret-coordinate) (if left-handed (- string-coordinate
(1-
string-count))
(- (1-
string-count) string-coordinate))) (else (cons (if left-handed (- string-coordinate) string-coordinate)
(-
fret-coordinate)))))
I think it shows the structure better (i.e. it shows three different orientations, and it explicitly shows where the left-handed changes
things (y
coordinate for landscape, x coordinate for regular). But I don't
insist on this
by any means.
Yep, the difference between left- and right-handed are certain negative instead of a positive values. I'd love to use a multiplier retrieved from left-handed. Though, how to code properly and user-friendly. One could do (1) separate conditions (that's what I've done). Disadvantage: code is more complex. No multiplier. (2) Let 'left-handed accept RIGHT/LEFT. Disadvantage is the need to input it via an quasiquoted list (3) Let 'left-handed accept -1/1. Disadvantage: it may be not that obvious to the user what -1/1 actually means (4) Let 'left-handed accept symbols left/right. Disadvantage: need to transform these symbols into -1/1. Something like (if (eq? left-handed 'left) -1 1) needs to be added. (4) Drop 'left-handed and get the -1/1 from (sign string-distance) Opinions? https://codereview.appspot.com/339270043/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel