lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Tracker 836: Add facility to change output file-name for a \book blo


From: ian
Subject: Re: Tracker 836: Add facility to change output file-name for a \book block
Date: Wed, 04 Nov 2009 23:03:27 +0000

Thanks for the feedback.  Is the now patch readu to push now?

Cheers,
Ian


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

http://codereview.appspot.com/143055/diff/19/1017#newcode676
Line 676: PARSER->lexer_->set_identifier (ly_symbol2scm
("book-filename"), SCM_BOOL_F);
On 2009/11/04 18:21:47, Neil Puttock wrote:
There are a few spaces between tabs here, i.e.,

[tab][spaces][tab]

iso

[tab][tab][spaces]




Done.

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

http://codereview.appspot.com/143055/diff/19/1019#newcode16
Line 16: %% TODO: Move this declaration to music-functions.scm
On 2009/11/04 18:21:47, Neil Puttock wrote:
Please remove this completely.

Done.

http://codereview.appspot.com/143055/diff/19/1019#newcode616
Line 616: (let* ( (get-notes (lambda (ev-chord)
On 2009/11/04 04:31:44, Carl wrote:
eliminate space between ( (, and align the following lines

Done.

http://codereview.appspot.com/143055/diff/19/1019#newcode626
Line 626: (let*(  (trill-pitch (ly:music-property (car sec-note-events)
'pitch))
On 2009/11/04 04:31:44, Carl wrote:
space after let*, no space before (trill-pitch

Done.

http://codereview.appspot.com/143055/diff/19/1019#newcode628
Line 628: 'force-accidental)))
On 2009/11/04 04:31:44, Carl wrote:
This alignment isn't right.  'force-accidental should align with (car

Done.

http://codereview.appspot.com/143055/diff/19/1019#newcode630
Line 630: (if (ly:pitch? trill-pitch)
On 2009/11/04 04:31:44, Carl wrote:
(if should align with e in let*

Done.

http://codereview.appspot.com/143055/diff/19/1019#newcode632
Line 632: (ly:music-set-property! m 'pitch trill-pitch)) trill-events)
On 2009/11/04 04:31:44, Carl wrote:
should be indented relative to lambda

Done.

http://codereview.appspot.com/143055/diff/19/1019#newcode638
Line 638: (for-each (lambda (m)
On 2009/11/04 04:31:44, Carl wrote:
align with (eq?

Done.

http://codereview.appspot.com/143055/diff/19/1019#newcode639
Line 639: (ly:music-set-property! m 'force-accidental forced))
On 2009/11/04 04:31:44, Carl wrote:
align with (eq?

Shouldn't this indent form (lambda (m) as it's the procedure body
declaration?

http://codereview.appspot.com/143055/diff/19/1020
File scm/lily-library.scm (right):

http://codereview.appspot.com/143055/diff/19/1020#newcode99
Line 99: (for-each (lambda (symbol)
On 2009/11/04 01:06:34, Patrick McCarty wrote:
I would recommend indenting like so:

   (for-each
     (lambda (symbol)
       (let ((..

Done.

http://codereview.appspot.com/143055/diff/19/1020#newcode99
Line 99: (for-each (lambda (symbol)
On 2009/11/04 18:21:47, Neil Puttock wrote:
On 2009/11/04 01:06:34, Patrick McCarty wrote:
> I would recommend indenting like so:
>
>   (for-each
>     (lambda (symbol)
>       (let ((..

I'd recommend reverting the changes, since there's absolutely nothing
wrong with
how this is currently indented. :)
Didn't get your e-mail until after I'd implemented Patrick's.  Sorry

http://codereview.appspot.com/143055/diff/19/1020#newcode151
Line 151: (ly:parser-lookup parser 'output-suffix)
On 2009/11/04 01:06:34, Patrick McCarty wrote:
align these with (not ...)

Done.

http://codereview.appspot.com/143055




reply via email to

[Prev in Thread] Current Thread [Next in Thread]