Fix critical regressions to lyric spacing

Fix critical regressions to lyric spacing

Cleaned up documentation

Add snippet that shows how to get old spacing behavior if desired

Add old spacing snippet to Docs; run makelsr.py

Update changes.tely

Modify vocal templates

Run convert-ly on everything

[Carl, 2011-01-23 04:14:01]

Here is a potential patch for fixing the lyric spacing regressions.

It updates the documentation to clarify what is needed to make lyrics behave as
desired.

It also adds a snippet that demonstrates how to achieve the old spacing
behavior.

Please respond with any comments.

Thanks,

Carl

[Trevor Daniels, 2011-01-23 09:09:09]

LGTM

I think the templates would be improved with
system-system-spacing #'basic-distance = #20
added to \paper.  Otherwise the bass lyrics will be too
close to the soprano lyrics on the following system.

Trevor

[Graham Percival (old account), 2011-01-23 14:20:35]

I think we have a convert-ly problem.

http://codereview.appspot.com/4061043/diff/1/Documentation/snippets/new/lyric...
File Documentation/snippets/new/lyrics-old-spacing-settings.ly (right):

http://codereview.appspot.com/4061043/diff/1/Documentation/snippets/new/lyric...
Documentation/snippets/new/lyrics-old-spacing-settings.ly:19: % VERSE ONE
This differs from our normal indentation style... and in particular, it
conflicts with the two-space indentation style that you used above in the global
section... but meh, go ahead.

We'll have an auto-ly-indenter soon, so let's not fuss with this manually.

http://codereview.appspot.com/4061043/diff/1/input/regression/baerenreiter-sa...
File input/regression/baerenreiter-sarabande.ly (right):

http://codereview.appspot.com/4061043/diff/1/input/regression/baerenreiter-sa...
input/regression/baerenreiter-sarabande.ly:174: obsolete-between-system-space =
25\mm  system-system-spacing #'basic-distance = #(/
obsolete-between-system-space staff-space)  score-system-spacing
#'basic-distance = #(/ obsolete-between-system-space staff-space)
woah, what happened here?  Could we get some linebreaks? (is this some weird
osx-linebreaks+git malfunction?)

http://codereview.appspot.com/4061043/diff/1/input/regression/mozart-hrn-3.ly
File input/regression/mozart-hrn-3.ly (right):

http://codereview.appspot.com/4061043/diff/1/input/regression/mozart-hrn-3.ly...
input/regression/mozart-hrn-3.ly:51: obsolete-between-system-space = 20 \mm 
system-system-spacing #'basic-distance = #(/ obsolete-between-system-space
staff-space)  score-system-spacing #'basic-distance = #(/
obsolete-between-system-space staff-space)
ditto.  hmm, maybe Keith's recent convert-ly patch has a problem?

http://codereview.appspot.com/4061043/diff/1/input/regression/page-spacing.ly
File input/regression/page-spacing.ly (right):

http://codereview.appspot.com/4061043/diff/1/input/regression/page-spacing.ly...
input/regression/page-spacing.ly:71: obsolete-between-system-space = 1.0 
system-system-spacing #'basic-distance = #(/ obsolete-between-system-space
staff-space)  score-system-spacing #'basic-distance = #(/
obsolete-between-system-space staff-space)
ditto.

http://codereview.appspot.com/4061043/diff/1/input/regression/page-top-space.ly
File input/regression/page-top-space.ly (right):

http://codereview.appspot.com/4061043/diff/1/input/regression/page-top-space....
input/regression/page-top-space.ly:25: obsolete-page-top-space = 3 \cm 
top-system-spacing #'basic-distance = #(/ obsolete-page-top-space staff-space)
ditto.

[Neil Puttock, 2011-01-23 22:55:54]

http://codereview.appspot.com/4061043/diff/1/Documentation/changes.tely
File Documentation/changes.tely (right):

http://codereview.appspot.com/4061043/diff/1/Documentation/changes.tely#newco...
Documentation/changes.tely:70: Lyrics above a staff must have their
@code{staff-affinity} set to
I don't think this is appropriate here.

