Re: Clef support for cue notes (issue2726043)

2010-12-29 Thread Felipe Gonçalves Assis
Hi, I have just updated my local repository, and noticed that this patch introduces a compilation error. In fact, there is an undefined variable in lily/cue-clef-engraver.cc:175 (compare with lily/clef-engraver.cc). I am attaching a simple patch that solves the issue, but you might want to

Re: Clef support for cue notes (issue2726043)

2010-12-29 Thread Reinhold Kainhofer
Am Mittwoch, 29. Dezember 2010, um 09:06:01 schrieb Felipe Gonçalves Assis: I have just updated my local repository, and noticed that this patch introduces a compilation error. In fact, there is an undefined variable in lily/cue-clef-engraver.cc:175 (compare with lily/clef-engraver.cc). I am

Re: [PATCH] Re: Clef support for cue notes (issue2726043)

2010-12-29 Thread Trevor Daniels
Reinhold Kainhofer wrote Tuesday, December 28, 2010 9:10 PM I have now uploaded a final patch for cue notes with custom clef, which I think is in a state so that it can be included in lilypond master: http://codereview.appspot.com/2726043/ Any further objectsions/suggestions? Or can I push

Re: [PATCH] Re: Clef support for cue notes (issue2726043)

2010-12-29 Thread Reinhold Kainhofer
Am Mittwoch, 29. Dezember 2010, um 12:13:08 schrieb Trevor Daniels: I see this has been pushed now, but I wondered why ambitus was removed from the space-alist of Clef? AFAIU, the space-alist controls spacing of the current grob to a following grob of the listed break-align-symbol. Now, in

Re: [PATCH] Re: Clef support for cue notes (issue2726043)

2010-12-29 Thread Trevor Daniels
Reinhold, you wrote Wednesday, December 29, 2010 12:13 PM Am Mittwoch, 29. Dezember 2010, um 12:13:08 schrieb Trevor Daniels: I see this has been pushed now, but I wondered why ambitus was removed from the space-alist of Clef? AFAIU, the space-alist controls spacing of the current grob to

Re: Clef support for cue notes (issue2726043)

2010-12-28 Thread reinhold . kainhofer
http://codereview.appspot.com/2726043/diff/1/ly/music-functions-init.ly File ly/music-functions-init.ly (right): http://codereview.appspot.com/2726043/diff/1/ly/music-functions-init.ly#newcode250 ly/music-functions-init.ly:250: 'origin location)) On 2010/11/25 22:50:40, Neil Puttock wrote:

[PATCH] Re: Clef support for cue notes (issue2726043)

2010-12-28 Thread Reinhold Kainhofer
I have now uploaded a final patch for cue notes with custom clef, which I think is in a state so that it can be included in lilypond master: http://codereview.appspot.com/2726043/ Any further objectsions/suggestions? Or can I push to master? I have included almost all comments so far, except

Re: Clef support for cue notes (issue2726043)

2010-12-28 Thread n . puttock
Hi Reinhold, You're missing definitions for explicitCueClefVisibility and quoted-music-clef. Cheers, Neil http://codereview.appspot.com/2726043/diff/21001/Documentation/changes.tely File Documentation/changes.tely (right):

Re: Clef support for cue notes (issue2726043)

2010-12-28 Thread n . puttock
On 2010/12/28 20:26:54, Reinhold wrote: Okay, I just copied this from cueDuring. I notice that lots of definitions in music-functions-init.ly contain 'origin location. So, can we remove it from all those definitions without breaking anything? It should be safe to remove 'origin from all

Re: Clef support for cue notes (issue2726043)

2010-12-28 Thread reinhold . kainhofer
Thanks, Neil, for all your comments (and indent nitpicks ;) ). I have included/fixed all of them (and some other minor issues) and posted the patch again (needs my other patch for the OctavateEight break-visibility to get octavated cue clefs right). Any further comments? Cheers, Reinhold

Re: Clef support for cue notes (issue2726043)

2010-12-28 Thread n . puttock
LGTM. http://codereview.appspot.com/2726043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Clef support for cue notes (issue2726043)

