Re: New attempt on tuplets/percent repeats

[Han-Wen Nienhuys]

Erik Sandberg schreef:
> Hi,
>
> Here's a patch that massively refactors tuplets and percent repeats.
>
> Highlights:
> - tuplets are now signalled by start/stop events.

This patch looks almost ready for inclusion. Can you address my 
tuplet-event nit, verify that a clean make web still work, and commit?

The tuplet-engraver looks good, but I would still like the events to be 
generated in the Tuplet_iterator (which you may rename from 
time-scaled-music-iterator), so the music representation is analogous to 
the

 \times 2/3

syntax.

Keeping the music representation and syntax as closely related as 
possible makes things easier, both for me (understanding the whole 
thing), Nicolas (maintaining display-music) and you (when you want to 
separate the parser and tree generation).

In general, I think it makes sense to move parser substitutions as much 
as possible to the iterators, eg. at some point, we should also change 
the implementation of mmrests, \clef and \time.

Nicolas, does that make sense to you too?

> - each percent/slash is signalled by an individual event, whose
> repeat-count property tells whether it should be numbered (this moves
> some decisions from engraver to iterator)

Huh, this doesn't correspond with your code, where countPercentRepeats 
is still the determining factor? I'd like to leave countPercentRepeats 
as is, btw.

> The diffs of some files are huge, so I'm sending the new files instead.
>
> I think it's a bit out of place to merge percent_repeat and
> double_percent_repeat into one engraver. 

yes, I agree.

> Some next steps that would be
> nice are:
> - move handling of double percent repeats to slash_engraver (or
> possibly to a new double_percent_engraver, but imho they are similar
> enough to share engraver)

Mabye even make a brand new engraver? WE have a lot of precomputations 
that make adding another engraver rather cheap.

> - handle percent_repeat with start/stop event
> - create different event types for percent event, slash event and
> double percent event.


Oh, there's another thing: it would also be nice to link back the 
synthetic events to the input, by setting their origins to the origins 
of the music expression. This is neccessary to provide sensible error 
messages.


>                 "DoublePercentRepeatCounter",


nitpick: add a ' ' before the final " , and sort alphabetically. The 
same for all similar lists of properties.

>     Music *percent = make_music_by_name (ly_symbol2scm ("PercentEvent"));
>     percent->set_spot (*mus->origin ());
>     percent->set_property ("length", length);
>     if (repeats > 1)
>       percent->set_property ("repeat-count", scm_int2num (i - 1));
>     Music *percent_chord = make_music_by_name (ly_symbol2scm ("EventChord"));
>     percent_chord->set_spot (*mus->origin ());
>     percent_chord->set_property ("elements", scm_list_1 (percent->self_scm ()));
>     child_list_ = scm_cons (percent_chord->self_scm (), child_list_);


can you double check that you're not leaking the percent event? I think 
it should be unprotect()ed somewhere. The same for any other synthetic 
event that you create.

--

Han-Wen Nienhuys - address@hidden - http://www.xs4all.nl/~hanwen

LilyPond Software Design
 -- Code for Music Notation
http://www.lilypond-design.com