[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Place barres on fret diagrams if they can be inferred (issue 294570043 b
From: |
thomasmorley65 |
Subject: |
Place barres on fret diagrams if they can be inferred (issue 294570043 by address@hidden) |
Date: |
Thu, 03 May 2018 15:28:25 -0700 |
Hi Carl,
nice work.
A general thought:
As far as I understand the code will work automatically. Though, I
foresee some users wanting to switch it off (ofcourse not the majority,
but there is always somebody with different wishes).
Any chance to create an option for it?
Some inline remarks below.
https://codereview.appspot.com/294570043/diff/1/scm/translation-functions.scm
File scm/translation-functions.scm (right):
https://codereview.appspot.com/294570043/diff/1/scm/translation-functions.scm#newcode258
scm/translation-functions.scm:258: (let* ((barres (make-array 0 5 4)) ;
5 fingers, 4 elements/barre
Afaik, this will be the first use of make-array in our source,
so I'd love some more explanations in the comments, especially about 5
and 4 bounds.
5 is obviously the amount of possible fingers, as you stated already.
About 4: this will be filled lateron with
(barre-symbol
from-higher-string, p.e. 1
to-lower-string, p.e. 6
fret-number)
Why this order for second/third element?
I'm aware it makes no programmatical difference, but all NR examples do
it the other way round, something like:
(barre 5 1 3) and not (barre 1 5 3)
Probably exchange those to be consistent with the NR.
Here, while doing array-set! and in the filtering-condition later.
https://codereview.appspot.com/294570043/diff/1/scm/translation-functions.scm#newcode278
scm/translation-functions.scm:278: (not (= (list-ref l 3 ) 0))
Why not use (second/third/fourth l) instead of (list-ref l 1/2/3)?
https://codereview.appspot.com/294570043/diff/1/scm/translation-functions.scm#newcode289
scm/translation-functions.scm:289: (have-fingers (not (null? (filter
(lambda (sf)
Why (have-fingers (not ...))
and as predicate in line 306 as
(not have-fingers)
Couldn't both (not ...) be deleted?
https://codereview.appspot.com/294570043/
- Place barres on fret diagrams if they can be inferred (issue 294570043 by address@hidden),
thomasmorley65 <=