Re: T2026: Use new (scm markup-facility-defs) scheme module for markup commands. (issue 5464045)

[ianhulin44]

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/