T1686 - First patch to allow LilyPond Initialisation to use Guile V2 if required. (issue 6849088)
http://codereview.appspot.com/6849088/diff/1/lily/main.cc File lily/main.cc (right): http://codereview.appspot.com/6849088/diff/1/lily/main.cc#newcode65 lily/main.cc:65: int guile_major_version = 2; I don't think it makes sense to even define a guile_major_version since the executable will only work with the version it has been compiled with. So there is no point in having a run-time decision criterion available. http://codereview.appspot.com/6849088/diff/1/lily/main.cc#newcode296 lily/main.cc:296: /* You have quite a few commits that "just" add comments to existing functions. I think it would make sense to commit these improvements of existing code as a separate commit to staging. No point really to have them go through a review unless you are unsure whether some comment is correct (adding an incorrect comment obviously is not an improvement). If those are interspersed with other material in some commit, you can use git checkout -p commit-id to selectively pick out material from a given commit (in a different branch), then do git commit to make a commit from the picked changes. http://codereview.appspot.com/6849088/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: markup-commands rest-by-number and rest (issue 6850073)
On 2012/11/27 20:03:06, benko.pal wrote: > Did you had a look on the compiled output of the new reg-tests? no. could you push to a dev/ branch? Sorry. I'm still a newbie in devel tasks. Especially with the tools needed for it. Imagine that I tried to upload my new patch this evening. It lasted over 4 hours before I managed to _upload_ it. And it was wrong. I had to do it again. A hundred times (wasn't, but it feels like) I had to start again. Most of the mistakes were typos in git, which I couldn't correct. Two times I corrupted my build-directory, so I had to start from scratch. Or I produced a [PATCH 1/2] and I have no clue why, or why not the next time I tried. etc., etc I remember trying to checkout a dev/ branch. It resulted in a messed up git. So no, I don't know how to push a dev/branch and currently I feel not motivated to learn it. But in case this will change, do you have a hint where to look to learn not alone about dev/branch but to deal with git? Something like git for dummies would be suitable. > The main question is: > If there are no defined glyphs for a specific style, > should I try to select others (currently tried) > or > should I return default-glyphs (I suspect this would be much easier)? > > Opinions? I don't exactly know what is style and how is it set. I thought it's a local property of your new command and must be set explicitly by the user; in this case you can either declare that petrucci is an invalid style or make sure that setting petrucci is read as setting mensural. in other words, translate petrucci to mensural once and for all in the beginning. style _is_ a local property, but I tried to make it consistent with the style-property from \note-by-number, \note and the usage in \override NoteHead #'style ... and \override Rest #'style ... petrucci is now translated to mensural. I don't know whether it's relevant or not, but (in the default style) there's a ledgered variant for breve rest too (though not for longa). Yep. I decided not to use it, _because_ there are no ledgered variants for longa and maxima. If wanted, this could be changed quite easily. http://codereview.appspot.com/6850073/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Uses single algorithm for side-position spacing. (issue 6827072)
http://codereview.appspot.com/6827072/diff/18003/lily/box-quarantine.cc File lily/box-quarantine.cc (right): http://codereview.appspot.com/6827072/diff/18003/lily/box-quarantine.cc#newcode69 lily/box-quarantine.cc:69: int mid = ii0.mid_; assert ((double)(ii0.index - mid) >= 0.0) assert ((double)(ii1.index - mid) >= 0.0) http://codereview.appspot.com/6827072/diff/18003/lily/box-quarantine.cc#newcode93 lily/box-quarantine.cc:93: Offset shift (-(epsilon + (ii[0].amount_ + padding_) / 2.0), 0.0); The padding appears in some places but not others; see 'fingering-column.ly' http://codereview.appspot.com/6827072/diff/18003/lily/skyline.cc File lily/skyline.cc (right): http://codereview.appspot.com/6827072/diff/18003/lily/skyline.cc#newcode234 lily/skyline.cc:234: // Flatten to height h What is the difference between this and Skyline:::set_minimum_height() at line 817 ? http://codereview.appspot.com/6827072/diff/18003/scm/define-grob-properties.scm File scm/define-grob-properties.scm (right): http://codereview.appspot.com/6827072/diff/18003/scm/define-grob-properties.scm#newcode492 scm/define-grob-properties.scm:492: (horizon-padding ,number? "The amount to pad the axis We already have 'skyline-horizontal-padding' on line 815 and 'skyline-vertical-padding', so it would be better to use those. You could look up one or the other depending on which axis, X or Y, in which the buildings grow. http://codereview.appspot.com/6827072/diff/18003/scm/define-grobs.scm File scm/define-grobs.scm (right): http://codereview.appspot.com/6827072/diff/18003/scm/define-grobs.scm#newcode253 scm/define-grobs.scm:253: (horizon-padding . 0.05) This could be 'skyline-horizontal-padding' as in System and VerticalAxisGroup. http://codereview.appspot.com/6827072/diff/18003/scm/define-grobs.scm#newcode886 scm/define-grobs.scm:886: (add-stem-support . #t) Several regression tests assume this is false by default, and then toggle it to true, so as to test both cases. add-stem-support = #f doesn't seem to work very well with beams with this patch. http://codereview.appspot.com/6827072/diff/18003/scm/define-grobs.scm#newcode909 scm/define-grobs.scm:909: (FingeringCollision This would need a convert-ly rule. Why change the name ? http://codereview.appspot.com/6827072/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
