Fixes issue 40.

Fixes issue 40.

[MikeSol, 2011-08-06 20:17:59]

Here's a fix for Issue 40.

Cheers,
MS

[pkx166h, 2011-08-06 20:51:01]

Pass make and reg tests

[Carl, 2011-08-08 15:19:06]

Can we have a regtest, please?

[MikeSol, 2011-08-08 18:52:58]

On 2011/08/08 15:19:06, Carl wrote:
> Can we have a regtest, please?

Regtest added.

Cheers,
MS

[Carl, 2011-08-08 20:39:21]

LGTM.

Thanks,

Carl

[hanwenn, 2011-08-09 05:08:56]

LGTM

note that image of the issue will also require a minimum distance setting,
otherwise, the glissando will be shortened into a dot?

[MikeSol, 2011-08-09 06:41:24]

On 2011/08/09 05:08:56, hanwenn wrote:
> LGTM
> 
> note that image of the issue will also require a minimum distance setting,
> otherwise, the glissando will be shortened into a dot?

Done.  New patchset uploaded.

Cheers,
MS

[Neil Puttock, 2011-08-09 17:11:21]

On 9 August 2011 07:41,  <mtsolo@gmail.com> wrote:
> On 2011/08/09 05:08:56, hanwenn wrote:
>>
>> LGTM
>
>> note that image of the issue will also require a minimum distance
>
> setting,
>>
>> otherwise, the glissando will be shortened into a dot?
>
> Done.  New patchset uploaded.

This doesn't ensure the glissando's visible (and messes up two
tablature regtests).  Han-Wen probably means minimum-length (which
requires a springs-and-rods setting); alternatively, you could set
ragged-right = ##f.

Cheers,
Neil

[Neil Puttock, 2011-08-09 17:15:23]

http://codereview.appspot.com/4801083/diff/8001/input/regression/glissando-ac...
File input/regression/glissando-accidental.ly (right):

http://codereview.appspot.com/4801083/diff/8001/input/regression/glissando-ac...
input/regression/glissando-accidental.ly:8: a1 \glissando cis
a1\glissando

http://codereview.appspot.com/4801083/diff/8001/lily/line-spanner.cc
File lily/line-spanner.cc (right):

http://codereview.appspot.com/4801083/diff/8001/lily/line-spanner.cc#newcode113
lily/line-spanner.cc:113: Grob *acc = Note_column::accidentals
(bound_grob->get_parent (X_AXIS));
acc is an AccidentalPlacement, so should be renamed to avoid confusion.

As an AccidentalPlacement, it won't work very well with chords that have
accidentals far enough away from the head the glissando ends on, or chord
glissandos where some heads don't have accidentals:

\relative c, {
  \clef bass
  \override Glissando #'minimum-length = #6
  \override Glissando #'springs-and-rods = #ly:spanner::set-spacing-rods
  <cis gis' dis'>8[\glissando <d! a' e'>]  
}

http://codereview.appspot.com/4801083/diff/8001/lily/line-spanner.cc#newcode380
lily/line-spanner.cc:380: "minimum-distance "
The docstring for this will need changing.

[mike_apollinemike.com, 2011-08-09 18:23:42]

On Aug 9, 2011, at 7:15 PM, n.puttock@gmail.com wrote:

>
http://codereview.appspot.com/4801083/diff/8001/lily/line-spanner.cc#newcode113
> lily/line-spanner.cc:113: Grob *acc = Note_column::accidentals
> (bound_grob->get_parent (X_AXIS));
> acc is an AccidentalPlacement, so should be renamed to avoid confusion.
> 
> As an AccidentalPlacement, it won't work very well with chords that have
> accidentals far enough away from the head the glissando ends on, or
> chord glissandos where some heads don't have accidentals:
> 
> \relative c, {
>  \clef bass
>  \override Glissando #'minimum-length = #6
>  \override Glissando #'springs-and-rods = #ly:spanner::set-spacing-rods
>  <cis gis' dis'>8[\glissando <d! a' e'>]
> }
> 

Good call - I've uploaded a new patch that uses "accidental-grob" (don't know
why I didn't think of that before...too much code on the brain!).
I've taken out all of that minimum-distance stuff.  The way the regtest is
designed, it does not reproduce the snippet in Issue 40 to the letter but it
reproduces it in spirit (I think) - it uses whole notes so that the glissando is
visible.

Cheers,
MS

[Neil Puttock, 2011-08-09 20:04:36]

On 2011/08/09 18:23:42, mike_apollinemike.com wrote:

> Good call - I've uploaded a new patch that uses "accidental-grob" (don't know
> why I didn't think of that before...too much code on the brain!).

The latest patch set needs rebasing against master.

Cheers,
Neil

[mike_apollinemike.com, 2011-08-09 20:50:57]

On Aug 9, 2011, at 10:04 PM, n.puttock@gmail.com wrote:

> On 2011/08/09 18:23:42, mike_apollinemike.com wrote:
> 
>> Good call - I've uploaded a new patch that uses "accidental-grob"
> (don't know
>> why I didn't think of that before...too much code on the brain!).
> 
> The latest patch set needs rebasing against master.
> 
> Cheers,
> Neil
> 
> http://codereview.appspot.com/4801083/

Done.

Cheers,
MS

[Neil Puttock, 2011-08-09 22:08:37]

LGTM.

http://codereview.appspot.com/4801083/diff/14002/scm/define-grobs.scm
File scm/define-grobs.scm (right):

http://codereview.appspot.com/4801083/diff/14002/scm/define-grobs.scm#newcode933
scm/define-grobs.scm:933: (end-on-accidental . #t)
trailing whitespace

[MikeSol, 2011-08-22 06:39:39]

Pushed as b3c0b773bde05b3971f087ed6c5f4fb40a1b20b1.