Re: Place barres on fret diagrams if they can be inferred (issue 294570043 by address@hidden)

[Carl . D . Sorensen]

Reviewers: thomasmorley651,

Message:
Harm,

Thanks for the great comments.

If the user doesn't want the barre to be displayed, they can avoid it by
setting fret-diagram-details.barre-type = #'none

Thanks,

Carl



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
On 2018/05/03 22:28:25, thomasmorley651 wrote:
> 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.

Hmm.  I just wanted to make a range from lower string number to higher
string number, because that's the way I think about intervals.  I can't
see any problem with making it go from higher string number to lower
string number.  And I guess that consistency with the NR is not a bad
idea, even though I don't think it's essential.

https://codereview.appspot.com/294570043/diff/1/scm/translation-functions.scm#newcode278
scm/translation-functions.scm:278: (not (= (list-ref l 3 ) 0))
On 2018/05/03 22:28:25, thomasmorley651 wrote:
> Why not use (second/third/fourth l) instead of (list-ref l 1/2/3)?

Because when I learned Scheme more than 30 years ago, first/second were
"syntactic sugar" and were frowned upon.

But I can see that for this case, first and second would be easier for
the casual reader to understand.  Good suggestion.

https://codereview.appspot.com/294570043/diff/1/scm/translation-functions.scm#newcode289
scm/translation-functions.scm:289: (have-fingers (not (null? (filter
(lambda (sf)
On 2018/05/03 22:28:26, thomasmorley651 wrote:
> Why (have-fingers (not ...))
> and as predicate in line 306 as
> (not have-fingers)
> Couldn't both (not ...) be deleted?

Certainly, if we changed the name from have-fingers to no-fingers.  And
getting rid of the negative logic would certainly make it easier to
understand.  Good call!

Description:
Place barres on fret diagrams if they can be inferred

Please review this at https://codereview.appspot.com/294570043/

Affected files (+65, -6 lines):
  A Documentation/snippets/new/automatic-fretboards-barre.ly
  M scm/translation-functions.scm


Index: Documentation/snippets/new/automatic-fretboards-barre.ly
diff --git a/Documentation/snippets/new/automatic-fretboards-barre.ly  
b/Documentation/snippets/new/automatic-fretboards-barre.ly
new file mode 100644
index  
0000000000000000000000000000000000000000..6423346b1607b093d357df083bfd9fa727a7a9d1
--- /dev/null
+++ b/Documentation/snippets/new/automatic-fretboards-barre.ly
@@ -0,0 +1,22 @@
+\version "2.19.21"
+
+\header {
+  lsrtags = "fretted-strings"
+
+  texidoc = "
+When automatic fretboards are used, barre indicators will be drawn whenever
+one finger is responsible for multiple strings.
+
+If no finger indications are given in the chord from which the automatic
+fretboard is created, no barre indicators will be included, because
+there is no way to identify where barres should be placed.
+
+"
+  doctitle = "Barres in automatic fretboards"
+} % begin verbatim
+
+\new FretBoards {
+  <f,-1 c-3 f-4 a-2 c'-1 f'-1>1
+  <f, c f a c' f'>1
+}
+
Index: scm/translation-functions.scm
diff --git a/scm/translation-functions.scm b/scm/translation-functions.scm
index  
22f8648c31e8c0dcaacdad9a4894c6ef057bd0fc..9cd54b6ce346b594127b5ad6ba479f15cdf930fc  
100644
--- a/scm/translation-functions.scm
+++ b/scm/translation-functions.scm
@@ -252,14 +252,44 @@ If the context-property @code{supportNonIntegerFret}  
is set @code{#t},
 micro-tones are supported for TabStaff, but not not for FretBoards."

   ;;  helper functions
+  (define (barre-list string-frets)
+    "Create a barre-list that reflects the string, fret, and finger
+entries in @var{string-frets}."
+    (let* ((barres (make-array 0 5 4)) ; 5 fingers, 4 elements/barre
+           (add-string-fret
+             (lambda(sf)
+               (let ((string (car sf))
+                     (fret (cadr sf))
+                     (finger (if (null? (caddr sf)) 0 (caddr sf))))
+                   (if (and (not (= fret 0)) (not (= finger 0)))
+                       (begin
+                         (array-set! barres 'barre finger 0)
+                         (array-set! barres fret finger 3)
+                         (if (or (> (array-ref barres finger 1) string)
+                                 (= 0 (array-ref barres finger 1)))
+                             (array-set! barres string finger 1))
+                         (if (or (< (array-ref barres finger 2) string)
+                                 (= 0 (array-ref barres finger 2)))
+                             (array-set! barres string finger 2)))))))
+           (barre-list (begin
+                        (map add-string-fret string-frets)
+                        (array->list barres))))
+       (filter (lambda(l) (and (eq? (car l) 'barre)
+                               (not (= (list-ref l 3 ) 0))
+                               (< (list-ref l 1) (list-ref l 2))))
+               barre-list)))
+

   (define (string-frets->placement-list string-frets string-count)
     "Convert @var{string-frets} to @code{fret-diagram-verbose}
 dot placement entries."
     (let* ((placements (list->vector
                         (map (lambda (x) (list 'mute  x))
-                             (iota string-count 1)))))
-
+                             (iota string-count 1))))
+           (have-fingers (not (null? (filter (lambda (sf)
+                                          (not (null? (caddr sf))))
+                                        string-frets))))
+           (b-list (barre-list string-frets)))
       (for-each (lambda (sf)
                   (let* ((string (car sf))
                          (fret (cadr sf))
@@ -269,11 +299,13 @@ dot placement entries."
                      (1- string)
                      (if (= 0 fret)
                          (list 'open string)
-                         (if finger
-                             (list 'place-fret string fret finger)
-                             (list 'place-fret string fret))))))
+                         (if (null? finger)
+                             (list 'place-fret string fret)
+                             (list 'place-fret string fret finger))))))
                 string-frets)
-      (vector->list placements)))
+      (if (or (null? b-list) (not have-fingers))
+          (vector->list placements)
+          (append (vector->list placements) b-list))))

   (define (placement-list->string-frets placement-list)
     "Convert @var{placement-list} to string-fret list."
@@ -383,6 +415,11 @@ the current tuning?"
                   (not (ly:context-property context 'supportNonIntegerFret  
#f)))
                 (ly:warning (_ "Missing fret for pitch ~a on string ~a")
                             (car pitch-entry) string)))
+        (if (and (= this-fret 0)
+                 (and finger
+                     (not (null? finger))))
+            (ly:warning (_ "Open fret on string ~a has finger of ~a")
+                        string finger))
         (delete-free-string string)
         (set! specified-frets (cons this-fret specified-frets))
         (list-set! string-fret-fingers