Re: Charles Winston's GSoC code: Chord Semantics (issue 337870043 by chazwi...@gmail.com)
Hi Charles, Today I built and ran 'make check' with your patch applied to current master. I was able to get it to pass 'make check' by making the following two changes. 1. In `chord-entry.scm` line 267, remove `(write-me "base3: " bass)`. 2. In that same file, line 100, remove the parens to change `(chord-semantics)` to just `chord-semantics`. The first change fixed this error (but note the type check warning): input/regression/chord-names-languages.ly' ~~~ Parsing... Renaming input to: `/home/james/lilypond-git/input/regression/chord-names-languages.ly' warning: type check for `bass' failed; value `(#t . #t)' must be of type `boolean' /home/james/lilypond-git/build/out/share/lilypond/current/scm/chord-entry.scm:267:59: In procedure memoization in expression (if ba ss (write-me "base3: " bass) ...): /home/james/lilypond-git/build/out/share/lilypond/current/scm/chord-entry.scm:267:59: In file "/home/james/lilypond-git/build/out/s hare/lilypond/current/scm/chord-entry.scm", line 266: Missing or extra expression in (if bass (write-me "base3: " bass) (list (make -note-ev bass (quote bass) #t)) (quote ())). ~~~ And the second change fixed this error: input/regression/chord-name-entry.ly ~~~ $ /home/dev/lilypond-git/build/out/bin/lilypond input/regression/chord-name-entry.ly GNU LilyPond 2.21.0 Processing `input/regression/chord-name-entry.ly' Parsing.../home/dev/lilypond-git/build/out/share/lilypond/current/scm/chord-entry.scm:100:35: In expression (chord-semantics): /home/dev/lilypond-git/build/out/share/lilypond/current/scm/chord-entry.scm:100:35: Wrong type to apply: ((modifier . #f) (root . #) (extension . 7) (additions) (removals) (bass . #f)) ~~~ So if you have a chance to upload a new patch set with those two changes, that should get things moving forward with the code review process. Cheers, -Paul https://codereview.appspot.com/337870043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improve markup->string (issue 347000043 by thomasmorle...@gmail.com)
I wonder whether it might be reasonable to just have all markup commands with a last argument of markup? produce their last (recursively treated) argument as default, and possibly all markup list commands with a last argument of markup-list? similarly produce their last argument? That leaves fewer commands to cater for and would also make some user-defined commands work reasonably well. https://codereview.appspot.com/34743/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Improve markup->string (issue 347000043 by thomasmorle...@gmail.com)
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/34743/
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)
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
bindings in lily-module
Hi, while working on a patch to improve markup->string from markup.scm I noticed, that (pretty-print (list? (ly:module->alist (resolve-module '(lily) inserted at toplevel in markup.scm returns several times: programming error: unbound variable in module continuing, cross fingers While doing same in a ly-files return no error. Adding some debugging code in markup.scm: (pretty-print (filter-map identity (module-map (lambda (e v) (if (variable-bound? v) #f e)) (resolve-module '(lily) The following list is returned: (make-safe-lilypond-module make-span-event midi-program percussion? remove-stencil-warnings scale-layout score-lines-markup-list score-markup span-bar::notify-grobs-of-my-existence stencil-whiteout tremolo::get-music-list volta-bracket::calc-hook-visibility write-performances-midis invalidate-alterations all-music-font-encodings alterations-in-key backend-testing base-length layout-extract-page-properties beam-exceptions lilypond-main beat-structure line-markup calc-repeat-slash-count lookup-font default-time-signature-settings) I guess at the time it is called not all bindings are done in the lily-module as opposed to caling it far later in a ly-file. Is this correct? I can cope with it, just wanted to be sure. Thanks, Harm ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
