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 review the related code.

Regards,
Felipe

On 28 December 2010 21:57,  n.putt...@gmail.com wrote:
 LGTM.

 http://codereview.appspot.com/2726043/

 ___
 lilypond-devel mailing list
 lilypond-devel@gnu.org
 http://lists.gnu.org/mailman/listinfo/lilypond-devel



0001-Fix-compilation-error-due-to-undefined-symbol.patch
Description: application/download
___
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-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 attaching a simple patch
 that solves the issue, but you might want to review the related code.

Thanks, it seems I missed this occurrence of forceClef, when I ripped out that 
feature (I don't see any need for it with cue clefs). I have no idea why the 
compilation didn't fail here, though...

Cheers,
Reinhold

-- 
--
Reinhold Kainhofer, reinh...@kainhofer.com, http://reinhold.kainhofer.com/
 * Financial  Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel


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 to master?


I see this has been pushed now, but I wondered why
ambitus was removed from the space-alist of Clef?

(Clef
 . (
 (avoid-slur . inside)
 (break-align-anchor . 
,ly:break-aligned-interface::calc-extent-aligned-anchor)

 (break-align-symbol . clef)
 (break-visibility . ,begin-of-line-visible)
 (glyph-name . ,ly:clef::calc-glyph-name)
 (non-musical . #t)
- (space-alist . ((ambitus . (extra-space . 2.0))
+ (space-alist . ((cue-clef . (extra-space . 2.0))
   (staff-bar . (extra-space . 0.7))
   (key-cancellation . (minimum-space . 3.5))
   (key-signature . (minimum-space . 3.5))
   (time-signature . (minimum-space . 4.2))
   (first-note . (minimum-fixed-space . 5.0))
   (next-note . (extra-space . 0.5))
   (right-edge . (extra-space . 0.5

Trevor



___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel


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 break-align-orders the clef is 
always placed AFTER the ambitus, and the Ambitus grob already has an entry in 
its space-alist:
(clef . (extra-space . 0.5))

Furthermore, I had run into real trouble when I had both a space-alist entry 
for clef in the CueClef grob and a space-alist entry for cue-clef in the Clef 
grob. The cue clef would then not take up any horizontal space and always 
collide with the Clef. As soon as I removed the space-alist entry for clef 
from the CueClef grob, things worked as normal. 

Cheers,
Reinhold
-- 
--
Reinhold Kainhofer, reinh...@kainhofer.com, http://reinhold.kainhofer.com/
 * Financial  Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel


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 a 
following
grob of the listed break-align-symbol. Now, in break-align-orders 
the clef is
always placed AFTER the ambitus, and the Ambitus grob already has 
an entry in

its space-alist:
(clef . (extra-space . 0.5))

Furthermore, I had run into real trouble when I had both a 
space-alist entry
for clef in the CueClef grob and a space-alist entry for cue-clef 
in the Clef
grob. The cue clef would then not take up any horizontal space and 
always
collide with the Clef. As soon as I removed the space-alist entry 
for clef

from the CueClef grob, things worked as normal.


That makes sense to me now.  Thanks for the explanation.

Trevor



___
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


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:

don't need to set 'origin (it's done automatically by the syntax

constructor

`music-function')


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?

http://codereview.appspot.com/2726043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel


[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 for:

-) code duplication with the Clef_engraver, which is not easily solved, since 
we need to call also member functions of the engraver and access lots of 
different properties. The only way to share code would be to use a function 
with ~7 arguments, one of which is a pointer to the engraver, two would be 
refernces to the engraver's clef_ and octavate_ members, which the function 
would set. I don't think this is really good coding practice, so I didn't 
change that. 
If did resolve the OctavateEight code duplication in Cue_clef_engraver, 
though.

-) code duplication in scm/parser-clef.scm, which also needs to set several 
properties with different names for standard clefs and for cue clefs.

Cheers,
Reinhold
-- 
--
Reinhold Kainhofer, reinh...@kainhofer.com, http://reinhold.kainhofer.com/
 * Financial  Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

___
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 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):

http://codereview.appspot.com/2726043/diff/21001/Documentation/changes.tely#newcode71
Documentation/changes.tely:71: clef, which is correctly reset at the end
of the cue notes. At the begin
notes.  At

http://codereview.appspot.com/2726043/diff/21001/Documentation/changes.tely#newcode79
Documentation/changes.tely:79: \clef bass
indent

http://codereview.appspot.com/2726043/diff/21001/Documentation/changes.tely#newcode81
Documentation/changes.tely:81: c4 \cueDuringWithClef #vIQuote #DOWN
#treble { r4 r2 |
{
  r4 r2 |

http://codereview.appspot.com/2726043/diff/21001/Documentation/changes.tely#newcode82
Documentation/changes.tely:82: r4 } c4 c2 |
  r4
}

http://codereview.appspot.com/2726043/diff/21001/Documentation/changes.tely#newcode83
Documentation/changes.tely:83: \cueDuringWithClef #vIQuote #DOWN
soprano {R1*2 \break R1 } |
{ R1*2

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef-begin-of-score.ly
File input/regression/cue-clef-begin-of-score.ly (right):

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef-begin-of-score.ly#newcode1
input/regression/cue-clef-begin-of-score.ly:1: \version 2.13.41
2.13.45

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef-begin-of-score.ly#newcode4
input/regression/cue-clef-begin-of-score.ly:4: texidoc = Clefs for cue
notes at the start of a score should print the
indent

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef-begin-of-score.ly#newcode12
input/regression/cue-clef-begin-of-score.ly:12: \clef bass
indent

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef-new-line.ly
File input/regression/cue-clef-new-line.ly (right):

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef-new-line.ly#newcode1
input/regression/cue-clef-new-line.ly:1: \version 2.13.41
2.13.45

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef-new-line.ly#newcode4
input/regression/cue-clef-new-line.ly:4: texidoc = Clefs for cue notes
and line breaks. If the cue notes start in a
breaks.  If

indent

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef-new-line.ly#newcode5
input/regression/cue-clef-new-line.ly:5: new line, the cue clef should
not be printed at the end of the previuos line.
previous

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef-new-line.ly#newcode9
input/regression/cue-clef-new-line.ly:9: Cue notes going over a line
break should print the standard clef in the new
on the new

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef-new-line.ly#newcode17
input/regression/cue-clef-new-line.ly:17: \clef bass
indent

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef-octavation.ly
File input/regression/cue-clef-octavation.ly (right):

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef-octavation.ly#newcode1
input/regression/cue-clef-octavation.ly:1: \version 2.13.41
2.13.45

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef-octavation.ly#newcode4
input/regression/cue-clef-octavation.ly:4: texidoc = Octavation for
clefs for cue notes.
indent

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef-octavation.ly#newcode11
input/regression/cue-clef-octavation.ly:11: \clef treble_8 c1 |
indent

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef-octavation.ly#newcode13
input/regression/cue-clef-octavation.ly:13: c1 |\break
| \break

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef-octavation.ly#newcode18
input/regression/cue-clef-octavation.ly:18: \cueDuringWithClef
#vIQuote #UP #treble_8 { R1 \break R} |
R }

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef.ly
File input/regression/cue-clef.ly (right):

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef.ly#newcode1
input/regression/cue-clef.ly:1: \version 2.13.41
2.13.45

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef.ly#newcode3
input/regression/cue-clef.ly:3: % FIXME: Split this up into several
different test cases
remove

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef.ly#newcode5
input/regression/cue-clef.ly:5: texidoc = Clefs for cue notes: Print a
cue clef at the begin of the cue
indent

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef.ly#newcode13
input/regression/cue-clef.ly:13: \clef bass
indent

http://codereview.appspot.com/2726043/diff/21001/input/regression/cue-clef.ly#newcode14
input/regression/cue-clef.ly:14: c4 \cueDuringWithClef #vIQuote #DOWN
#treble { r4 r2 |
formatting { }


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 the music functions which
are basic `make-music' constructs (so long as they don't set 'origin on
any child music).

Cheers,
Neil



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

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

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 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?



It should be safe to remove 'origin from all the music functions which

are basic

`make-music' constructs (so long as they don't set 'origin on any

child music).


Okay, I removed all trivial 'origin settings:
http://codereview.appspot.com/3847041

I didn't look at the more complex function, where I suspect your
parenthesized condition comes into play...



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-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

parser-clef.scm.


In an ideal C++ world, that would be very easy by deriving
Cue_clef_engraver from Clef_engraver. However, in lilypond the engravers
should not have any hierarchy, so I don't see how I could easily share
some common functionality between two different engravers (where the
code accesses some class variables of the engravers).


Description:
Clef support for cue notes

-) Added \cleffedCueDuring, which allows to specify a clef for
   the cue notes. At the end of the cue section, the clef is
   automatically reset to the containing voice's clef.
-) Cue clefs are implemented as CueClef and CueEndClef grobs,
   created by a dedicated Cue_clef_engraver, which reads
   some cueClef* context properties.
-) After a line break, a cue clef does NOT override the global clef
   of the containing voice, but prints (in smaller size) after
   the containing clef.

Please review this at http://codereview.appspot.com/2726043/

Affected files:
  A input/regression/cue-clef.ly
  A lily/cue-clef-engraver.cc
  M lily/pitch-scheme.cc
  M ly/engraver-init.ly
  M ly/music-functions-init.ly
  M scm/define-context-properties.scm
  M scm/define-grobs.scm
  M scm/music-functions.scm
  M scm/parser-clef.scm



___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel


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):

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
indent

this looks like several tests bundled together

http://codereview.appspot.com/2726043/diff/1/input/regression/cue-clef.ly#newcode12
input/regression/cue-clef.ly:12: \clef bass
indent

http://codereview.appspot.com/2726043/diff/1/lily/cue-clef-engraver.cc
File lily/cue-clef-engraver.cc (right):

http://codereview.appspot.com/2726043/diff/1/lily/cue-clef-engraver.cc#newcode41
lily/cue-clef-engraver.cc:41: Direction octave_dir_;
remove

(appears to be left over in clef-engraver.cc from old ottava code)

http://codereview.appspot.com/2726043/diff/1/lily/cue-clef-engraver.cc#newcode47
lily/cue-clef-engraver.cc:47: //   DECLARE_TRANSLATOR_LISTENER
(cue_end_clef);
remove?

http://codereview.appspot.com/2726043/diff/1/lily/cue-clef-engraver.cc#newcode74
lily/cue-clef-engraver.cc:74: octave_dir_ = CENTER;
remove

http://codereview.appspot.com/2726043/diff/1/lily/cue-clef-engraver.cc#newcode100
lily/cue-clef-engraver.cc:100: Item *item = dynamic_castItem *
(info.grob ());
Item *item = info.item ();

http://codereview.appspot.com/2726043/diff/1/lily/cue-clef-engraver.cc#newcode121
lily/cue-clef-engraver.cc:121: Item *g = make_item (OctavateEight,
SCM_EOL);
farm out to separate function?

http://codereview.appspot.com/2726043/diff/1/lily/cue-clef-engraver.cc#newcode198
lily/cue-clef-engraver.cc:198: if (scm_is_string (glyph)) {
remove { }

http://codereview.appspot.com/2726043/diff/1/lily/cue-clef-engraver.cc#newcode200
lily/cue-clef-engraver.cc:200: } else {
remove { }

http://codereview.appspot.com/2726043/diff/1/lily/cue-clef-engraver.cc#newcode220
lily/cue-clef-engraver.cc:220: if (clef_  octavate_) {
newline for {

http://codereview.appspot.com/2726043/diff/1/lily/pitch-scheme.cc
File lily/pitch-scheme.cc (right):

http://codereview.appspot.com/2726043/diff/1/lily/pitch-scheme.cc#newcode173
lily/pitch-scheme.cc:173: if (scm_is_number (cue_pos)) {
remove { }

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))
don't need to set 'origin (it's done automatically by the syntax
constructor `music-function')

http://codereview.appspot.com/2726043/diff/1/scm/music-functions.scm
File scm/music-functions.scm (right):

http://codereview.appspot.com/2726043/diff/1/scm/music-functions.scm#newcode802
scm/music-functions.scm:802: (make-music 'Music)
indent

http://codereview.appspot.com/2726043/diff/1/scm/music-functions.scm#newcode806
scm/music-functions.scm:806: (context-spec-music
(make-voice-props-revert)  'CueVoice cue)
remove extra space

http://codereview.appspot.com/2726043/diff/1/scm/music-functions.scm#newcode808
scm/music-functions.scm:808: (make-music 'Music)
indent

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-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 you substitute 'Normal' for something like 
'Mensural' (if that is correct) or to use an erroneous example 'Eastern' 
as in Eastern Clefs which are 'Clefs from the East' it could apply.


I wasn't confident enough in my knowledge of the whole musical lexicon 
to know that perhaps, in some parts of Music Notation (see what I did 
there?), a 'Normal' clef might be something specific.


I've already shown my ignorance on the mail-lists (system vs score) so I 
was hedging my bets here.


;)

James







___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel


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):

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
On 2010/10/31 19:32:50, Valentin Villenave wrote:

Is it Normal that this word is capitalized?

Not normally, but in this case it is correct, even in English English.
The part before the colon is a heading, and the actual sentence begins
after the colon, so has a capitalised first word.

http://codereview.appspot.com/2726043/diff/1/lily/cue-clef-engraver.cc
File lily/cue-clef-engraver.cc (right):

http://codereview.appspot.com/2726043/diff/1/lily/cue-clef-engraver.cc#newcode108
lily/cue-clef-engraver.cc:108: if (!clef_)
Indentation?

http://codereview.appspot.com/2726043/diff/1/scm/define-context-properties.scm
File scm/define-context-properties.scm (right):

http://codereview.appspot.com/2726043/diff/1/scm/define-context-properties.scm#newcode250
scm/define-context-properties.scm:250: (forceCueClef ,boolean? Show cue
 clef symbol, even if it has not
spacing

http://codereview.appspot.com/2726043/diff/1/scm/define-grobs.scm
File scm/define-grobs.scm (right):

http://codereview.appspot.com/2726043/diff/1/scm/define-grobs.scm#newcode572
scm/define-grobs.scm:572: (break-align-anchor .
,ly:break-aligned-interface::calc-extent-aligned-anchor)
Indentation

http://codereview.appspot.com/2726043/diff/1/scm/define-grobs.scm#newcode594
scm/define-grobs.scm:594: (break-align-anchor .
,ly:break-aligned-interface::calc-extent-aligned-anchor)
Indentation

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 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):

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 is capitalized?

http://codereview.appspot.com/2726043/diff/1/lily/pitch-scheme.cc
File lily/pitch-scheme.cc (right):

http://codereview.appspot.com/2726043/diff/1/lily/pitch-scheme.cc#newcode175
lily/pitch-scheme.cc:175: }
Just out of curiosity: have you checked that it isn't affected by #466?

http://codereview.appspot.com/2726043/diff/1/ly/engraver-init.ly
File ly/engraver-init.ly (right):

http://codereview.appspot.com/2726043/diff/1/ly/engraver-init.ly#newcode79
ly/engraver-init.ly:79:
Are you sure that we want to \consist the Cue_clef_engraver by default?
Most of the time, it won't be needed at all... (I'm searching for a way
to conditionally load it when needed, but I can't find any though.)

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#newcode219
ly/music-functions-init.ly:219: (make-cue-clef-set type))
I'm not sure this is the proper indentation for .ly code.

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 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 is capitalized?

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.  :)

Cheers,
- Graham

___
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

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 =
I very much dislike this name.  What about \cueDuringWithClef?   English
speaking people are very creative in creating verbs out of nouns, but
somehow `cleffed' really hurts to me...

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 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 31, 2010 at 8:59 PM,  lemzw...@googlemail.com wrote:
 I very much dislike this name.  What about \cueDuringWithClef?   English
 speaking people are very creative in creating verbs out of nouns, but
 somehow `cleffed' really hurts to me...

Ditto. I didn't want to point this out, but cleffed is plain ugly (at
least to my French ears ;)

Cheers.

___
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 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 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?

--

If it's being used as a 'proper noun' it would be capitalised.

But it seems the rules are slightly different for American English (which our 
Docs are in), here from wikipedia (so it must be true!)

In English, the word following the colon is in lower case unless it is a 
proper noun or an acronym, or if it is normally capitalized for some other 
reason. However, in American English a colon may be followed either by a 
capital letter or by a lower-case letter, depending on usage; where direct 
speech follows, a capital letter is used; where an acronym or proper noun 
follows, a capital is used; otherwise, a lower-case letter is used. Some modern 
American style guides, including those published by the Associated Press and 
the Modern Language Association, prescribe capitalization where the colon is 
followed by an independent clause (i.e. a complete sentence). However, The 
Chicago Manual of Style requires capitalization only when the colon introduces 
two or more complete sentences.

So either this is a proper noun or an independent clause.

James

PS It looks wrong to my eyes BTW (but I'm not American...or Canadian ;) )

___
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 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 (which our 
 Docs are in), here from wikipedia (so it must be true!)

 capitalization where the colon is followed by an independent clause (i.e. a 
 complete sentence).

This clause doesn't exist in French. Has to be what Graham was referring to.

 PS It looks wrong to my eyes BTW (but I'm not American...or Canadian ;) )

What makes it look wrong is also that we do not have such grobs as
NormalClef or whatever...

Thank you for the explanation; nice to see that some people still
speak proper English :)

Cheers!
Valentin.

___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel