Issue 2584 (redo 1967): please make partcombine merge slurs

Issue 2584 (redo 1967): please make partcombine merge slurs

The partcombiner does not really bother about keeping the number of
generated start and end slur events matched, so this attempts to cope
by implementing the following behavior:

a) multiple slur starts on the same moment are not an error but the
   same as one.
b) multiple slur ends on the same moment are not an error but the same
   as one.
a2) there will be a slur with direction UP if there is at least one
   such start event, and there will be a slur with direction DOWN if
   there is at least one such end event.  This might imply a double
   slur, but for ending it, a single slur end is sufficient.

Consequently, a^(_( c)) (the second closing paren not required, just
added for prettiness) will add a double slur.

Revert "Allow multiple identical slurs (issue 1967)"

This reverts commit b986b38f14195f31e26b0a929c8ac23a8ecfc204.

Conflicts:

	lily/slur-engraver.cc

[Keith, 2012-06-08 08:02:02]

Works good for me.

You might consider three commits
revert b986b3
fix 1967 and 1884 thoroughly
let c(_(^( e) engrave a double slur, bzw phrasing slur

[dak, 2012-06-08 08:24:55]

On 2012/06/08 08:02:02, Keith wrote:
> Works good for me.
> 
> You might consider three commits
> revert b986b3
> fix 1967 and 1884 thoroughly
> let c(_(^( e) engrave a double slur, bzw phrasing slur

Oh, 1884 was also on the loose?  I have two commits here, the first being the
revert, the second everything else.  The double slur logic is not really
separate from the rest: it is a consequence of dealing with all cases. 
Designing an artificial second commit with different behavior would require
writing the logic more or less from scratch and making a decision under which
circumstances which directions are to survive.  How to merge an undirected and a
directed slur?  If you actually check, and keep the directed slur, then it seems
awkward to drop one of conflicting directed slurs.  If you don't check and just
keep the first of several slurs, this looks like a defect artificially left in
place for a single commit.

Keeping two slurs around in the case of _(^( is not as much a planned feature
than a consequence of trying to get consistent behavior for multiple slur
starts.  If you have a combination of - _ ^ on the _same_ event (for example, by
doing -\tweak ... ^\tweak ... _ ...), the last explicit direction to be applied
to the event (so usually the first in sequence) wins.

The problem with applying this logic also here is that "last" is rather
ill-defined with regard to parallel events.  Creating a second commit means
having to make decisions and write logic implementing a choice I consciously
avoided having to make.

In contrast, the separate commit reverting the original "fix" is a no-brainer,
and indeed that is what my feature branch looks like at the moment.

[Reinhold, 2012-06-08 09:44:22]

> The partcombiner does not really bother about keeping the number of
> generated start and end slur events matched, so this attempts to cope
> by implementing the following behavior:
> 
> a) multiple slur starts on the same moment are not an error but the
>    same as one.

I actually did NOT implement it like this on purpose: If you start a slur and a
phrasing slur and forget the backslash in the phrasing slur start (which
happened to me quite frequently!), you'll have to manually find out where the
phrasing slur should start (which is typically several lines before), while on
the other hand two concurrent slurs are not supported, anyway. So to me it
seemed reasonable to warn the user about the double slur start, which typically
indicates a missing backslash for a phrasing slur.

[dak, 2012-06-08 09:57:54]

On 2012/06/08 09:44:22, Reinhold wrote:
> So to me it
> seemed reasonable to warn the user about the double slur start, which
typically
> indicates a missing backslash for a phrasing slur.

I perfectly agree.  It is just that fixing the partcombiner is quite harder, and
changing both partcombiner and slur engraver again is likely to give us another
few months of the "is this a regression? this worked differently before" game.

On the plus side, the side effect of being able to use _(^( for double slurs is
nice.  Actually (here I go again) it might be conceivable to allow _) and ^) for
matched beginning/ending of slurs beyond the simple case of double slurs,
basically as if _^ were a weak spanner id.

But I guess that this can be a consideration for another time.

[Keith, 2012-07-18 23:00:14]

http://codereview.appspot.com/6294047/diff/4/lily/slur-engraver.cc
File lily/slur-engraver.cc (right):

http://codereview.appspot.com/6294047/diff/4/lily/slur-engraver.cc#newcode220
lily/slur-engraver.cc:220: ev->origin ()->warning (_ ("already have slur"));
You could simply finish here, with
    } 
    start_events_.erase (start_events_.begin () + i);
    break;
  }
replacing everything through line 258.  Then you simply keep the direction of
first slur-event you saw.

http://codereview.appspot.com/6294047/diff/4/lily/slur-engraver.cc#newcode238
lily/slur-engraver.cc:238: // be decidedly strange for manual input.
When each of the parts being combined is a few pages long, I find I write a few
inconsistencies equivalent to

  \partcombine {c''2^( d'')} {c'2( d')}

The slurs are then silently dropped, for complicated reasons.

http://codereview.appspot.com/6294047/diff/4/lily/slur-engraver.cc#newcode246
lily/slur-engraver.cc:246: Direction slur_dir = to_dir (slurs_[j]->get_property
("direction"));
This call might wait to see the layout under the slur, including effects of
line-breaks, before reporting back.  If you understand "pure" in LilyPond, you
can use get_pure_property()