Re: Adds a warning for non-list fingeringOrientations settings. (issue4650070)
On Jul 5, 2011, at 9:20 PM, Neil Puttock wrote: > On 5 July 2011 08:26, m...@apollinemike.com wrote: > >> Just to get the ball rolling on this, were I to start on a patch that >> implements this sort of settings checking, where would be a good place to >> start? >> I know where the context mods are set and where the properties are set, but >> I don't know how the layout block escapes this checking. > > Context::internal_set_property (). > > Cheers, > Neil Thanks Neil! I should have been clearer before: what I don't understand is not the function call (pardon the double negative), but rather how the layout block evades setting do_internal_type_checking_global and/or how layout blocks are excepted in the function type_check_assignment. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a warning for non-list fingeringOrientations settings. (issue4650070)
On 5 July 2011 21:26, m...@apollinemike.com wrote: > Thanks Neil! I should have been clearer before: what I don't understand is > not the function call (pardon the double negative), but rather how the layout > block evades setting do_internal_type_checking_global and/or how layout > blocks are excepted in the function type_check_assignment. If -dcheck-internal-types is set, do_internal_type_checking_global is set, so the type-checking is applied to all \layout blocks. I'm not sure how you can ensure there's no checking for internal .ly files (i.e., what the lexer sets as new input before user files via `init.ly'). Cheers, Neil ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a warning for non-list fingeringOrientations settings. (issue4650070)
On 5 July 2011 08:26, m...@apollinemike.com wrote: > Just to get the ball rolling on this, were I to start on a patch that > implements this sort of settings checking, where would be a good place to > start? > I know where the context mods are set and where the properties are set, but I > don't know how the layout block escapes this checking. Context::internal_set_property (). Cheers, Neil ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a warning for non-list fingeringOrientations settings. (issue4650070)
On Jul 4, 2011, at 4:39 PM, Neil Puttock wrote: > On 4 July 2011 15:31, Carl Sorensen wrote: >> Would a redundant check of settings from default context definitions be a >> problem? I can't imagine that such a check would take 1% of the processing >> time. > > I don't know, though I agree is unlikely to be a significant overhead. > >> Plus, I don't think it's really a redundant check; I think it's a real >> check. Absent such a check, we're trusting on the *-init.ly files being >> correct, which admits a potential programming error. > > The *-init.ly files are covered by regression testing since > -dcheck-internal-types triggers an assertion error for incorrect > context property settings. > > Cheers, > Neil > Just to get the ball rolling on this, were I to start on a patch that implements this sort of settings checking, where would be a good place to start? I know where the context mods are set and where the properties are set, but I don't know how the layout block escapes this checking. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a warning for non-list fingeringOrientations settings. (issue4650070)
On 4 July 2011 21:33, wrote: > Where is set_property defined? lily/include/lily-guile-macros.hh The actual type-checking occurs in Context::internal_set_property (). Cheers, Neil ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a warning for non-list fingeringOrientations settings. (issue4650070)
On 2011/07/04 13:14:55, Neil Puttock wrote: Context_def::add_context_mod () is where the assignment takes place (and you can see from set_property () how the type-checking is done). Where is set_property defined? cheers, Janek http://codereview.appspot.com/4650070/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a warning for non-list fingeringOrientations settings. (issue4650070)
On Jul 4, 2011, at 5:04 PM, Reinhold Kainhofer wrote: > Am Montag, 4. Juli 2011, 16:39:08 schrieb Neil Puttock: >> On 4 July 2011 15:31, Carl Sorensen wrote: >>> Plus, I don't think it's really a redundant check; I think it's a real >>> check. Absent such a check, we're trusting on the *-init.ly files being >>> correct, which admits a potential programming error. >> >> The *-init.ly files are covered by regression testing since >> -dcheck-internal-types triggers an assertion error for incorrect >> context property settings. > > Is there any possibility to install those checks only after all internal init > files have been processed? > Alternatively, there could be a lazy_internal_set_property method in context.cc that has a scheme binding accessed in the init files. To make sure regtests work, we could make it so that this is still responsive to the assert error for type checking. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a warning for non-list fingeringOrientations settings. (issue4650070)
Am Montag, 4. Juli 2011, 16:39:08 schrieb Neil Puttock: > On 4 July 2011 15:31, Carl Sorensen wrote: > > Plus, I don't think it's really a redundant check; I think it's a real > > check. Absent such a check, we're trusting on the *-init.ly files being > > correct, which admits a potential programming error. > > The *-init.ly files are covered by regression testing since > -dcheck-internal-types triggers an assertion error for incorrect > context property settings. Is there any possibility to install those checks only after all internal init files have been processed? Cheers, Reinhold -- -- Reinhold Kainhofer, reinh...@kainhofer.com, http://reinhold.kainhofer.com/ * Financial & Actuarial Math., Vienna Univ. of Technology, Austria * http://www.fam.tuwien.ac.at/, DVR: 0005886 * LilyPond, Music typesetting, http://www.lilypond.org ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a warning for non-list fingeringOrientations settings. (issue4650070)
On 4 July 2011 15:31, Carl Sorensen wrote: > Would a redundant check of settings from default context definitions be a > problem? I can't imagine that such a check would take 1% of the processing > time. I don't know, though I agree is unlikely to be a significant overhead. > Plus, I don't think it's really a redundant check; I think it's a real > check. Absent such a check, we're trusting on the *-init.ly files being > correct, which admits a potential programming error. The *-init.ly files are covered by regression testing since -dcheck-internal-types triggers an assertion error for incorrect context property settings. Cheers, Neil ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a warning for non-list fingeringOrientations settings. (issue4650070)
On 7/4/11 7:14 AM, "Neil Puttock" wrote: > On 4 July 2011 13:53, m...@apollinemike.com wrote: > >> I didn't realize this was the real issue :) >> Any tips as to how one would go about fixing this? Anything that happens >> before engravers kick in (dispatchers, parsers, etc.) remains a mystery to >> me... > > Context_def::add_context_mod () is where the assignment takes place > (and you can see from set_property () how the type-checking is done). > I suppose though this is deliberate, otherwise every compilation would > redundantly type-check settings from default context definitions, > which we assume are correct (i.e., from > engraver-init.ly/performer-init.ly). Would a redundant check of settings from default context definitions be a problem? I can't imagine that such a check would take 1% of the processing time. Plus, I don't think it's really a redundant check; I think it's a real check. Absent such a check, we're trusting on the *-init.ly files being correct, which admits a potential programming error. Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a warning for non-list fingeringOrientations settings. (issue4650070)
On 4 July 2011 13:53, m...@apollinemike.com wrote: > I didn't realize this was the real issue :) > Any tips as to how one would go about fixing this? Anything that happens > before engravers kick in (dispatchers, parsers, etc.) remains a mystery to > me... Context_def::add_context_mod () is where the assignment takes place (and you can see from set_property () how the type-checking is done). I suppose though this is deliberate, otherwise every compilation would redundantly type-check settings from default context definitions, which we assume are correct (i.e., from engraver-init.ly/performer-init.ly). Cheers, Neil ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a warning for non-list fingeringOrientations settings. (issue4650070)
On Jul 4, 2011, at 2:47 PM, n.putt...@gmail.com wrote: > On 2011/07/04 12:38:06, MikeSol wrote: >> Fixes issue 1734. > > On 2011/07/04 12:38:06, MikeSol wrote: >> Fixes issue 1734. > > I think this covers up the real problem: context settings in a \layout > block have no type check. I didn't realize this was the real issue :) Any tips as to how one would go about fixing this? Anything that happens before engravers kick in (dispatchers, parsers, etc.) remains a mystery to me... Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a warning for non-list fingeringOrientations settings. (issue4650070)
On 2011/07/04 12:38:06, MikeSol wrote: Fixes issue 1734. On 2011/07/04 12:38:06, MikeSol wrote: Fixes issue 1734. I think this covers up the real problem: context settings in a \layout block have no type check. Your addition simply duplicates Guile's error message for fingeringOrientations set in music (and doesn't cover stringNumberOrientations or strokeFingerOrientations). http://codereview.appspot.com/4650070/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Adds a warning for non-list fingeringOrientations settings. (issue4650070)
Reviewers: ,
Message:
Fixes issue 1734.
Description:
Adds a warning for non-list fingeringOrientations settings.
Please review this at http://codereview.appspot.com/4650070/
Affected files:
M lily/new-fingering-engraver.cc
Index: lily/new-fingering-engraver.cc
diff --git a/lily/new-fingering-engraver.cc b/lily/new-fingering-engraver.cc
index
db99dede95b2979324c3d7858911928bb67ac069..9909790b1d72e0c368f91837c078c4e6af4029bf
100644
--- a/lily/new-fingering-engraver.cc
+++ b/lily/new-fingering-engraver.cc
@@ -178,6 +178,11 @@ void
New_fingering_engraver::position_scripts (SCM orientations,
vector *scripts)
{
+ if (scm_list_p (orientations) != SCM_BOOL_T)
+{
+ warning ("fingeringOrientations must be a list. Setting to '(up
down).");
+ orientations = scm_list_2 (ly_symbol2scm ("up"), ly_symbol2scm
("down"));
+}
for (vsize i = 0; i < scripts->size (); i++)
if (stem_ && to_boolean (scripts->at (i).script_->get_property
("add-stem-support")))
Side_position_interface::add_support (scripts->at (i).script_,
stem_);
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