If it must stay, it needs a snippet to show how 'staff-affinity and
alignAboveContext are used.

http://codereview.appspot.com/4061043/diff/1/Documentation/snippets/new/vocal...
File
Documentation/snippets/new/vocal-ensemble-template-with-automatic-piano-reduction.ly
(right):

http://codereview.appspot.com/4061043/diff/1/Documentation/snippets/new/vocal...
Documentation/snippets/new/vocal-ensemble-template-with-automatic-piano-reduction.ly:6:
%% Translation of GIT committish: fa19277d20f8ab0397c560eb0e7b814bd804ecec
remove all translation stuff

http://codereview.appspot.com/4061043/diff/1/Documentation/snippets/new/vocal...
File Documentation/snippets/new/vocal-ensemble-template.ly (right):

http://codereview.appspot.com/4061043/diff/1/Documentation/snippets/new/vocal...
Documentation/snippets/new/vocal-ensemble-template.ly:6: %% Translation of GIT
committish: fa19277d20f8ab0397c560eb0e7b814bd804ecec
remove

http://codereview.appspot.com/4061043/diff/1/input/regression/mozart-hrn-3.ly
File input/regression/mozart-hrn-3.ly (right):

http://codereview.appspot.com/4061043/diff/1/input/regression/mozart-hrn-3.ly...
input/regression/mozart-hrn-3.ly:51: obsolete-between-system-space = 20 \mm 
system-system-spacing #'basic-distance = #(/ obsolete-between-system-space
staff-space)  score-system-spacing #'basic-distance = #(/
obsolete-between-system-space staff-space)
On 2011/01/23 14:20:35, Graham Percival wrote:
> ditto.  hmm, maybe Keith's recent convert-ly patch has a problem?

It's deliberate, in case user scores have commented-out paper settings: if these
are converted, the obsolete-* variables are undefined, causing Guile to
complain.

[Carl, 2011-01-23 23:43:07]

On 2011/01/23 09:09:09, Trevor Daniels wrote:
> I think the templates would be improved with
> system-system-spacing #'basic-distance = #20
> added to \paper.  Otherwise the bass lyrics will be too
> close to the soprano lyrics on the following system.

Done.  I also added top-system-spacing and last-bottom-spacing to take care of
the first and last systems of the score.

[Carl, 2011-01-23 23:49:26]

I've responded to the comments in detail below.

Graham, relative to your comments on issues 1483 and 1486, both are addressed in
this patch.

The templates now have the lyrics attached properly to the staves, as do the
examples in the NR.  This takes care of 1483.

The templates also have sufficient space for the lyrics at the top and bottom of
the score.  This takes care of 1486.

For good measure, there is a snippet in the docs that demonstrates the way to
get the same spacing behavior as in 2.12.

As far as I can see, all of the problems associated with both issues are
resolved by this patch.

http://codereview.appspot.com/4061043/diff/1/Documentation/changes.tely
File Documentation/changes.tely (right):

http://codereview.appspot.com/4061043/diff/1/Documentation/changes.tely#newco...
Documentation/changes.tely:70: Lyrics above a staff must have their
@code{staff-affinity} set to
I think it is appropriate, because people who used the old vocal templates will
suddenly find that the soprano lyrics will be attached to the previous system,
and the tenor lyrics will be attached to the womens' staff.  One rational place
for them to go to investigate is to look at CHANGES.

What if, instead of a snippet, it referenced the appropriate section in the
Notation Reference?

I've gone ahead and added the reference; I'll change if you think it necessary.

http://codereview.appspot.com/4061043/diff/1/Documentation/notation/vocal.itely
File Documentation/notation/vocal.itely (right):

http://codereview.appspot.com/4061043/diff/1/Documentation/notation/vocal.ite...
Documentation/notation/vocal.itely:1021: % lyrics above a stave should have this
override
Changed stave to staff

http://codereview.appspot.com/4061043/diff/1/Documentation/snippets/new/lyric...
File Documentation/snippets/new/lyrics-old-spacing-settings.ly (right):

