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

[Ian Hulin]

Hi Neil,

On 09/08/10 22:34, address@hidden wrote:
> On 2010/08/09 20:01:10, Ian Hulin wrote:
>
> > Presumably this means make a parallel change to
> > Documentation/snippets/clip-systems.ly? OK.
>
> snippets/new/clip-systems.ly
>
> then run makelsr.py to update snippets/clip-systems.ly:
>
> scripts/auxiliar/makelsr.py

OK, you've convinced me. Changing the reg test is just nasty.

> > 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.
>
> Why would you want to add something to a file, only to remove it soon
> after?
>
> If we agree it's better from the user's perspective not to require
>
> (use-modules (scm clip-region))
>
> in a .ly file, then it doesn't make sense to defer the change.

Well, it seemed like a distinct and separate task and like it was
getting this patch 'off-topic'.

> > 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."
>
> Sure, but `mod' here is created by ly_make_module, which now uses
> module-export-all!, so all defines will automatically be public.
>
> > Because it doesn't work.
>
> I meant: leave it in lily.scm, even if you still want to use
> (use-modules (scm clip-region)) in clip-systems.ly.

OK.

> > 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)).
>
> I don't get any regtest failure without this code.

That's because I was talking rubbish and trying to answer your review in
a hurry.  I was confusing the crash with profile-property-access.ly
which I'd compiled separately as a test
.
> There's no format
> call inside define-grob-properties.scm, and as I said earlier, the
> handler doesn't work since there's no exception thrown specifically for
> `format' (+ Patrick's comments and the incorrect args to the handler).
>
> Try adding this to profile-property-access.ly:
>
> #(define format ergonomic-simple-format)
>
> You'll see the exception handler doesn't catch this.  If you change the
> code, it gets caught:

Errm, unless you're using an unpatched, vanilla lily.scm from git you
won't see it because the handler will already be defined in
ergonomic-simple-format.  Using the code as in the my patch certainly
stopped profile-property-access.ly from barfing here. I'll poke around
further with this.

> (define (simple-format-handler dest . rest)
>   (if (string? dest)
>       (apply fancy-format (cons #f (cons dest rest)))
>       (apply fancy-format (cons dest rest))))
>
> (define-public (ergonomic-simple-format dest . rest)
>   "Like ice-9 format, but without the memory consumption."
>   (catch #t
>      (lambda () (if (string? dest)
>             (apply simple-format (cons #f (cons dest rest)))
>             (apply simple-format (cons dest rest))))
>      (lambda (key . args) (apply simple-format-handler (cons dest rest)))))
See above.
> http://www.codereview.appspot.com/1160044/show

Cheers,
Ian