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
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
