Reviewers: Graham Percival,
Message:
On 2012/03/15 00:49:01, Graham Percival wrote:
looks basically good, but two minor points.
One big issue -- the patch is reversed.
My bad in attempting to use git-cl.
I'm currently trying to work out how to delete this issue and create a
new one with the patch the right way around.
Make that three minor points: please CC to -devel, not -devl.
http://codereview.appspot.com/5829043/diff/1/ly/articulate.ly
File ly/articulate.ly (right):
http://codereview.appspot.com/5829043/diff/1/ly/articulate.ly#newcode482
ly/articulate.ly:482: (let ((pset (make-music 'PropertySet
The indentation looks a bit off, but we don't have a tool for scheme
indentation, nor do we even have any consistency in terms of spaces
vs. tabs in
.scm files, so good enough for me.
http://codereview.appspot.com/5829043/diff/1/ly/articulate.ly#newcode625
ly/articulate.ly:625: (else music))
extra space?
http://codereview.appspot.com/5829043/diff/1/ly/articulate.ly#newcode630
ly/articulate.ly:630: % At last ... here's the music function that
aplies all
the above to a
this adds a typo?
Description:
Fix mordents and pralltriller in articulate.ly
Mordents should be on-beat, not grace notes. And pralltrillers
(half-shakes) are always either 4 alternating notes, or an inverted
mordent.
There is of course a general problem here in that the way ornaments
are realised has changed through the centuries. Even Bach and
Clementi disagree! I'm following CPE Bach's `True art of Keyboard
Playing' in the interpretation here. To do the job properly we'd
have
two articulations: \prall and \inverted_mordent and treat them
separately for MIDI, but typeset the same glyph.
Reported-by: Christopher Maden <cr...@maden.org>
Signed-off-by: Peter Chubb <peter.ch...@nicta.com.au>
Please review this at http://codereview.appspot.com/5829043/
Affected files:
M ly/articulate.ly
Index: ly/articulate.ly
diff --git a/ly/articulate.ly b/ly/articulate.ly
index
a345468ef71395bb745f1e867fc9d525a4d56526..d2d3b182ffd3dd835e607c53bf39f0f4cb3832f5
100644
--- a/ly/articulate.ly
+++ b/ly/articulate.ly
@@ -341,18 +341,8 @@
; (ac:accel trillMusic factor))
)))
-%
-% Generate a tempoChangeEvent and its associated property setting.
-%
-#(define (ac:tempoChange tempo)
- (make-sequential-music
- (list (make-music 'TempoChangeEvent
- 'metronome-count
- tempo
- 'tempo-unit
- (ly:make-duration 0 0 1 1))
- (context-spec-music
- (make-property-set 'tempoWholesPerMinute tempo) 'Score))))
+
+
% If there's an articulation, use it.
% If in a slur, use (1 . 1) instead.
@@ -424,14 +414,6 @@
(string= t "rall."))
(loop factor (cons e newelements) tail (cons 'rall actions)))
((or
- (string= t "accelerando")
- (string= t "accel")
- (string= t "accel."))
- (loop factor (cons e newelements) tail (cons 'accel actions)))
- ((or
- (string= t "poco accel."))
- (loop factor (cons e newelements) tail (cons 'pocoAccel actions)))
- ((or
(string= t "poco rall.")
(string= t "poco rit."))
(loop factor (cons e newelements) tail (cons 'pocoRall actions)))
@@ -495,37 +477,25 @@
(make-music 'RestEvent 'duration (ly:make-duration len dots newnum
newdenom))))))
music)))
- ((accel)
- (set! ac:lastTempo ac:currentTempo)
- (set! ac:currentTempo (ly:moment-div ac:currentTempo ac:rallFactor))
- (let ((pset (ac:tempoChange ac:currentTempo)))
- (if (null? (cdr actions))
- (make-sequential-music (list pset music))
- (make-sequential-music
- (list pset (loop (cdr actions)))))))
-
- ((pocoAccel)
- (set! ac:lastTempo ac:currentTempo)
- (set! ac:currentTempo (ly:moment-div ac:currentTempo ac:pocoRallFactor))
- (let ((pset (ac:tempoChange ac:currentTempo)))
- (if (null? (cdr actions))
- (make-sequential-music (list pset music))
- (make-sequential-music
- (list pset (loop (cdr actions)))))))
-
((rall)
- (set! ac:lastTempo ac:currentTempo)
(set! ac:currentTempo (ly:moment-mul ac:currentTempo ac:rallFactor))
- (let ((pset (ac:tempoChange ac:currentTempo)))
+ (let ((pset (make-music 'PropertySet
+ 'value
+ ac:currentTempo
+ 'symbol
+ 'tempoWholesPerMinute)))
(if (null? (cdr actions))
(make-sequential-music (list pset music))
(make-sequential-music
(list pset (loop (cdr actions)))))))
((pocoRall)
- (set! ac:lastTempo ac:currentTempo)
(set! ac:currentTempo (ly:moment-mul ac:currentTempo ac:pocoRallFactor))
- (let ((pset (ac:tempoChange ac:currentTempo)))
+ (let ((pset (make-music 'PropertySet
+ 'value
+ ac:currentTempo
+ 'symbol
+ 'tempoWholesPerMinute)))
(if (null? (cdr actions))
(make-sequential-music (list pset music))
(make-sequential-music
@@ -533,8 +503,11 @@
((aTempo)
(set! ac:currentTempo ac:lastTempo)
-
- (let ((pset (ac:tempoChange ac:currentTempo)))
+ (let ((pset (make-music 'PropertySet
+ 'value
+ ac:currentTempo
+ 'symbol
+ 'tempoWholesPerMinute)))
(if (null? (cdr actions))
(make-sequential-music (list pset music))
(make-sequential-music
@@ -649,12 +622,12 @@
(ac:adjust-props (ly:music-property music 'symbol) music)
music)
- (else music))
+ (else music))
))
-% At last ... here's the music function that applies all the above to a
+% At last ... here's the music function that aplies all the above to a
% score.
articulate = #(define-music-function (parser location music)
(ly:music?)
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel