Improve markup->string (issue 347000043 by address@hidden)

[thomasmorley65]

Reviewers: ,

Message:
Please review.
There are some TODOs in the code where I'd appreciate some feedback.
Thanks-

Description:
Improve markup->string

'all-relevant-markup-commands' is now a toplevel-defined procedure.
So it is not longer a part of the rekursive 'markup->string'.
It needs to be a procedure, because not all bindings of the
lily-module are already done in markup.scm, so it should be
evaluated at the time 'markup->string' is called.
Additionally we gain the chance to have user-defined
markup-commands from '(current-module)' been processed as well.

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

Affected files (+31, -23 lines):
  M scm/markup.scm


Index: scm/markup.scm
diff --git a/scm/markup.scm b/scm/markup.scm
index  
14a007a95bb27f8a89da2aeb51a607695af40068..169e52c78e3b8b15c560ff7efffce14f95aca279  
100644
--- a/scm/markup.scm
+++ b/scm/markup.scm
@@ -79,29 +79,38 @@ following stencil.  Stencils with empty Y extent are  
not given
 (define markup-commands-to-ignore
   '(page-ref-markup))

+(define (all-relevant-markup-commands)
+  ;; Returns a list containing the names of all markup-commands and
+  ;; markup-list-commands with predicate @code{cheap-markup?} or
+  ;; @code{markup-list?} in their @code{markup-command-signature}.
+  ;; It needs to be a procedure, because before it is called in
+  ;; @code{markup->string}, not all bindings in the lily-module are done  
and we
+  ;; want to catch user-defined markup-commands from @code{current-module}
+  ;; as well.
+  ;; @code{table-of-contents} is not caught, though.
+  ;; Markup-commands from @code{markup-commands-to-ignore} are removed.
+  (lset-difference eq?
+    (map car
+      (filter
+        (lambda (x)
+          (let* ((predicates (markup-command-signature (cdr x))))
+            (and predicates
+                 (not
+                   (null?
+                     (lset-intersection eq?
+                       '(cheap-markup? markup-list?)
+                       (map procedure-name predicates)))))))
+        ;; TODO
+        ;;  - other modules to look at?
+        ;;  - need to care about conflicts/duplicates?
+        (append
+          (ly:module->alist (current-module))
+          (ly:module->alist (resolve-module '(lily))))))
+    markup-commands-to-ignore))
+
 (define-public (markup->string m . argscopes)
   (let* ((scopes (if (pair? argscopes) (car argscopes) '())))

-    (define all-relevant-markup-commands
-      ;; Returns a list containing the names of all markup-commands and
-      ;; markup-list-commands with predicate @code{cheap-markup?} or
-      ;; @code{markup-list?} in their @code{markup-command-signature}.
-      ;; @code{table-of-contents} is not caught, same for user-defined  
commands.
-      ;; markup-commands from @code{markup-commands-to-ignore} are removed.
-      (lset-difference eq?
-        (map car
-          (filter
-            (lambda (x)
-              (let* ((predicates (markup-command-signature (cdr x))))
-                (and predicates
-                     (not
-                       (null?
-                         (lset-intersection eq?
-                           '(cheap-markup? markup-list?)
-                           (map procedure-name predicates)))))))
-            (ly:module->alist (resolve-module '(lily)))))
-        markup-commands-to-ignore))
-
     ;; helper functions to handle string cons like string lists
     (define (markup-cons->string-cons c scopes)
       (if (not (pair? c)) (markup->string c scopes)
@@ -113,8 +122,7 @@ following stencil.  Stencils with empty Y extent are  
not given
           (string-join (list (car c) (string-cons-join (cdr c))) "")))

     ;; We let the following line in for future debugging
-    ;; (display-scheme-music (sort all-relevant-markup-commands symbol<?))
-
+    ;; (display-scheme-music (sort (all-relevant-markup-commands)  
symbol<?))

     ;;;; Remark: below only works, if markup?- or markup-list? arguments  
are the
     ;;;;         last listed arguments in the commands definition
@@ -158,7 +166,7 @@ following stencil.  Stencils with empty Y extent are  
not given
         (markup->string (ly:modules-lookup scopes var) (cons mod scopes))))

      ((member (car m)
-              (primitive-eval (cons 'list all-relevant-markup-commands)))
+              (primitive-eval (cons 'list (all-relevant-markup-commands))))
       (markup->string
         (if (> (length (last-pair m)) 1)
             (last-pair m)