Re: Various clean-ups in stems and beams. (issue 6584045)

[aleksandr . andreev]

On 2012/11/07 07:52:33, mike7 wrote:
> On 7 nov. 2012, at 09:50, mailto:address@hidden wrote:

> > On 2012/11/07 05:32:40, mike7 wrote:
> >> On 7 nov. 2012, at 00:50, mailto:address@hidden wrote:
> >
> >> >
> >> >
> >
> >

http://codereview.appspot.com/6584045/diff/13014/input/regression/note-head-style.ly#newcode108
> >> >> > input/regression/note-head-style.ly:108: \override
> > Staff.Dots.style
> >> > =
> >> >> > #'kievan
> >> >> > Why can't we use the new function here, e.g.,
> >> >> >
> >> >> > \kievanOn
> >> >> >
> >> >
> >> >> \kievenOn only works on the voice level and the overrides happen
on
> >> > the staff
> >> >> level.
> >> >
> >> > How about making \pattern a music function taking a context
> >> > modification as argument?  Then you could write
> >> >
> >> > \pattern \with { \override NoteHead.style = #'slash }
> >> > \pattern \with { \kievanOn }
> >> > \pattern \with { ... }
> >> >
> >> > and just pass the context mod on to the \new Voice within
\pattern.
> >> >
> >> > It seems awkward to use Staff-wide overrides here.
> >> >
> >> > By the way: it's frightening how fast one gets used to the new
> >> > override syntax.  I had to think really hard about whether this
was
> >> > supposed to be different previously or not.  And then it seems
> > strange
> >> > that there would have been no dot.
> >> >
> >
> >> I'm not exactly sure how this'd be done - if it's OK w/ you, I'll
push
> > the patch
> >> after a countdown and then you can propose a followup patch.  Sorry
-
> > I have
> >> been out of the loop for a bit and am not up-to-date on the most
> > recent syntax
> >> modifications.
> >
> > There are no really new syntax modifications required.  The problem
with
> > the "OK w/ you" part is that it is simply wrong _not_ to use the
> > \kievanOn function in this regtest and instead copy and modify its
> > contents.  That way, the regtests ceases to test \kievanOn and
depends
> > on its internals not changing.
> >
> > I'll put up a git-format patch on the Google issue tracker as
Rietveld
> > does not appear to take attachments.
> >

> Excellent - I'll copy and paste whatever you come up with into my
patch.  Thanks
> for taking the time to do this.

> Cheers,
> MS


LGTM.

http://codereview.appspot.com/6584045/