Menu
▾
▴
#4348 Patch: Part combiner: move direction handling out of iterator
Originally created by: *anonymous
Originally created by: nine.fie...@gmail.com
Originally owned by: nine.fie...@gmail.com
Part combiner: move direction handling out of iterator
Originally posted by: nine.fie...@gmail.com
(No comment was entered for this change.)
Owner: nine.fie...@gmail.com
Originally posted by: pkx1...@gmail.com
Passes make, make check and a full make doc.
Reg test diff attached.
Labels: -Patch-new Patch-review
Originally posted by: nine.fie...@gmail.com
I tried this on my own scores just now, and I'm getting a lot of "this Voice needs a \voiceXx or \shiftXx setting" warnings.
I also notice the message "warning: Test unequal: BUG" in the log for input/regression/display-lily-tests.ly.
Labels: -Patch-review Patch-needs_work
Originally posted by: nine.fie...@gmail.com
I think the problems in my scores can and should be resolved in my scores.
I'm not sure what should be done about display-lily-tests.ly. I'd appreciate advice from an expert.
Labels: -Patch-needs_work Patch-review
Originally posted by: v.villenave
I don’t think it should be cause for alarm. All it signals is that display-lily prints two additional empty music blocks, as requested by your code in music-functions-init.ly. It should only affect people who rely on \displayLilyMusic, and even then it doesn’t produce "bad" (unusable) code.
(That being said, there may be a way of avoiding these empty blocks but I’m not sure what would be the cleanest way to achieve it.)
Originally posted by: nine.fie...@gmail.com
Call \displayLilyMusic output "NOT A BUG"
http://codereview.appspot.com/226430043
Labels: -Patch-review Patch-new
Originally posted by: dak@gnu.org
If \displayLilyMusic does no longer reflect input properly that it previously did, that is definitely a bug. Valentin argues that it "should not be cause for alarm" because it is sort of an expected consequence of not making the changed representation of \partCombine known to the music display but that's a bit absurd.
Check out some instances of
(define-extra-display-method SimultaneousMusic (expr parser) ...
in scm/define-music-display-methods.scm in order to figure out how to properly display music expressions that are not easily recognizable at the outermost level (in this case, as \partCombine).
Originally posted by: nine.fie...@gmail.com
Fix \displayLilyMusic output for \partcombine(Up|Down)
http://codereview.appspot.com/226430043
Originally posted by: k-ohara5...@oco.net
\displayLilyMusic tries to recognize LilyPond commands based on the Scheme structures they create. This is often a sophisticated programming task.
Maintaining \displayLilyMusic often requires that we re-teach it to recognize the output of commands that we change, adding a burden of synchronization to LilyPond. \displayLilyMusic does not even try to recognize some commands
\displayLilyMusic \new Voice {\voiceTwo b4}
The goal of this patch is to let \partcombine use the standard \voiceXxx commands, rather than re-implement them independently. I suggest we need only preserve the useful parts of existing Lilypond. I have trouble imagining a use for \displayLilyMusic\partcombine but the changed output seems as useful as the original.
Teaching \displayLilyMusic to reverse-engineer \partcombine as suggested in comment #7 might be possible, but I think it would be unwise. I think the resulting interdependencies in code would be enough of a burden to stop any further improvements to \partcombine.
Originally posted by: pkx1...@gmail.com
Passes make, make check and a full make doc.
The reg test "part-combine-force.ly" is the same as in comment #2.
The reg test "display-lily-tests.log" shows:
}
Interpreting music...
-Interpreting music...
+Interpreting music...Test unequal: NOT A BUG.
+in = \partcombine { c4 e4 }
+{ d4 f4 }
+out = \context Staff << { } { } \partcombine { c4 e4 }
+ { d4 f4 } >>
Writing header field `texidoc' to `./display-lily-tests.texidoc'...
Layout output to `./display-lily-tests.eps'...
Writing ./display-lily-tests-systems.texi...
Labels: -Patch-new Patch-review
Originally posted by: nine.fie...@gmail.com
The results in comment 10 seem to be based on the patch in comment 6 rather than the newer patch in comment 8.
> I think the resulting interdependencies in code would be enough of a burden to stop any further improvements to \partcombine.
I agree and foresee it becoming more difficult to maintain \displayLilyMusic with the rest of the things I hope to try in the near future.
Originally posted by: pkx1...@gmail.com
patch on countdown for April 24th
Labels: -Patch-review Patch-countdown
Originally posted by: pkx1...@gmail.com
Patch counted down - please push
Labels: -Patch-countdown Patch-push
Originally posted by: nine.fie...@gmail.com
Pushed to staging:
commit [r3740ac23c9e222602cec4ddc6c38b58504f17673]
Author: Dan Eble <nine.fierce.ballads@gmail.com>
Date: Fri Apr 17 23:01:55 2015 -0400
Issue 4348: Part combiner: move direction handling out of iterator
Labels: -Patch-push Fixed_2_19_19
Status: Fixed
Originally posted by: PhilEHol...@googlemail.com
(No comment was entered for this change.)
Status: Verified
Originally posted by: nine.fie...@gmail.com
Add DynamicLineSpanner to the set of grobs whose direction is controlled by \voiceOne etc. This reverses a difference in the regression test part-combine-force.ly shown in Issue 4348.
http://codereview.appspot.com/237740043
Labels: Patch-new
Status: Started
Originally posted by: nine.fie...@gmail.com
Argh! That patch was not supposed to go here!
Labels: -Patch-new
Status: Verified