[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Allow left-handed fret-markups (issue 339270043 by address@hidden)
From: |
thomasmorley65 |
Subject: |
Re: Allow left-handed fret-markups (issue 339270043 by address@hidden) |
Date: |
Tue, 23 Jan 2018 01:30:15 -0800 |
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/