Re: T1055: Avoid using deprecated %module-public-interface in guile initialisation. (issue1160044)

[ian]

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))
On 2010/08/09 19:08:58, Neil Puttock wrote:
> 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.)

Presumably this means make a parallel change to
Documentation/snippets/clip-systems.ly? OK.
> 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.

Valid point, but I'd rather do the work for clip-region as a separate
tracker item. Raise the tracker and I'll start work on it as soon as
this one's finished.

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);
On 2010/08/09 19:08:58, Neil Puttock wrote:
> Can you give an example justifying this addition?

> I can't think of any identifier set in a .ly file which would need
this.
It was an answer from Han-Wen about how we'd got into using
%module-public-interface in the first place:

"I think %public-interface hack is to force all of the definitions
including future ones to be public;  the code that executes the
assignments just does

      scm_module_define (mod, sym, val);  -- lily-lexer.cc

without doing anything to export sym."

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");
On 2010/08/09 19:08:58, Neil Puttock wrote:
> ly_lily_module_constant (

Done.

http://www.codereview.appspot.com/1160044/diff/51001/52003#newcode46
lily/ly-module.cc:46: Guile V1.9/2.0
On 2010/08/09 19:08:58, Neil Puttock wrote:
> Is it?

If it isn't, they forgot to forward-port "the-scm-module" as a synonym
for "the-root-module".
It's easier for us change our code base to avoid the problem than wait
for guile to change theirs.

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))
On 2010/08/09 19:08:58, Neil Puttock wrote:
> Why not leave this in lily.scm?

Because it doesn't work.
It used to pick up the definition in init.ly as a beneficial side-effect
of using %module-public-interface in ly_make_module when defining
scopes.
This change makes things cleaner all round because only things needing
this module use it and reference it specifically.
The other alternative was to dump all the definitions from the module
into lily.scm, Han-Wen said this had been on his to-do list for a while.
and favoured this approach.

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

http://www.codereview.appspot.com/1160044/diff/51001/52005#newcode215
scm/lily.scm:215: (apply fancy-format (cons dest . rest))))
On 2010/08/09 18:35:54, Patrick McCarty wrote:
> `cons' only works with two concrete arguments, like

>    (cons dest rest)
Generally it will get two arguments, but I was modeling this on the
ergonomic-simple-format procedure.
See below for the reasons for doing this, but I don't think it will get
here if rest isn't defined, as 'format only gets thrown if there's a
formatting string to have a directive in.

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?

> Also, I don't understand the logic here.  Is 'format ever thrown?  If
so, which
> procedure does this?
Yes,  If we don't have this handler, scm/define-grob-properties barfs
during initialization.  Procedure simple-format only handles a subset of
format directives, and throws 'format if it sees one it can't handle.
It kills the build and the regtests if we don't handle it here. The
other option is to change define-grob-properties to use fancy-format
(a.k.a the definition from module (ice-9 format)).

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