Re: Move New_dynamic_engraver over the unused Dynamic_engraver (issue 14460043)

[dak]

On 2013/10/06 23:00:25, thomasmorley651 wrote:
> Although, I can't review C++, I've applied the patch for testing
(hopefully
> without mistake)

> Testing this code

> { c''1_\mf^\> \break d''_\mp^\! }

> I've got:

> programming error: Spanner `Hairpin' is not fully contained in parent
spanner.
> Ignoring orphaned part
> { c''1_\mf
>            ^\> \break d''_\mp^\! }

> Same error as in 2.17.27.
> Any chance to make it work?

> And what is "parent spanner" here?
> The old dynamic-engraver seemed to collect Hairpins/DynamicText in the
> DynamicLineSpanner (I suppose this was the former "parent spanner"),
does the
> new one different?

> At least this coding prints the same with and without your patch:

> {
>    \override DynamicLineSpanner #'after-line-breaking =
>      #(lambda (grob)
>         (display (ly:grob-object grob 'elements)))

>    c''1_\mf^\> d''_\mp^\!
> }

> Or am I completely wrong and this patch has nothing to do with the
problem
> above?

In this case, you are completely wrong.  LilyPond currently has a
"Dynamic_engraver" in its code base that has not been used by LilyPond
itself since 2.11.something.  It may or may not work differently with
this problem: no idea.  But the code has not been exercised in all that
time, including rather large reorganizations in 2.15 and 2.17.  So I
decided to throw out that old code and rename the "New_dynamic_engraver"
which has been used all that time into "Dynamic_engraver".

That's all.  No change in behavior or bugs is expected here: it's just a
removal of dead code.  I wasted some time trying out things in
lily/dynamic-engraver.cc before noticing that the engraver is not
actually used anywhere.

https://codereview.appspot.com/14460043/