Re: Let \autochange accept optional arguments for the turning-point and clefs (issue 259080043 by address@hidden)

[thomasmorley65]

Thanks Dan and David for review


https://codereview.appspot.com/259080043/diff/20001/Documentation/notation/keyboards.itely
File Documentation/notation/keyboards.itely (right):

https://codereview.appspot.com/259080043/diff/20001/Documentation/notation/keyboards.itely#newcode294
Documentation/notation/keyboards.itely:294: If the staves are not
instantiated explicitely other clefs may be used.
On 2015/07/31 00:01:19, Dan Eble wrote:
> explicitly,
>            ^
> (correct spelling and insert comma)

Done.

https://codereview.appspot.com/259080043/diff/20001/input/regression/auto-change.ly
File input/regression/auto-change.ly (right):

https://codereview.appspot.com/259080043/diff/20001/input/regression/auto-change.ly#newcode10
input/regression/auto-change.ly:10: If the staves are not instantiated
explicitely, other clefs may be specified.
On 2015/07/31 00:01:19, Dan Eble wrote:
> IMO the new cases covering \with {} deserve to be in a separate file.
I
> probably would have made a separate file for the case covering the
specified
> switch point too.

Done.

https://codereview.appspot.com/259080043/diff/20001/ly/music-functions-init.ly
File ly/music-functions-init.ly (right):

https://codereview.appspot.com/259080043/diff/20001/ly/music-functions-init.ly#newcode189
ly/music-functions-init.ly:189: (ly:context-mod? #{ \with {
On 2015/07/31 05:12:05, dak wrote:
> Write (ly:context-mod?) here and (or clef-1 #{ \with { \clef "treble"
} #}) in
> the code.  I'd not recommend doing the same for #{ c' #} since you
cannot
> predict the active notename language at the time of execution.

Done.

https://codereview.appspot.com/259080043/diff/20001/ly/music-functions-init.ly#newcode204
ly/music-functions-init.ly:204: Setting clefs works only for not
explicitely instantiated staves.")
On 2015/07/31 00:01:19, Dan Eble wrote:
> not explicitly -> implicitly

Done.

https://codereview.appspot.com/259080043/diff/20001/ly/music-functions-init.ly#newcode210
ly/music-functions-init.ly:210: \context Staff = "up" $clef-1
On 2015/07/31 00:01:19, Dan Eble wrote:
> I don't understand why $ rather than #.  Is it my problem, or does it
deserve a
> comment?

Well, it does not work with #. I'm not sure enough about the reasons to
even try an explanation.

https://codereview.appspot.com/259080043/diff/20001/scm/autochange.scm
File scm/autochange.scm (right):

https://codereview.appspot.com/259080043/diff/20001/scm/autochange.scm#newcode36
scm/autochange.scm:36: (cond ((ly:pitch<? ref-pitch pitch)
On 2015/07/31 00:01:20, Dan Eble wrote:
> would ly:pitch-diff simplify this?

I was not happy with this nested if/cond either, but I don't see how
ly:pitch-diff would help.

At least I managed to shorten it a bit in the newest patch-set

https://codereview.appspot.com/259080043/