Implement define-event-function

Implement define-event-function

[dak, 2011-09-21 19:10:47]

This allows defining music functions that can be used as directionless events, a
frequently made request.  It may be noted that the amount of code needed for
implementing this functionality is not exactly staggering given the current
infrastructure in lexer and parser.

Example:

dyn=
#(define-event-function (parser location arg) (markup?)
  (make-dynamic-script arg))

{ c1\dyn pffff }

[Reinhold, 2011-09-21 19:17:09]

I haven't looked at the code itself, but a regtest is definitely missing from
the patch.

[dak, 2011-09-21 19:26:58]

reinhold.kainhofer@gmail.com writes:

> I haven't looked at the code itself, but a regtest is definitely missing
> from the patch.
>
> http://codereview.appspot.com/5083045/

Writing regtests is one of my least favorite occupations.  This feature
has been requested so often that I'd appreciate somebody else to
volunteer.

-- 
David Kastrup

[reinhold_kainhofer.com, 2011-09-21 19:40:37]

Am Wednesday, 21. September 2011, 21:26:54 schrieb David Kastrup:
> reinhold.kainhofer@gmail.com writes:
> > I haven't looked at the code itself, but a regtest is definitely missing
> > from the patch.
> > 
> > http://codereview.appspot.com/5083045/
> 
> Writing regtests is one of my least favorite occupations.

Huh? How do you verify that your feature works at all? I'm sure you are using 
some simple test file for this. So, simply take that file, add a \header { 
doctitle="some short description" } and you're done.

Cheers,
Reinhold

-- 
------------------------------------------------------------------
Reinhold Kainhofer, reinhold@kainhofer.com, http://reinhold.kainhofer.com/
 * Financial & Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

[Neil Puttock, 2011-09-21 19:44:46]

On 2011/09/21 19:40:37, reinhold_kainhofer.com wrote:

> Huh? How do you verify that your feature works at all? I'm sure you are using 
> some simple test file for this. So, simply take that file, add a \header { 
> doctitle="some short description" } and you're done.

The example David's posted above doesn't work:

/home/neil/lilypond/out/share/lilypond/current/scm/ly-syntax-constructors.scm:50:9:
In expression (pred m):
/home/neil/lilypond/out/share/lilypond/current/scm/ly-syntax-constructors.scm:50:9:
Wrong type to apply: #f

Cheers,
Neil

[dak, 2011-09-21 21:14:17]

On 2011/09/21 19:44:46, Neil Puttock wrote:
> On 2011/09/21 19:40:37, http://reinhold_kainhofer.com wrote:
> 
> > Huh? How do you verify that your feature works at all? I'm sure you are
using 
> > some simple test file for this. So, simply take that file, add a \header { 
> > doctitle="some short description" } and you're done.
> 
> The example David's posted above doesn't work:
> 
>
/home/neil/lilypond/out/share/lilypond/current/scm/ly-syntax-constructors.scm:50:9:
> In expression (pred m):
>
/home/neil/lilypond/out/share/lilypond/current/scm/ly-syntax-constructors.scm:50:9:
> Wrong type to apply: #f

Sigh.  Looks like I mixed up branches/binaries again while testing: this one
requires the patch for optional music arguments to be applied first.  I'll see
whether I can factor out the relevant functionality into a separate patch and
commit it to master.

I apologize for the confusion.

[Neil Puttock, 2011-09-21 21:32:29]

http://codereview.appspot.com/5083045/diff/1/scm/c++.scm
File scm/c++.scm (right):

http://codereview.appspot.com/5083045/diff/1/scm/c++.scm#newcode38
scm/c++.scm:38: (define-public (event? x)
I'd prefer a less vague name for this since it's going to conflict with the
stream event predicate (currently ly:stream-event?) when we get round to
renaming Stream_event -> Event

[dak, 2011-09-21 22:14:40]

http://codereview.appspot.com/5083045/diff/1/scm/c++.scm
File scm/c++.scm (right):

http://codereview.appspot.com/5083045/diff/1/scm/c++.scm#newcode38
scm/c++.scm:38: (define-public (event? x)
On 2011/09/21 21:32:30, Neil Puttock wrote:
> I'd prefer a less vague name for this since it's going to conflict with the
> stream event predicate (currently ly:stream-event?) when we get round to
> renaming Stream_event -> Event

Hm.  It corresponds to 'event being in types.  Since parser.yy also checks this
property in C, it might make sense implementing it in C and offering ly:event?
for calling it instead.

Since ly:music? is valid for all music objects having 'music in types, and not
just those of class Music, I can't quite see a good reason for preferring
ly:event? or event? to point to an Event music class.

[dak, 2011-09-21 22:57:46]

http://codereview.appspot.com/5083045/diff/1/scm/c++.scm
File scm/c++.scm (right):

http://codereview.appspot.com/5083045/diff/1/scm/c++.scm#newcode38
scm/c++.scm:38: (define-public (event? x)
On 2011/09/21 22:14:40, dak wrote:
> On 2011/09/21 21:32:30, Neil Puttock wrote:
> > I'd prefer a less vague name for this since it's going to conflict with the
> > stream event predicate (currently ly:stream-event?) when we get round to
> > renaming Stream_event -> Event
> 
> Hm.  It corresponds to 'event being in types.  Since parser.yy also checks
this
> property in C, it might make sense implementing it in C and offering ly:event?
> for calling it instead.
> 
> Since ly:music? is valid for all music objects having 'music in types, and not
> just those of class Music, I can't quite see a good reason for preferring
> ly:event? or event? to point to an Event music class.

Hm[2].  In scm/define-music-display-methods.scm (of all files) there is a
definition post-event? apparently intended for something like this.  More or
less: it checks for a number of classes explicitly instead of looking at types. 
But that's not what lily/parser.yy does for deciding whether to interpret a
Scheme value as MUSIC_IDENTIFIER or EVENT_IDENTIFIER.

[dak, 2011-09-22 00:28:38]

Ok, I have made this based off origin/master, and I moved the stuff to using
ly:event? instead of event? (not really addressing Neil's issue at all, merely
for somewhat more symmetry to ly:music?).

Untested.

[dak, 2011-09-22 01:00:53]

If this can be verified as working, it might go on the patch countdown, barring
comments that require addressing.

Note that ly:event? is not part of the public API of define-event-function: if
ly:event?'s function name changes at some later point of time, uses of
define-event-function will not need to adapt.

[dak, 2011-09-22 09:37:33]

James, could you put this through the ringer?  It's on patch countdown and I
don't really trust my regtesting abilities all too much.

Thanks.

[pkx166h, 2011-09-22 11:32:30]

David,

On Thu, Sep 22, 2011 at 10:37 AM,  <dak@gnu.org> wrote:
> James, could you put this through the ringer?  It's on patch countdown
> and I don't really trust my regtesting abilities all too much.
>
> Thanks.
>
> http://codereview.appspot.com/5083045/

No problem, I'll update the tracker to Patch-New (as that's my trigger
to test the patch) and then if it's ok I'll set it back and update the
Rietveld as well.

I won't be able to do this until this evening (BST).




-- 
--
James

[pkx166h, 2011-09-22 18:59:29]

Passes make and reg tests

[dak, 2011-09-24 22:26:24]

Pushed as 5aac8e8d8e2b1fc490354cba2cdecd204e9d5006