Part combiner: Ignore skips coinciding with rests within a part (issue 240790043 by address@hidden)

[nine . fierce . ballads]

Reviewers: ,

Description:
Part combiner: Ignore skips coinciding with rests within a part

Please review this at https://codereview.appspot.com/240790043/

Affected files (+19, -14 lines):
  M input/regression/part-combine-silence-mixed.ly
  M scm/part-combiner.scm


Index: input/regression/part-combine-silence-mixed.ly
diff --git a/input/regression/part-combine-silence-mixed.ly  
b/input/regression/part-combine-silence-mixed.ly
index  
7d59777775819bf3a148caa7b1ce68d6a2375436..a399431a006a70269b31db8a2a85d22729a35cc8  
100644
--- a/input/regression/part-combine-silence-mixed.ly
+++ b/input/regression/part-combine-silence-mixed.ly
@@ -1,18 +1,18 @@
 \version "2.19.16"

 \header {
-  texidoc = "Different kinds of silence are not merged into the shared  
voice even if they begin and end simultaneously."
+  texidoc = "Different kinds of silence are not merged into the shared  
voice even if they begin and end simultaneously; however, when rests and  
skips are present in the same part, the skips are ignored."
 }

 \score { <<
   \new Staff {
     \partcombine
-      \relative f' { R1^"R" | s1^"s" | r1^"r" }
-      \relative f' { r1_"r" | R1_"R" | s1_"s" }
+      \relative f' { R1^"R" | s1^"s" | r1^"r" | << R1 s1 s4 >> | << r1 s2  
s4 >> }
+      \relative f' { r1_"r" | R1_"R" | s1_"s" | << s4 s1 R1 >> | << s4 s2  
r1 >> }
   }
   \new Staff {
     \partcombine
-      \relative f' { r1^"r" | R1^"R" | s1^"s" }
-      \relative f' { R1_"R" | s1_"s" | r1_"r" }
+      \relative f' { r1^"r" | R1^"R" | s1^"s" | << s4 s1 R1 >> | << s4 s2  
r1 >> }
+      \relative f' { R1_"R" | s1_"s" | r1_"r" | << R1 s1 s4 >> | << r1 s2  
s4 >> }
   }
 >> }
Index: scm/part-combiner.scm
diff --git a/scm/part-combiner.scm b/scm/part-combiner.scm
index  
a2ebda492cff8c30ce8e6360f1df56d16020e84d..20b7b05cc56d329ce9208db2419504a32a7302ee  
100644
--- a/scm/part-combiner.scm
+++ b/scm/part-combiner.scm
@@ -63,11 +63,16 @@
              (note-events vs))
         note<?))

-(define-method (rest-and-skip-events (vs <Voice-state>))
-  (define (f? x)
-    (or (ly:in-event-class? x 'rest-event)
-        (ly:in-event-class? x 'skip-event)))
-  (filter f? (events vs)))
+(define-method (rest-or-skip-events (vs <Voice-state>))
+  (define (filtered-events event-class)
+    (filter (lambda(x) (ly:in-event-class? x event-class))
+            (events vs)))
+  (let ((result (filtered-events 'rest-event)))
+    ;; There may be skips in the same part with rests for various
+    ;; reasons.  Regard the skips only if there are no rests.
+    (if (and (not (pair? result)) (not (any-mmrest-events vs)))
+        (set! result (filtered-events 'skip-event)))
+  result))

 (define-method (any-mmrest-events (vs <Voice-state>))
   (define (f? x)
@@ -477,8 +482,8 @@ Only set if not set previously.
                  (vs2 (cdr (voice-states now-state))))

             (define (analyse-synced-silence)
-              (let ((rests1 (if vs1 (rest-and-skip-events vs1) '()))
-                    (rests2 (if vs2 (rest-and-skip-events vs2) '())))
+              (let ((rests1 (if vs1 (rest-or-skip-events vs1) '()))
+                    (rests2 (if vs2 (rest-or-skip-events vs2) '())))
                 (cond

                  ;; multi-measure rests (probably), which the
@@ -616,8 +621,8 @@ the mark when there are no spanners active.
         (let* ((now-state (vector-ref result result-idx))
                (vs1 (current-voice-state now-state 1))
                (vs2 (current-voice-state now-state 2))
-               (rests1 (if vs1 (rest-and-skip-events vs1) '()))
-               (rests2 (if vs2 (rest-and-skip-events vs2) '()))
+               (rests1 (if vs1 (rest-or-skip-events vs1) '()))
+               (rests2 (if vs2 (rest-or-skip-events vs2) '()))
                (prev-state (if (> result-idx 0)
                                (vector-ref result (- result-idx 1))
                                #f))