[frogs] Re: Tracker 836: Add facility to change output file-name for a \book block

[n.puttock@xxxxxxxxx]

Hi Ian,

I haven't commented on lily-library.scm, since there are too many
formatting issues.

I'd suggest installing emacs to sort out the indentation (even if you
prefer to use another editor for your main work), otherwise we're going
to spend ages pointing out every little nitpick when we should be
focusing on the code.

Cheers,
Neil


http://codereview.appspot.com/143055/diff/10/1004
File lily/parser.yy (left):

http://codereview.appspot.com/143055/diff/10/1004#oldcode659
Line 659: }
I've always found the formatting in parser.yy rather strange, but it's
consistent, so you should revert this change.

http://codereview.appspot.com/143055/diff/10/1004
File lily/parser.yy (right):

http://codereview.appspot.com/143055/diff/10/1004#newcode676
Line 676: PARSER->lexer_->set_identifier (ly_symbol2scm
("book-output-suffix"), SCM_BOOL_F);
trailing space

http://codereview.appspot.com/143055/diff/10/1004#newcode677
Line 677: PARSER->lexer_->set_identifier (ly_symbol2scm
("book-filename"), SCM_BOOL_F);
trailing space

http://codereview.appspot.com/143055/diff/10/1005
File ly/init.ly (right):

http://codereview.appspot.com/143055/diff/10/1005#newcode14
Line 14: #(define toplevel-bookparts (list))
trailing space

http://codereview.appspot.com/143055/diff/10/1006
File ly/music-functions-init.ly (left):

http://codereview.appspot.com/143055/diff/10/1006#oldcode182
Line 182:
restore

These spaces were added when the file was sorted alphabetically.

http://codereview.appspot.com/143055/diff/10/1006#oldcode183
Line 183:
restore

http://codereview.appspot.com/143055/diff/10/1006#oldcode591
Line 591: pitchedTrill =
I'm afraid this is a bit of a mess.

http://codereview.appspot.com/143055/diff/10/1006
File ly/music-functions-init.ly (right):

http://codereview.appspot.com/143055/diff/10/1006#newcode17
Line 17: #(define (void-make-music)
Like Carl's said, it's fine leaving this for another patch.

A few pointers:

For consistency with the other helper functions, it should be named
make-void-music

It should go in music-functions.scm

It should be define-public

http://codereview.appspot.com/143055/diff/10/1006#newcode176
Line 176: 'delta-step delta))
should be aligned with 'BendAfterEvent

http://codereview.appspot.com/143055/diff/10/1006#newcode180
Line 180: (_i "Direct output for the current book block to
@var{newfilename}")
full stop after @var{newfilename}

http://codereview.appspot.com/143055/diff/10/1006#newcode187
Line 187: @var{newsuffix}")
full stop after @var{newsuffix}

http://codereview.appspot.com/143055/diff/10/1006#newcode604
Line 604: (list part1 part2)))
aligns with parser

http://codereview.appspot.com/143055/diff/10/1006#newcode648
Line 648: 'element main-music
This and the following lines should be aligned with 'QuoteMusic.

http://codereview.appspot.com/143055

---

----
Join the Frogs!