Re: [PATCH] Re: Clef support for cue notes (issue2726043)
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: [PATCH] Re: Clef support for cue notes (issue2726043)
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)
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: Clef support for cue notes (issue2726043)
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: Clef support for cue notes (issue2726043)
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, 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)
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)
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)
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)
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)
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 |
[PATCH] Re: Clef support for cue notes (issue2726043)
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)
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
Re: Clef support for cue notes (issue2726043)
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)
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_cast
(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)
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)
On 31/10/2010 22:50, Valentin Villenave wrote: On Sun, Oct 31, 2010 at 11:26 PM, James Lowe 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)
On Sun, Oct 31, 2010 at 11:26 PM, James Lowe 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
RE: Clef support for cue notes (issue2726043)
-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 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)
On Sun, Oct 31, 2010 at 8:43 PM, Graham Percival 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, 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)
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)
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)
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)
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
Clef support for cue notes (issue2726043)
LGTM. Nicely done! Carl 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#newcode1 input/regression/cue-clef.ly:1: \version "2.11.39" 2.13.39 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#newcode5 lily/cue-clef-engraver.cc:5: Mats Bengtsson Is this copyright statement correct? http://codereview.appspot.com/2726043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
