lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: T1055: Avoid using deprecated %module-public-interface in guile init


From: n . puttock
Subject: Re: T1055: Avoid using deprecated %module-public-interface in guile initialisation. (issue1160044)
Date: Mon, 09 Aug 2010 19:08:57 +0000

Hi Ian,

Can you remove the tab->space changes you've made (particularly in
lily.scm)?  Some of them don't improve the indentation.

Cheers,
Neil


http://www.codereview.appspot.com/1160044/diff/51001/52001
File input/regression/clip-systems.ly (right):

http://www.codereview.appspot.com/1160044/diff/51001/52001#newcode47
input/regression/clip-systems.ly:47: #(use-modules (scm clip-region))
This is fine for a regression test, but not very user-friendly (don't
forget you'd also have to add it to the docs snippet from LSR, otherwise
a docs build will fail.)

Rather than make clipping systems even more esoteric, I'd be happier
moving the rhythmic-location code out of clip-region.scm and into
output-lib.scm or music-functions.scm.

http://www.codereview.appspot.com/1160044/diff/51001/52002
File lily/lily-lexer.cc (right):

http://www.codereview.appspot.com/1160044/diff/51001/52002#newcode303
lily/lily-lexer.cc:303: scm_c_export (symstr.c_str(), NULL);
Can you give an example justifying this addition?

I can't think of any identifier set in a .ly file which would need this.

http://www.codereview.appspot.com/1160044/diff/51001/52003
File lily/ly-module.cc (right):

http://www.codereview.appspot.com/1160044/diff/51001/52003#newcode36
lily/ly-module.cc:36: SCM maker =
ly_lily_module_constant("make-module");
ly_lily_module_constant (

http://www.codereview.appspot.com/1160044/diff/51001/52003#newcode46
lily/ly-module.cc:46: Guile V1.9/2.0
Is it?

http://www.codereview.appspot.com/1160044/diff/51001/52004
File scm/define-grob-properties.scm (right):

http://www.codereview.appspot.com/1160044/diff/51001/52004#newcode19
scm/define-grob-properties.scm:19: (use-modules (scm clip-region))
Why not leave this in lily.scm?

http://www.codereview.appspot.com/1160044/diff/51001/52005
File scm/lily.scm (right):

http://www.codereview.appspot.com/1160044/diff/51001/52005#newcode219
scm/lily.scm:219: (catch 'format
On 2010/08/09 18:35:54, Patrick McCarty wrote:
Is this change supposed to be part of the patchset?

I don't think it should be. :)

Also, I don't understand the logic here.  Is 'format ever thrown?  If
so, which
procedure does this?

There's no exception for 'format, so it's never thrown.  I think this
would only work if it were changed to

(catch #t

http://www.codereview.appspot.com/1160044/diff/51001/52005#newcode223
scm/lily.scm:223: (lambda (key dest . rest) (simple-format-handler dest
. rest))))
Should be

(lambda (key . args) (apply simple-format-handler (cons dest rest)))

http://www.codereview.appspot.com/1160044/diff/51001/52005#newcode258
scm/lily.scm:258: (define-public  (guile-v2 )
(define-public (guile-v2)

http://www.codereview.appspot.com/1160044/show



reply via email to

[Prev in Thread] Current Thread [Next in Thread]