Re: Implements multiple-line non-cross-staff glissandi (issue4527086)

[Colin Campbell]

On 11-06-27 01:16 AM, address@hidden wrote:
> http://codereview.appspot.com/4527086/diff/17001/input/regression/glissando-broken-multiple.ly 
>
> File input/regression/glissando-broken-multiple.ly (right):
>
> http://codereview.appspot.com/4527086/diff/17001/input/regression/glissando-broken-multiple.ly#newcode7 
>
> input/regression/glissando-broken-multiple.ly:7: ragged-right = ##T }
> On 2011/06/26 17:01:36, Neil Puttock wrote:
> > ##t
> > }
>
> Done.
>
> http://codereview.appspot.com/4527086/diff/17001/input/regression/glissando-broken-multiple.ly#newcode10 
>
> input/regression/glissando-broken-multiple.ly:10: \override Glissando
> #'breakable = ##t
> On 2011/06/26 17:01:36, Neil Puttock wrote:
> > you probably also want after-line-breaking = ##t to show the last
> glissando
>
> Done.
>
> http://codereview.appspot.com/4527086/diff/17001/input/regression/glissando-broken-multiple.ly#newcode11 
>
> input/regression/glissando-broken-multiple.ly:11: d1 \glissando |
> On 2011/06/26 17:01:36, Neil Puttock wrote:
> > d1\glissando
>
> Done.
>
> http://codereview.appspot.com/4527086/diff/17001/input/regression/glissando-broken-multiple.ly#newcode15 
>
> input/regression/glissando-broken-multiple.ly:15: c,1^\ff\trill
> On 2011/06/26 17:01:36, Neil Puttock wrote:
> > is the dynamic relevant?
>
> Done.
>
> http://codereview.appspot.com/4527086/diff/17001/scm/output-lib.scm
> File scm/output-lib.scm (right):
>
> http://codereview.appspot.com/4527086/diff/17001/scm/output-lib.scm#newcode798 
>
> scm/output-lib.scm:798: (define-public (glissando::before-line-breaking
> grob)
> On 2011/06/26 17:01:36, Neil Puttock wrote:
> > use assoc-set! to simplify this (i.e., just mutate bound-details
> instead of
> > rebuilding it)
>
> Done.
>
> http://codereview.appspot.com/4527086/diff/17001/scm/output-lib.scm#newcode809 
>
> scm/output-lib.scm:809: (ly:grob-set-property! grob 'bound-details bd)))
> On 2011/06/26 17:01:36, Neil Puttock wrote:
> > (set! (ly:grob-property grob 'bound-details)
>
> Done.
>
> http://codereview.appspot.com/4527086/diff/17001/scm/output-lib.scm#newcode811 
>
> scm/output-lib.scm:811: (define-public
> (glissando::internal-before-line-breaking grob dir)
> On 2011/06/26 17:01:36, Neil Puttock wrote:
> > define inside callback as helper function
>
> Done.
>
> http://codereview.appspot.com/4527086/diff/17001/scm/output-lib.scm#newcode823 
>
> scm/output-lib.scm:823: (extra-dy (assoc-get 'extra-dy details 0.0))
> On 2011/06/26 17:01:36, Neil Puttock wrote:
> > extra-dy isn't a nested property
>
> Done.
>
> http://codereview.appspot.com/4527086/diff/17001/scm/output-lib.scm#newcode826 
>
> scm/output-lib.scm:826: (y (+ (interval-center (ly:grob-extent bound
> common-y Y))
> On 2011/06/26 17:01:36, Neil Puttock wrote:
> > this overwrites user override for 'Y
>
> Fixed to allow for user Y offset.
>
> http://codereview.appspot.com/4527086/diff/17001/scm/output-lib.scm#newcode829 
>
> scm/output-lib.scm:829: (cons (cons 'Y y) details)))
> On 2011/06/26 17:01:36, Neil Puttock wrote:
> > (acons 'Y y details)
>
> Done.
>
> http://codereview.appspot.com/4527086/



Would you (Mike and Neil) like me to re-start the countdown? Otherwise, 
Neil, if you're happy with Mike's changes, this can be pushed, marked 
closed on Reitveld and fixed on the tracker.

Thanks, Mike and Neil

Colin

--
The human race has one really effective weapon, and that is laughter.
-- Mark Twain