Re: Add new merge-function.ly file for rests (issue4005046)

[pkx166h]

Reviewers: carl.d.sorensen_gmail.com, Graham Percival,

Message:
I have done all the changes (except added a reg test) and uploaded them.
However this does not make cleanly. I am getting an error

current/scm/lily.scm:851:21: Unbound variable:
merge-rests-on-positioning

Any help would be appreciated as I can't see any formatting errors.


http://codereview.appspot.com/4005046/diff/1/ly/declarations-init.ly
File ly/declarations-init.ly (right):

http://codereview.appspot.com/4005046/diff/1/ly/declarations-init.ly#newcode31
ly/declarations-init.ly:31: \include "merge-function.ly"
On 2011/01/22 23:29:24, Carl wrote:
> Once we move ly/merge-function.ly to scm/merge-rests.scm, this will
need to move
> to scm/lily.scm as part of the definition of init-scheme-files (see
lines 393
> and thereabouts.

Done.

http://codereview.appspot.com/4005046/diff/1/ly/merge-function.ly
File ly/merge-function.ly (right):

http://codereview.appspot.com/4005046/diff/1/ly/merge-function.ly#newcode1
ly/merge-function.ly:1: %{
On 2011/01/22 23:29:24, Carl wrote:
> Once you have moved the commands out to ly/property-init.ly and
> ly/engraver-init.ly, there's nothing but scheme code in this file, so
it should
> become a scheme file.

> Let's call it scm/merge-rests.scm.

> Put a LilyPond copyright statement at the top of it (use one from any
other .scm
> file), with Wilbert Berendsen and James Lowe as the copyright holders.

Done.

http://codereview.appspot.com/4005046/diff/1/ly/merge-function.ly#newcode8
ly/merge-function.ly:8: \include "merge-rests"
On 2011/01/23 00:07:33, Graham Percival wrote:
> The whole point of this work is so that you _won't_ need to \include
> merge-rests, so I'd omit this part.  :)

Done.

http://codereview.appspot.com/4005046/diff/1/ly/merge-function.ly#newcode118
ly/merge-function.ly:118: mergeRestsOn = {
On 2011/01/22 23:29:24, Carl wrote:
> You have properly defined mergeRestsOn and mergeRestsOff in
ly/property-init.ly,
> so they should be removed here.

> mergeRests should be moved to ly/engraver-init.ly (and removed here).

Done.

http://codereview.appspot.com/4005046/diff/1/ly/property-init.ly
File ly/property-init.ly (right):

http://codereview.appspot.com/4005046/diff/1/ly/property-init.ly#newcode282
ly/property-init.ly:282: mergeRests = \layout {
On 2011/01/22 23:29:24, Carl wrote:
> mergeRests should have the \layout{} block removed, so that it
> can be included in anybody's layout block.

> It should be in ly/engraver-init.ly.


Done.

Description:
Add new merge-function.ly file for rests

Taken from Tracker issue 1228

Function to merge rests that occur, for example, in two voices at
the same moment so that only a single rest is printed.

Added a new .ly file and included the functions in declarations-init.ly
and property-init.ly

This may not be the best way to implement this but I am hoping this is
a start at least.

Please review this at http://codereview.appspot.com/4005046/

Affected files:
  M ly/engraver-init.ly
  M ly/property-init.ly
  M scm/lily.scm
  A scm/merge-rests.scm