http://codereview.appspot.com/4061043/diff/1/Documentation/snippets/new/lyric...
Documentation/snippets/new/lyrics-old-spacing-settings.ly:19: % VERSE ONE
On 2011/01/23 14:20:35, Graham Percival wrote:
> This differs from our normal indentation style... and in particular, it
> conflicts with the two-space indentation style that you used above in the
global
> section... but meh, go ahead.
> 

No, I'll fix it.  This is what happens when I copy somebody else's code and
don't review it carefully.  I made sure it worked, and that we had the right
header stuff, but didn't pay a lot of attention to the coding style.

Thanks for the catch.

I've fixed the indentation, along with removing the unnecessary { s1 } from each
\new Lyrics construct.

http://codereview.appspot.com/4061043/diff/1/Documentation/snippets/new/vocal...
File
Documentation/snippets/new/vocal-ensemble-template-with-automatic-piano-reduction.ly
(right):

http://codereview.appspot.com/4061043/diff/1/Documentation/snippets/new/vocal...
Documentation/snippets/new/vocal-ensemble-template-with-automatic-piano-reduction.ly:6:
%% Translation of GIT committish: fa19277d20f8ab0397c560eb0e7b814bd804ecec
On 2011/01/23 22:55:54, Neil Puttock wrote:
> remove all translation stuff

Done.

http://codereview.appspot.com/4061043/diff/1/Documentation/snippets/new/vocal...
File Documentation/snippets/new/vocal-ensemble-template.ly (right):

http://codereview.appspot.com/4061043/diff/1/Documentation/snippets/new/vocal...
Documentation/snippets/new/vocal-ensemble-template.ly:6: %% Translation of GIT
committish: fa19277d20f8ab0397c560eb0e7b814bd804ecec
On 2011/01/23 22:55:54, Neil Puttock wrote:
> remove

Done.

http://codereview.appspot.com/4061043/diff/1/input/regression/baerenreiter-sa...
File input/regression/baerenreiter-sarabande.ly (right):

http://codereview.appspot.com/4061043/diff/1/input/regression/baerenreiter-sa...
input/regression/baerenreiter-sarabande.ly:174: obsolete-between-system-space =
25\mm  system-system-spacing #'basic-distance = #(/
obsolete-between-system-space staff-space)  score-system-spacing
#'basic-distance = #(/ obsolete-between-system-space staff-space)
On 2011/01/23 14:20:35, Graham Percival wrote:
> woah, what happened here?  Could we get some linebreaks? (is this some weird
> osx-linebreaks+git malfunction?)

No, this is exactly the way convert-ly was intended to work for this change,
because in some cases the lines were commented.

This was discussed before the convert-ly rule was pushed.

The convert-ly rule was pushed in commit
4921d3518f4961abcfaf9ea243bec33efc943574  IIUC.  The author was Keith O'Hara and
the committer was Graham Percival ;-)

But I will do a manual fix and make it the way it would be if we were writing it
from scratch.

Done.

http://codereview.appspot.com/4061043/diff/1/input/regression/mozart-hrn-3.ly
File input/regression/mozart-hrn-3.ly (right):

http://codereview.appspot.com/4061043/diff/1/input/regression/mozart-hrn-3.ly...
input/regression/mozart-hrn-3.ly:51: obsolete-between-system-space = 20 \mm 
system-system-spacing #'basic-distance = #(/ obsolete-between-system-space
staff-space)  score-system-spacing #'basic-distance = #(/
obsolete-between-system-space staff-space)
On 2011/01/23 14:20:35, Graham Percival wrote:
> ditto.  hmm, maybe Keith's recent convert-ly patch has a problem?

Done.

http://codereview.appspot.com/4061043/diff/1/input/regression/page-spacing.ly
File input/regression/page-spacing.ly (right):

http://codereview.appspot.com/4061043/diff/1/input/regression/page-spacing.ly...
input/regression/page-spacing.ly:71: obsolete-between-system-space = 1.0 
system-system-spacing #'basic-distance = #(/ obsolete-between-system-space
staff-space)  score-system-spacing #'basic-distance = #(/
obsolete-between-system-space staff-space)
On 2011/01/23 14:20:35, Graham Percival wrote:
> ditto.

