[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Checks to see if tuplet brackets have bounds (issue 13582046)
From: |
k-ohara5a5a |
Subject: |
Re: Checks to see if tuplet brackets have bounds (issue 13582046) |
Date: |
Fri, 13 Sep 2013 20:12:35 +0000 |
The first two patches look good.
Defensive checks for null pointers are a good thing.
We do not want a bracket for the input in the bug-report.
We want a message when our .ly input asks for a tuplet bracket that
cannot be printed, and we get such a message (just before a crash,
without this patch).
Maybe "warning" would be more appropriate than "programming error" as
the message in tuplet-engraver.cc, since LilyPond allows both the input
in the bug report and this variant where we want, and get, a bracket:
\new Voice << {
%%% \once\override Voice.TupletBracket #'cross-staff = ##f
\times 2/3 { s8 s8 s8 } r2. }
{ b8*2/3 b b } >>
If there are no bounds to attach the bracket to, it would make sense to
skip the creation of the TupletBracket with a continue; statement after
the error message in tuplet-engraver.cc
https://codereview.appspot.com/13582046/diff/11001/lily/tuplet-bracket.cc
File lily/tuplet-bracket.cc (right):
https://codereview.appspot.com/13582046/diff/11001/lily/tuplet-bracket.cc#newcode809
lily/tuplet-bracket.cc:809:
Why check the bounds here?
We certainly don't want to check break_status_dir() here.
Conceptually, the (line-broken) tuplet /could/ end on a line-break, but
still be cross-staff.
In fact, the first time cross-staff is called is now before
line-breaking, so break_status_dir() is 0, and the value of cross-staff
is stored in the property so it need not be called again.
Now that there is no more pure-relevant filter, adjacent_pure_heights()
should probably use the get_pure_property() method of checking
cross-staff, omitting object for which cross-staff is true or
non-boolean (a.k.a. "don't know yet").
https://codereview.appspot.com/13582046/