Possible fix for #372.

Possible fix for #372.

[Carl, 2010-09-18 21:54:29]

LGTM

[Carl, 2010-09-19 08:29:23]

As currently implemented, this patch fails when \partial is called in the middle
of a measure.  I'm not sure \partial *should* work when called in the middle of
a measure, but we have plenty of history on -user where it is called in the
middle of a measure.

This code:


\version "2.13.34"
{
  c''4 c''
  \partial 4
  c'' c'' c''
}

Makes a measure 5/4 long, rather than 3/4, which the old code would have done.

I've put the code that duplicates the previous behavior in the comments below.

P.S. How should one handle a code variant when doing reviews on Rietveld? 
Should we create a new issue?  Should we show alternate code in comments?

Thanks,

Carl

http://codereview.appspot.com/2228042/diff/1/lily/partial-iterator.cc
File lily/partial-iterator.cc (right):

http://codereview.appspot.com/2228042/diff/1/lily/partial-iterator.cc#newcode42
lily/partial-iterator.cc:42: ctx->set_property ("measurePosition", (now -
length).smobbed_copy ());
This code fails when \partial is called in the middle of a measure.

I did the following to get it to work:

Moment now_grace = Moment (0, ctx->now_mom ().grace_part_);
ctx->set_property ("measurePosition", (now_grace - length).smobbed_copy ());

[Neil Puttock, 2010-09-20 22:32:24]

On 2010/09/19 08:29:23, Carl wrote:
> As currently implemented, this patch fails when \partial is called in the
middle
> of a measure.  I'm not sure \partial *should* work when called in the middle
of
> a measure, but we have plenty of history on -user where it is called in the
> middle of a measure.

I see two options here:

-) keep the patch as it is, but warn the user (and do nothing)

-) warn the user, but set measurePosition as you suggest to retain the current
behaviour

I don't mind which we choose, though the former's simpler.

> I've put the code that duplicates the previous behavior in the comments below.

Thanks.

> P.S. How should one handle a code variant when doing reviews on Rietveld? 
> Should we create a new issue?  Should we show alternate code in comments?

I think creating a new issue is overkill, unless the changes are significant.

I'm happy with code changes in comments.

Cheers,
Neil

[Carl, 2010-09-25 02:12:06]

Looks very nice to me.  Thanks for taking care of this!

Carl

[Neil Puttock, 2010-09-26 00:16:14]

On 2010/09/25 02:12:06, Carl wrote:
> Looks very nice to me.  Thanks for taking care of this!

Thanks for doing the debug work on this; I don't think I'd have come up with a
fix without it.

Cheers,
Neil