2009/11/12 :
> Is the patch ready to push now?
LGTM, apart from a few remaining whitespaces and wonky indents. :)
I've sorted these and will push to master shortly.
Thanks for being so patient,
Neil
___
lilypond-devel mailing list
lilypond-devel@gn
music-functions-init.ly
spaces+tab lines sorted.
indentation ll 626-633 fixed.
Is the patch ready to push now?
Cheers,
Ian
http://codereview.appspot.com/150044/diff/11/1015
File ly/music-functions-init.ly (right):
http://codereview.appspot.com/150044/diff/11/1015#newcode611
ly/music-functi
On 2009/11/12 01:12:35, Carl wrote:
A few whitespace errors (tab following spaces) and one indenting
mistake. Then
I think it's good to go.
A useful tip for catching these errors is to apply the patch locally:
git will tell you where the whitespace errors are.
http://codereview.appspot.com/
On 2009/11/12 01:12:35, Carl wrote:
A few whitespace errors (tab following spaces) and one indenting
mistake. Then
I think it's good to go.
A useful tip for catching these errors is to apply the patch locally:
git will tell you where the whitespace errors are.
http://codereview.appspot.com/
http://codereview.appspot.com/150044/diff/11/1015
File ly/music-functions-init.ly (right):
http://codereview.appspot.com/150044/diff/11/1015#newcode621
ly/music-functions-init.ly:621: (forced (ly:music-property (car
sec-note-events ) 'force-accidental)))
(car sec-note-events)
http://codereview.
A few whitespace errors (tab following spaces) and one indenting
mistake. Then I think it's good to go.
http://codereview.appspot.com/150044/diff/11/1015
File ly/music-functions-init.ly (right):
http://codereview.appspot.com/150044/diff/11/1015#newcode611
ly/music-functions-init.ly:611: (fil
Hi all,
Is it O K to push this patch now?
If so, can either Carl or Neil push this on origin/master.
Cheers and thanks for all the help on this one,
Ian
http://codereview.appspot.com/150044
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http
Have also looked for traling spaces in lily-library.scm
http://codereview.appspot.com/150044/diff/1/4
File ly/music-functions-init.ly (right):
http://codereview.appspot.com/150044/diff/1/4#newcode612
ly/music-functions-init.ly:612: (lambda (m) (eq? 'NoteEvent
(ly:music-property m 'name)))
On 20
http://codereview.appspot.com/150044/diff/1/4
File ly/music-functions-init.ly (left):
http://codereview.appspot.com/150044/diff/1/4#oldcode604
ly/music-functions-init.ly:604: (ly:music-property main-note
'elements
with (lambda
http://codereview.appspot.com/150044/diff/1/4
File ly/music-func
On 2009/11/08 20:17:37, Ian Hulin wrote:
The latest version of this patch is now ready for review.
`warning: 15 lines add whitespace errors.'
There were twelve in the last patchset. :)
Five are space-before-tab issues, the rest trailing spaces.
Generally looks OK, apart from a few remaining
Reviewers: Neil Puttock, carl.d.sorensen_gmail.com,
Message:
The latest version of this patch is now ready for review.
Cheers,
Ian
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
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-fun
On 2009/11/04 23:03:27, Ian Hulin wrote:
Thanks for the feedback. Is the now patch readu to push now?
There are several lines with trailing spaces.
You've reverted some recent changes in lily-library.scm which breaks
compilation; see `flatten-list' and `eval-carefully'.
Regards,
Neil
http:/
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_B
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)
On 2009/11/04 01:06:34, Patrick McCarty wrote:
The if block sho
Hi Ian,
Carl's already commented on \pitchedTrill, but there's absolutely
nothing wrong with its current indentation; the only thing that you
might change is the newline after the let*.
Regards,
Neil
http://codereview.appspot.com/143055/diff/19/1017
File lily/parser.yy (right):
http://coderev
Ian,
I found some indentation errors in the .ly file.
Thanks for your patience,
Carl
http://codereview.appspot.com/143055/diff/19/1019
File ly/music-functions-init.ly (right):
http://codereview.appspot.com/143055/diff/19/1019#newcode616
Line 616: (let* ( (get-notes (lambda (ev-chord)
elimin
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 so:
(for-each
(lambda (symbol)
(let ((..
http://codereview.appspot.c
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 so:
(for-each
(lambda (symbol)
(let ((..
http://codereview.appspot.c
Hi all,
I've installed emacs and checked the indenting as per Carl and Neils
recommendations. Patch set has been on rietveld 143055 for some days.
Is this ready to push now?
Cheers,
Ian
Carl Sorensen wrote:
On 10/29/09 6:10 PM, "Graham Percival" wrote:
On Thu, Oct 29, 2009 at 06:00
http://codereview.appspot.com/143055
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
http://codereview.appspot.com/143055
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
On 2009-11-03, Ian Hulin wrote:
> Hi all,
> I've installed emacs and checked the indenting as per Carl and Neils
> recommendations. Patch set has been on rietveld 143055 for some
> days.
>
> Is this ready to push now?
Hi Ian,
The indentation in lily-library.scm is still very weird in several
sp
Carl Sorensen writes:
> On 10/29/09 6:10 PM, "Graham Percival" wrote:
>
>> On Thu, Oct 29, 2009 at 06:00:06PM -0600, Carl Sorensen wrote:
>>>
>>> On 10/29/09 3:14 PM, "n.putt...@gmail.com" wrote:
>>>
I'd suggest installing emacs to sort out the indentation (even if you
prefer to use
On 10/29/09 6:10 PM, "Graham Percival" wrote:
> On Thu, Oct 29, 2009 at 06:00:06PM -0600, Carl Sorensen wrote:
>>
>> On 10/29/09 3:14 PM, "n.putt...@gmail.com" wrote:
>>
>>> I'd suggest installing emacs to sort out the indentation (even if you
>>> prefer to use another editor for your main
On Thu, Oct 29, 2009 at 06:00:06PM -0600, Carl Sorensen wrote:
>
> On 10/29/09 3:14 PM, "n.putt...@gmail.com" wrote:
>
> > 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
On 10/29/09 3:14 PM, "n.putt...@gmail.com" wrote:
> 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
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 wrote:
trailing space
Done.
http://codereview.appspot.com/143055/diff/10/10
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 shoul
Carl gave some feedback via e-mail. I've recorded them here before
uploading a patch set.
Ian
http://codereview.appspot.com/143055/diff/1/5
File scm/lily-library.scm (right):
http://codereview.appspot.com/143055/diff/1/5#newcode162
Line 162: (let* ( (base-name (get-current-filename parser))
r
Reviewers: Neil Puttock,
Message:
This patch is now available for review.
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
Reviewers: Neil Puttock,
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 manipulating
32 matches
Mail list logo
