Issue 4650070: Adds a warning for non-list fingeringOrientations settings. (Closed)
|
Created:
12 years, 10 months ago by MikeSol Modified:
12 years, 10 months ago CC:
lilypond-devel_gnu.org Visibility:
Public. |
DescriptionAdds a warning for non-list fingeringOrientations settings.
Patch Set 1 #
MessagesTotal messages: 12
Fixes issue 1734.
Sign in to reply to this message.
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).
Sign in to reply to this message.
On Jul 4, 2011, at 2:47 PM, n.puttock@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
Sign in to reply to this message.
On 4 July 2011 13:53, mike@apollinemike.com <mike@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
Sign in to reply to this message.
On 7/4/11 7:14 AM, "Neil Puttock" <n.puttock@gmail.com> wrote: > On 4 July 2011 13:53, mike@apollinemike.com <mike@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
Sign in to reply to this message.
On 4 July 2011 15:31, Carl Sorensen <c_sorensen@byu.edu> 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
Sign in to reply to this message.
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
Sign in to reply to this message.
On 4 July 2011 21:33, <lemniskata.bernoullego@gmail.com> wrote: > Where is set_property defined? lily/include/lily-guile-macros.hh The actual type-checking occurs in Context::internal_set_property (). Cheers, Neil
Sign in to reply to this message.
On Jul 4, 2011, at 4:39 PM, Neil Puttock wrote: > On 4 July 2011 15:31, Carl Sorensen <c_sorensen@byu.edu> 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
Sign in to reply to this message.
On 5 July 2011 08:26, mike@apollinemike.com <mike@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
Sign in to reply to this message.
On Jul 5, 2011, at 9:20 PM, Neil Puttock wrote: > On 5 July 2011 08:26, mike@apollinemike.com <mike@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
Sign in to reply to this message.
On 5 July 2011 21:26, mike@apollinemike.com <mike@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
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
