I've had to do some major mangling with my local git rep, so the next patch-set is coming up as a new Rietveld issue.
It's http://codereview.appspot.com/5504107 Will also update Google tracker. Ian http://codereview.appspot.com/5464045/diff/10002/ly/music-functions-init.ly File ly/music-functions-init.ly (right): http://codereview.appspot.com/5464045/diff/10002/ly/music-functions-init.ly#newcode31 ly/music-functions-init.ly:31: %% need (scm markup-facility-defs)for markup? On 2012/01/02 14:04:39, dak wrote:
Wasn't this supposed to get removed? This was not supposed to be
needed in
user-level docs, right?
No. This isn't a user-level document, it's part of the run-time initialization we do for each user document. Regression tests will break without it. User document testing.ly with this (copy of a definition from music-functions-init.ly): \version "2.14.0" myFootnoteGrob = #(define-music-function (parser location grob-name offset text footnote) (symbol? number-pair? markup? markup?) (_i "Attach @var{text} to @var{grob-name} at offset @var{offset}, with @var{text} referring to @var{footnote} (use like @code{\\once})") (make-music 'FootnoteEvent 'automatically-numbered #f 'symbol grob-name 'X-offset (car offset) 'Y-offset (cdr offset) 'text text 'footnote-text footnote)) \relative c' { \time 4/4 c4 d e f | g a b c \bar "|." } Still compiles OK. http://codereview.appspot.com/5464045/diff/10002/ly/titling-init.ly File ly/titling-init.ly (right): http://codereview.appspot.com/5464045/diff/10002/ly/titling-init.ly#newcode1 ly/titling-init.ly:1: \version "2.15.20" On 2012/01/02 14:04:39, dak wrote:
Isn't the version string wrong here?
Done. http://codereview.appspot.com/5464045/diff/10002/scm/define-event-classes.scm File scm/define-event-classes.scm (right): http://codereview.appspot.com/5464045/diff/10002/scm/define-event-classes.scm#newcode20 scm/define-event-classes.scm:20: (ice-9 optargs) On 2012/01/02 14:04:39, dak wrote:
Why is optargs needed here? There is a define* below, but that looks
like it
should rather be plain define.
Done. http://codereview.appspot.com/5464045/diff/10002/scm/define-event-classes.scm#newcode165 scm/define-event-classes.scm:165: (define* (simplify e) Will change to (define http://codereview.appspot.com/5464045/diff/10002/scm/framework-ps.scm File scm/framework-ps.scm (right): http://codereview.appspot.com/5464045/diff/10002/scm/framework-ps.scm#newcode20 scm/framework-ps.scm:20: #:duplicates (replace)) On 2012/01/02 14:04:39, dak wrote:
What is with the duplicate? Should this be fixed?
Some modules occasionally need to re-import (lily) via a (use-modules (lily)). The #:duplicates (replace) stops guile throwing out warnings during initialization, particularly about us over-riding the guile base interface definition for (format). This works round that. If you feel the whole issue about modules needing to re-import (lily) needs looking at, lets look at it as a separate issue. http://codereview.appspot.com/5464045/diff/10002/scm/framework-ps.scm#newcode429 scm/framework-ps.scm:429: (fancy-format port "/~a (~a)\n" field (metadata-encode (markup->string val)))))) On 2012/01/02 14:04:39, dak wrote:
Why would we need fancy-format here? The format codes are usual?
Another weirdo. Also reverted on my local repository. Maybe git-cl upload isn't doing what I think it does. http://codereview.appspot.com/5464045/diff/10002/scm/lily.scm File scm/lily.scm (right): http://codereview.appspot.com/5464045/diff/10002/scm/lily.scm#newcode47 scm/lily.scm:47: (define-public lilypond-module (resolve-module '(lily))) ;; should be equivalent to (current-module) for Guile V1.8 On 2012/01/02 14:04:39, dak wrote:
What is this equivalent to in GuileV2? Or rather: what module are we
in if not
in lily?
For Guile V2, eventually we will want to do $guild compile --output=/scm/out/lily.go -L /scm lily.scm in the build. As it won't be able to run up the Lily image to create the (lily) module in the main.cc code, (current-module) will be set to an "anonymous" module by the guile code within its compile-file function. So I can't just do (define-public lilypond-module (current-module)) http://codereview.appspot.com/5464045/diff/10002/scm/markup-facility-defs.scm File scm/markup-facility-defs.scm (right): http://codereview.appspot.com/5464045/diff/10002/scm/markup-facility-defs.scm#newcode92 scm/markup-facility-defs.scm:92: Use `markup*' in a \\notemode context." On 2012/01/02 14:04:39, dak wrote:
You are still not using the current definition of markup as your
source here,
correct? The doc string refers to markup*.
Ouch! Nice catch. http://codereview.appspot.com/5464045/diff/10002/scm/markup.scm File scm/markup.scm (right): http://codereview.appspot.com/5464045/diff/10002/scm/markup.scm#newcode56 scm/markup.scm:56: ;; Use `markup*' in a \\notemode context." On 2012/01/02 14:04:39, dak wrote:
The outcommented parts are not from the current source. Why are they outcommented instead of removed?
OK. Cleanup time. http://codereview.appspot.com/5464045/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel