Re: reopened Issue 2584: please make partcombine merge slurs (issue 6432047)

[dak]

On 2012/07/19 17:52:13, Keith wrote:
> On 2012/07/19 17:25:45, dak wrote:
> > On 2012/07/19 17:12:21, Keith wrote:
> > > Again, I suggest first committing the code in a state that
> > > merely fixes the reported bugs about warnings,

> >  One could remove line 264 in slur.cc

> Simpler to finish the loop over j at line 227, similarly to the
ver2.14 code,

That's not equivalent.

> with

>    if (j < old_slurs)
>      ev->origin ()->warning (_ ("already have slur"));
>    start_events_.erase (start_events_.begin () + i);
>    break;
> }

> > and there is no point in first making the less convenient decision.

> The point is to separate the fix to one regression (1967) from code
that causes
> another regression (2584).

But there is no code in the current patch that would fix one and not the
other.  You are asking for artificially designing something doing only
half the job, and fitting it in between.

> Linked strings of regressions make it /look/ more
> difficult than it really is to fall back to a state free of known
regressions.

But there is nothing won by designing and writing artificially
half-broken code that exists for no other purpose than being only
half-broken.  For the current code design, there is no version that
would be a logical intermediate step, fixing only one regression, and
using the original code in a code path causing the other regression to
be unfixed.

Your proposal does not even make sense without committing a sequence of
commits starting with _reverting_ the original fix, then doing a version
changing the warning, then committing the state after the original fix
_again_, then committing the current fix.

If you really want that, create an issue with all those steps except the
last (resulting in a net patch changing nothing when all steps are
combined) and block this issue on it.  Once your issue passes, you can
commit the sequence of patches (making sure that each of those has the
intended state), leaving the work tree in the same state, and then I can
commit the final patch of this issue on top.

I consider that fabulously pointless, but if it is important to you,
that would be the way to proceed.

http://codereview.appspot.com/6432047/