Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1676)

Issue 143055: Tracker 836: Add facility to change output file-name for a \book block (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 8 months ago by Ian Hulin
Modified:
9 years, 7 months ago
Reviewers:
Patrick McCarty, carl.d.sorensen, Neil Puttock
CC:
lilypond-devel_gnu.org, frogs_lilynet.net
Visibility:
Public.

Description

Tracker 836: Add facility to change output file-name for a \book block or to set a suffix to prevent multiple files over-writing each other during a compilation. This change allows user to to this via functions rather than having to do so by setting and defining semi-documented parser variables.

Patch Set 1 #

Total comments: 3

Patch Set 2 : Implement indentation changes after feedback from Carl. #

Total comments: 20

Patch Set 3 : Implement Neil's formatting recommedation, defer using make-void-music function. #

Patch Set 4 : lily-library.scm indentation checked using emacs. #

Total comments: 30

Patch Set 5 : Apply Patrick's indentation recommendations for lily-library.scm. #

Patch Set 6 : Apply latest feedback from Carl and Neil #

Total comments: 16
Unified diffs Side-by-side diffs Delta from patch set Stats (+110 lines, -91 lines) Patch
M lily/parser.yy View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M ly/init.ly View 1 2 2 chunks +2 lines, -1 line 0 comments Download
M ly/music-functions-init.ly View 1 2 3 4 5 4 chunks +43 lines, -34 lines 12 comments Download
M scm/lily-library.scm View 1 2 3 4 4 chunks +63 lines, -56 lines 4 comments Download

Messages

Total messages: 13
Ian Hulin
This patch is now available for review.
9 years, 8 months ago (2009-10-29 14:01:19 UTC) #1
Ian Hulin
Carl gave some feedback via e-mail. I've recorded them here before uploading a patch set. ...
9 years, 8 months ago (2009-10-29 19:50:05 UTC) #2
Neil Puttock
Hi Ian, I haven't commented on lily-library.scm, since there are too many formatting issues. I'd ...
9 years, 8 months ago (2009-10-29 21:14:16 UTC) #3
Ian Hulin
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)) On 2009/10/29 21:14:17, Neil Puttock ...
9 years, 8 months ago (2009-10-29 23:44:15 UTC) #4
Patrick McCarty
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) I would recommend indenting like ...
9 years, 7 months ago (2009-11-04 01:06:34 UTC) #5
Patrick McCarty
9 years, 7 months ago (2009-11-04 01:07:39 UTC) #6
Patrick McCarty
9 years, 7 months ago (2009-11-04 01:08:32 UTC) #7
Carl
Ian, I found some indentation errors in the .ly file. Thanks for your patience, Carl ...
9 years, 7 months ago (2009-11-04 04:31:44 UTC) #8
Neil Puttock
Hi Ian, Carl's already commented on \pitchedTrill, but there's absolutely nothing wrong with its current ...
9 years, 7 months ago (2009-11-04 18:21:46 UTC) #9
Ian Hulin
lily-library.scm changes after Patrick's feedback. http://codereview.appspot.com/143055/diff/19/1020 File scm/lily-library.scm (right): http://codereview.appspot.com/143055/diff/19/1020#newcode142 Line 142: (if (not book-filename) ...
9 years, 7 months ago (2009-11-04 19:57:27 UTC) #10
Ian Hulin
Thanks for the feedback. Is the now patch readu to push now? Cheers, Ian http://codereview.appspot.com/143055/diff/19/1017 ...
9 years, 7 months ago (2009-11-04 23:03:27 UTC) #11
Neil Puttock
On 2009/11/04 23:03:27, Ian Hulin wrote: > Thanks for the feedback. Is the now patch ...
9 years, 7 months ago (2009-11-05 00:39:26 UTC) #12
Neil Puttock
9 years, 7 months ago (2009-11-05 00:40:43 UTC) #13
http://codereview.appspot.com/143055/diff/2010/2013
File ly/music-functions-init.ly (left):

http://codereview.appspot.com/143055/diff/2010/2013#oldcode604
Line 604: (ly:music-property main-note 'elements))))
goes with (lambda

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

http://codereview.appspot.com/143055/diff/2010/2013#newcode611
Line 611: (filter
goes with `a' in (lambda

http://codereview.appspot.com/143055/diff/2010/2013#newcode612
Line 612: (lambda (m) (eq? 'NoteEvent (ly:music-property m 'name)))
goes with filter

http://codereview.appspot.com/143055/diff/2010/2013#newcode613
Line 613: (ly:music-property ev-chord 'elements))))
same as above

http://codereview.appspot.com/143055/diff/2010/2013#newcode614
Line 614: (sec-note-events (get-notes secondary-note))
goes with (get-notes

http://codereview.appspot.com/143055/diff/2010/2013#newcode615
Line 615: (trill-events (filter (lambda (m) (music-has-type m
'trill-span-event))
as above

http://codereview.appspot.com/143055/diff/2010/2013#newcode618
Line 618: (if (pair? sec-note-events)
goes with `e' in (let*

http://codereview.appspot.com/143055/diff/2010/2013#newcode619
Line 619: (begin
aligns with (pair?

http://codereview.appspot.com/143055/diff/2010/2013#newcode621
Line 621: (forced (ly:music-property (car sec-note-events )
(car sec-note-events)

http://codereview.appspot.com/143055/diff/2010/2013#newcode631
Line 631: (if (eq? forced #t)
same level as (if (ly:pitch? trill-pitch)

http://codereview.appspot.com/143055/diff/2010/2013#newcode634
Line 634: trill-events)))))
aligns with (lambda

http://codereview.appspot.com/143055/diff/2010/2013#newcode635
Line 635: main-note))
unindent to same level as (if (pair? sec-note-events)

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

http://codereview.appspot.com/143055/diff/2010/2014#newcode102
Line 102: (if (symbol? permission)
aligns with `e' in (let

http://codereview.appspot.com/143055/diff/2010/2014#newcode175
Line 175: "[^-[:alnum:]]"
goes with `s' in string-regexp-substitute

http://codereview.appspot.com/143055/diff/2010/2014#newcode176
Line 176: "_"
as above

http://codereview.appspot.com/143055/diff/2010/2014#newcode177
Line 177: output-suffix))))
as above
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b