2010-12-28 Thread reinhold . kainhofer
On 2010/12/28 23:32:18, Neil Puttock wrote: On 2010/12/28 20:26:54, Reinhold wrote: Okay, I just copied this from cueDuring. I notice that lots of definitions in music-functions-init.ly contain 'origin location. So, can we remove it from all those definitions without breaking anything?

Re: Clef support for cue notes (issue2726043)

2010-12-27 Thread reinhold . kainhofer
Reviewers: carl.d.sorensen_gmail.com, Valentin Villenave, graham_percival-music.ca, lemzwerg, james.lowe_datacore.com, Trevor Daniels, Neil Puttock, Message: On 2010/11/25 22:50:40, Neil Puttock wrote: I think there's some scope for reducing code duplication in the engraver and

Re: Clef support for cue notes (issue2726043)

2010-11-25 Thread n . puttock
Hi Reinhold, I think there's some scope for reducing code duplication in the engraver and parser-clef.scm. Cheers, Neil http://codereview.appspot.com/2726043/diff/1/input/regression/cue-clef.ly File input/regression/cue-clef.ly (right):

Re: Clef support for cue notes (issue2726043)

2010-11-01 Thread James
On 31/10/2010 22:50, Valentin Villenave wrote: On Sun, Oct 31, 2010 at 11:26 PM, James Lowejames.l...@datacore.com wrote: If it's being used as a 'proper noun' it would be capitalised. I fail to see how it is a proper noun. Clefs that are normal - adjective, isn't it? Yes I suppose, but if

Re: Clef support for cue notes (issue2726043)

2010-11-01 Thread tdanielsmusic
Code not checked; and I still don't understand Scheme indentation, but at least it ought to be consistent. Trevor http://codereview.appspot.com/2726043/diff/1/input/regression/cue-clef.ly File input/regression/cue-clef.ly (right):

Re: Clef support for cue notes (issue2726043)

2010-10-31 Thread v . villenave
Hi Reinhold, it certainly looks good! I haven't tested your patch set though, so these are just a couple of nitpicks off the top of my head. http://codereview.appspot.com/2726043/diff/1/input/regression/cue-clef.ly File input/regression/cue-clef.ly (right):

Re: Clef support for cue notes (issue2726043)

2010-10-31 Thread Graham Percival
On Sun, Oct 31, 2010 at 07:32:50PM +, v.villen...@gmail.com wrote: http://codereview.appspot.com/2726043/diff/1/input/regression/cue-clef.ly#newcode4 input/regression/cue-clef.ly:4: texidoc = Clefs for cue notes: Normal clefs should be printed, and in addition Is it Normal that this word

Re: Clef support for cue notes (issue2726043)

2010-10-31 Thread lemzwerg
http://codereview.appspot.com/2726043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Clef support for cue notes (issue2726043)

2010-10-31 Thread lemzwerg
Oops, somehow I've just created an empty comment. http://codereview.appspot.com/2726043/diff/1/ly/music-functions-init.ly File ly/music-functions-init.ly (right): http://codereview.appspot.com/2726043/diff/1/ly/music-functions-init.ly#newcode238 ly/music-functions-init.ly:238: cleffedCueDuring

Re: Clef support for cue notes (issue2726043)

2010-10-31 Thread Valentin Villenave
On Sun, Oct 31, 2010 at 8:43 PM, Graham Percival gra...@percival-music.ca wrote: In correct English, that word would be capitalized.  However, most people don't bother to write that well.  So it's not normal to see this capitalized correctly.  :) Interesting. Could you elaborate? On Sun, Oct

RE: Clef support for cue notes (issue2726043)

2010-10-31 Thread James Lowe
-Original Message- From: lilypond-devel-bounces+james.lowe=datacore@gnu.org on behalf of Valentin Villenave Sent: Sun 31/10/2010 22:12 To: Graham Percival Cc: re...@codereview.appspotmail.com; lilypond-devel@gnu.org Subject: Re: Clef support for cue notes (issue2726043) On Sun, Oct

Re: Clef support for cue notes (issue2726043)

2010-10-31 Thread Valentin Villenave
On Sun, Oct 31, 2010 at 11:26 PM, James Lowe james.l...@datacore.com wrote: If it's being used as a 'proper noun' it would be capitalised. I fail to see how it is a proper noun. Clefs that are normal - adjective, isn't it? But it seems the rules are slightly different for American English