Remove debugging cruft, also other changes in patch-set one not needed
since patchset two.
Add handler to handle conditions thrown from markup module code in
lily.scm.
http://codereview.appspot.com/5464045/diff/8002/input/regression/markup-cyclic-reference.ly
File input/regression/markup-cyclic-reference.ly (right):
http://codereview.appspot.com/5464045/diff/8002/input/regression/markup-cyclic-reference.ly#newcode2
input/regression/markup-cyclic-reference.ly:2: #(use-modules (scm
markup-facility-defs))
On 2011/12/21 18:32:50, dak wrote:
Is this required?
Not with latest patch-set, will remove
http://codereview.appspot.com/5464045/diff/8002/ly/toc-init.ly
File ly/toc-init.ly (right):
http://codereview.appspot.com/5464045/diff/8002/ly/toc-init.ly#newcode4
ly/toc-init.ly:4: %% scheme code now defined in (scm
markup-facility-defs)
On 2011/12/21 18:32:50, dak wrote:
Can you explain why it was necessary to move the stuff in here to
markup-facility-defs? It looks like the kind of thing that should
continue
working.
Reverted and removed corresponding changes in markup-facility-defs.scm
http://codereview.appspot.com/5464045/diff/8002/scm/define-event-classes.scm
File scm/define-event-classes.scm (right):
http://codereview.appspot.com/5464045/diff/8002/scm/define-event-classes.scm#newcode22
scm/define-event-classes.scm:22: (if (not (defined?
'ly:is-listened-event-class)) (use-modules (lily)) )
On 2011/12/21 18:32:50, dak wrote:
Uh, why the conditional here? Isn't it normal to use-modules whenever
one needs
something defined in a different module, and let use-modules cater for
loading
the stuff if not already done?
Left-over cruft from debugging. Removing.
http://codereview.appspot.com/5464045/diff/8002/scm/define-markup-commands.scm
File scm/define-markup-commands.scm (right):
http://codereview.appspot.com/5464045/diff/8002/scm/define-markup-commands.scm#newcode370
scm/define-markup-commands.scm:370: x laboris nisi ut aliquip ex ea
commodo consequat.
On 2011/12/21 18:32:50, dak wrote:
What's with the x?
x est res icognita. . .
OK it's a typo, will remove.
http://codereview.appspot.com/5464045/diff/8002/scm/lily.scm
File scm/lily.scm (right):
http://codereview.appspot.com/5464045/diff/8002/scm/lily.scm#newcode829
scm/lily.scm:829: (set! failed (append (list failed-file) failed)))))
Needs to be modified to handle conditions thrown from markup module.
(See also comment below)
http://codereview.appspot.com/5464045/diff/8002/scm/lily.scm#newcode873
scm/lily.scm:873: (lambda (x . args) (handler x file-name))))
Enhance to handle conditions thrown from markup module, also
handler assignment above in (let* block in lilypond-all
http://codereview.appspot.com/5464045/diff/8002/scm/markup-facility-defs.scm
File scm/markup-facility-defs.scm (right):
http://codereview.appspot.com/5464045/diff/8002/scm/markup-facility-defs.scm#newcode17
scm/markup-facility-defs.scm:17: (define-public lilypond-module
(current-module))
To be removed, along with mirroring line at the bottom.
http://codereview.appspot.com/5464045/diff/8002/scm/markup-facility-defs.scm#newcode34
scm/markup-facility-defs.scm:34: markup* )
On 2011/12/21 18:32:50, dak wrote:
markup*?
It's cruft. Will lose it.
http://codereview.appspot.com/5464045/diff/8002/scm/markup-facility-defs.scm#newcode94
scm/markup-facility-defs.scm:94: Use `markup*' in a \\notemode context."
On 2011/12/21 18:32:50, dak wrote:
Why are you reintroducing markup*?
It's cruft. Will lose it.
http://codereview.appspot.com/5464045/diff/8002/scm/markup-facility-defs.scm#newcode169
scm/markup-facility-defs.scm:169: (ly:debug "Defined ~s function in
~s\n" ,(symbol->string command-name) (current-module))
On 2011/12/21 18:32:50, dak wrote:
Is there a reason to insert this debug output including the current
module?
Would like to leave this and similar until patch for Issue 1686 is done.
http://codereview.appspot.com/5464045/diff/8002/scm/markup-facility-defs.scm#newcode198
scm/markup-facility-defs.scm:198: (ly:debug "Defined ~s function in
~s\n" ,(symbol->string make-markup-name) (current-module))
On 2011/12/21 18:32:50, dak wrote:
Same here.
See above
http://codereview.appspot.com/5464045/diff/8002/scm/markup-facility-defs.scm#newcode392
scm/markup-facility-defs.scm:392: (defmacro*-public markup* (#:rest
body)
On 2011/12/21 18:32:50, dak wrote:
Again: is there a reason you reintroduce markup*?
It's cruft. Will lose it.
http://codereview.appspot.com/5464045/diff/8002/scm/markup-facility-defs.scm#newcode413
scm/markup-facility-defs.scm:413: ;; (eval-when (load compile eval) ;
Guile V2 only
On 2011/12/21 18:32:50, dak wrote:
Is there a reason you keep this line in after commenting it out? It
does not
seem to convey useful information.
It's a TODO for me - will comment it as such, see matching comment
below.
http://codereview.appspot.com/5464045/diff/8002/scm/markup-facility-defs.scm#newcode424
scm/markup-facility-defs.scm:424: (ly:message "Current module: ~s"
(current-module))
On 2011/12/21 18:32:50, dak wrote:
You throw a signal here that I don't see being caught anywhere.
Thinko. It's caught in the version of lily.scm I was using on my Guile
V2 system. Will ensure relevant handling code is added to ly:load in
lily.scm in next patch-set.
http://codereview.appspot.com/5464045/diff/8002/scm/markup-facility-defs.scm#newcode458
scm/markup-facility-defs.scm:458: ;;) ; Guile V2 only
On 2011/12/21 18:32:50, dak wrote:
What is Guile V2 only here?
Part of the same TODO as at the top. Going to have to do something
like:
(define (compile-markup-expression-aux <current-args>)
<current compile-markup-expression code>)
(define (compile-markup-expression <current args>)
(cond-expand
(guile-2
(eval-when (compile load eval)
(compile-markup-expression-aux <current-args>)))
(guile
(compile-markup-expression-aux <current-args>)))
http://codereview.appspot.com/5464045/diff/8002/scm/markup-facility-defs.scm#newcode690
scm/markup-facility-defs.scm:690: (set-current-module lilypond-module)
On 2011/12/21 18:32:50, dak wrote:
Is this line necessary? What happens with it when loading, what when
compiling?
Makes me nervous.
It was transitional while the file was still being loaded by lily.scm.
Should be redundant now lily.scm loads this via (use-modules) will
remove this and matching line 17.
http://codereview.appspot.com/5464045/
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