Done.

http://codereview.appspot.com/4061043/diff/1/input/regression/page-top-space.ly
File input/regression/page-top-space.ly (right):

http://codereview.appspot.com/4061043/diff/1/input/regression/page-top-space....
input/regression/page-top-space.ly:25: obsolete-page-top-space = 3 \cm 
top-system-spacing #'basic-distance = #(/ obsolete-page-top-space staff-space)
On 2011/01/23 14:20:35, Graham Percival wrote:
> ditto.

Done.

[Keith, 2011-01-24 08:10:23]

Carl, having struggled myself trying to smooth the transition to the new spacing
with a snippet(s), I appreciate the simplicity of this solution.   It looks good
to me.

Having text in CHANGES seems important.
A little sugar would help the medicine go down; if spin is not your style, one
of the others of us could sweeten in a follow-up-patch.

[Keith, 2011-01-24 08:10:48]

http://codereview.appspot.com/4061043/diff/1/Documentation/changes.tely
File Documentation/changes.tely (right):

http://codereview.appspot.com/4061043/diff/1/Documentation/changes.tely#newco...
Documentation/changes.tely:460: The vertical spacing engine has been drastically
changed, making
The new note on Lyrics might fits, pedagogically, in or after this item. 
Sweetened text might go like:

Staves and lines of lyrics can also spread to use the space on the page.  By
default, lyrics stay close to the staff above, or to the staff below if they
were placed using the @code{alignAboveContext} setting.  If you use a different
method to set lyrics above the music, see the manual on how to set
@code{staff-affinity} to {#DOWN}."

[Graham Percival (old account), 2011-01-24 16:33:22]

Almost there.

http://codereview.appspot.com/4061043/diff/19001/Documentation/snippets/new/l...
File Documentation/snippets/new/lyrics-old-spacing-settings.ly (right):

http://codereview.appspot.com/4061043/diff/19001/Documentation/snippets/new/l...
Documentation/snippets/new/lyrics-old-spacing-settings.ly:1: \version "2.13.47"
Could you run makelsr and then don't forget to do
  git add Documentation/snippets/*.ly

so that git master will compile after applying this patch?  :)
I'm missing lyrics-old-spacing-settings.ly in Documentation/snippets/ .

[c_sorensen, 2011-01-24 16:42:33]

On 1/24/11 9:33 AM, "percival.music.ca@gmail.com"
<percival.music.ca@gmail.com> wrote:

> Almost there.
> 
> 
> http://codereview.appspot.com/4061043/diff/19001/Documentation/snippets/new/ly
> rics-old-spacing-settings.ly
> File Documentation/snippets/new/lyrics-old-spacing-settings.ly (right):
> 
> http://codereview.appspot.com/4061043/diff/19001/Documentation/snippets/new/ly
> rics-old-spacing-settings.ly#newcode1
> Documentation/snippets/new/lyrics-old-spacing-settings.ly:1: \version
> "2.13.47"
> Could you run makelsr and then don't forget to do
>    git add Documentation/snippets/*.ly

Ah.  I did the makelsr, but forgot to do the git add.

It compiles on my machine.....

Thanks,

Carl

[Carl, 2011-01-25 00:20:29]

On 2011/01/24 16:33:22, Graham Percival wrote:
> Almost there.

> Could you run makelsr and then don't forget to do
>   git add Documentation/snippets/*.ly


Done, and new patch set uploaded.

Thanks,

Carl

[Graham Percival (old account), 2011-01-25 13:57:32]

On 2011/01/25 00:20:29, Carl wrote:
> On 2011/01/24 16:33:22, Graham Percival wrote:
> > Almost there.
> 
> Done, and new patch set uploaded.

Compiles fine; please push.

Cheers,
- Graham

[Carl, 2011-01-25 14:27:31]

On 2011/01/25 13:57:32, Graham Percival wrote:
Pushed.

cb03a19174fd9245008176716742e1a2eb3a0b93


Thanks,

Carl