Fixes issue 1881 (cyclic dependency with beam calculations) (issue 5038042)
Reviewers: ,
Message:
Hey all,
A one-line-deletion patch to fix 1881.
I think the re-introduction of length and stem-begin-position both made
this line obsolete and triggered cyclic dependencies.
I'm still not sure why these dependencies resulted in changed visual
output, but this patch seems to fix the cyclic dependency error without
causing any change in the regtests.
Cheers,
MS
Description:
Fixes issue 1881 (cyclic dependency with beam calculations)
Please review this at http://codereview.appspot.com/5038042/
Affected files:
M lily/stem.cc
Index: lily/stem.cc
diff --git a/lily/stem.cc b/lily/stem.cc
index
8a755afaa5aec63946003a19017be335f0c39b80..93ebea4c96cfaf75db438a1cbcd83b675a3f2dc9
100644
--- a/lily/stem.cc
+++ b/lily/stem.cc
@@ -160,7 +160,6 @@ Stem::set_stem_positions (Grob *me, Real se)
me->set_property ("stem-begin-position", scm_from_double (height[-d] * 2
/ staff_space));
me->set_property ("length", scm_from_double (height.length () * 2 /
staff_space));
- (void) me->extent (me, Y_AXIS);
}
/* Note head that determines hshift for upstems
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reg test check differences after last two commits this morning 17th Sept (by Joe)
Hello, I saw that Joe had put two commits in since last night (16th Sept) and when I ran a make check against a snapshot of yesterday's tree and this morning's tree, I got two 'significant' reg test differences. See: http://lilypond-stuff.1065243.n5.nabble.com/make-check-diffs-between-16-and-17-Sept-2011-td4813254.html I don't know if this is expected based on either/both commits but in the two reg tests the 'instrument names' have now disappeared. Sorry if this is just noise. Regards -- -- James ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Reg test check differences after last two commits this morning 17th Sept (by Joe)
On 17 September 2011 09:27, Peekay Ex wrote:
> I don't know if this is expected based on either/both commits but in
> the two reg tests the 'instrument names' have now disappeared.
Nope, it's a regression. I'm afraid Joe's fallen into the same trap I
did when I looked at issue #1598. We can't filter out non-spaceable
lines in the Instrument_name_engraver since it prevents them having
instrument names (or vocal names for lyrics).
\version "2.15.12"
\relative c' {
\set Staff.instrumentName = #"Music"
c d e f
}
\addlyrics {
\set vocalName = #"Lyrics"
foo bar baz
}
-> missing vocal name
Cheers,
Neil
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Terminates outside_staff_callback early if a grob is outside a slur's X-extent (issue 5056041)
Reviewers: , Message: Hey all, This patch fixes the programming error coming from the StrokeFinger discussion. Currently, none of my outstanding slur work would fix this problem. The issue is that the offset callback is tacked onto outside-staff objects before we know if the object falls within the span of the slur. Thus, even though these objects are weeded out of slur scoring in score_extra_ecompasses, the callback function doesn't know they've been weeded out and a separate weeding out needs to happen. This patch does that. I'm not sure where/when/why this behavior would have changed, but I think this is the appropriate fix. Cheers, MS Description: Terminates outside_staff_callback early if a grob is outside a slur's X-extent Please review this at http://codereview.appspot.com/5056041/ Affected files: M lily/slur.cc Index: lily/slur.cc diff --git a/lily/slur.cc b/lily/slur.cc index 964a4d091b210e0ab78f4bb8c980291679a317e1..582803832165f8fb3f2a783cadcb3f50699b5c15 100644 --- a/lily/slur.cc +++ b/lily/slur.cc @@ -264,12 +264,22 @@ Slur::outside_slur_callback (SCM grob, SCM offset_scm) Grob *cy = script->common_refpoint (slur, Y_AXIS); Bezier curve = Slur::get_curve (slur); - + curve.translate (Offset (slur->relative_coordinate (cx, X_AXIS), slur->relative_coordinate (cy, Y_AXIS))); Interval yext = robust_relative_extent (script, cy, Y_AXIS); Interval xext = robust_relative_extent (script, cx, X_AXIS); + Interval slur_wid (curve.control_[0][X_AXIS], curve.control_[3][X_AXIS]); + + bool contains = false; + Direction d = LEFT; + do +contains = contains || slur_wid.contains (xext[d]); + while (flip (&d) != LEFT); + + if (!contains) +return offset_scm; Real offset = robust_scm2double (offset_scm, 0); yext.translate (offset); ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fixes issue 1881 (cyclic dependency with beam calculations) (issue 5038042)
LGTM. BTW, I have a few queries about stem::length: 76 (let* ((d (ly:grob-property grob 'direction)) You don't use 'direction; is it still necessary to get it to trigger other calculations? 79 (beam (ly:grob-object grob 'beam))) Why do you need to access 'beam? AFAICT, the callback will never be triggered on beamed notes, so it's redundant. It also makes the last line look like a thinko: 82 (ly:grob-property grob 'length This would be a calculation-in-progress, since you're already inside the callback. http://codereview.appspot.com/5038042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add support for custom ledger positions, using two new staff-symbol properties (issue 4974075)
On 16 September 2011 12:50, Peekay Ex wrote: > The CG doesn't mention this for reg test checking. Indeed, it only mentions this for debugging. > Is this something I should be doing now and if so does it matter where > this switch comes in the syntax? It's part of the configure process, so either ./autogen.sh --disable-optimising or (out-of-tree build with --noconfigure) ../configure --disable-optimising You'll have to do `make bin-clean' before reconfiguring to ensure the binary is built properly if you're not starting from scratch. Cheers, Neil ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Progress on loose columns. (issue 4841052)
Hey all, This patch hasn't been on a countdown yet - could it get put on a countdown and could people please give it a look in its most recent and rebased form? It fixes issue 1301, where there is a loose column that collides with an earlier non-loose column. Cheers, MS http://codereview.appspot.com/4841052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fixes issue 1881 (cyclic dependency with beam calculations) (issue 5038042)
On Sep 17, 2011, at 11:07 AM, n.putt...@gmail.com wrote: > LGTM. > > BTW, I have a few queries about stem::length: > > 76 (let* ((d (ly:grob-property grob 'direction)) > > You don't use 'direction; is it still necessary to get it to trigger > other calculations? > I doubt it - it's likely vestigial. I'll work on a patch that addresses the issues below and remove this line. > 79 (beam (ly:grob-object grob 'beam))) > > Why do you need to access 'beam? AFAICT, the callback will never be > triggered on beamed notes, so it's redundant. It also makes the last > line look like a thinko: > > 82 (ly:grob-property grob 'length > > This would be a calculation-in-progress, since you're already inside the > callback. > Agreed - I can likely get rid of all this stuff. I like the term `thinko' :) I'll try reducing this as much as possible and will post a patch. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Prunes stem::length down to the bare minimum. (issue 5057041)
Reviewers: , Message: A response to Neil's e-mail concerning stem::length - this is a cleaner implementation. Cheers, MS Description: Prunes stem::length down to the bare minimum. Please review this at http://codereview.appspot.com/5057041/ Affected files: M scm/output-lib.scm Index: scm/output-lib.scm diff --git a/scm/output-lib.scm b/scm/output-lib.scm index 70e3ba3eeb07885ff46475ec6f20b2a841dd01f8..33a2df36a70afcec5747ebaca2363028786e26a8 100644 --- a/scm/output-lib.scm +++ b/scm/output-lib.scm @@ -73,17 +73,16 @@ (ly:event-property (event-cause grob) 'duration))) (define-public (stem::length grob) - (let* ((d (ly:grob-property grob 'direction)) - (ss (ly:staff-symbol-staff-space grob)) + (let* ((ss (ly:staff-symbol-staff-space grob)) (beg (ly:grob-property grob 'stem-begin-position)) (beam (ly:grob-object grob 'beam))) (if (null? beam) (abs (- (ly:stem::calc-stem-end-position grob) beg)) -(ly:grob-property grob 'length +(ly:programming-error + "stem::length called but will not be used for beamed stem." (define-public (stem::pure-length grob beg end) - (let* ((d (ly:grob-property grob 'direction)) - (ss (ly:staff-symbol-staff-space grob)) + (let* ((ss (ly:staff-symbol-staff-space grob)) (beg (ly:grob-pure-property grob 'stem-begin-position 0 1000))) (abs (- (ly:stem::pure-calc-stem-end-position grob 0 2147483646) beg ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Prevents nested tuplets from colliding. (issue 4808082)
On Sep 6, 2011, at 9:41 AM, tdanielsmu...@googlemail.com wrote: > Mike says this is not yet ready for pushing in comment #11. We should > not push while he is away without his confirmation. > > Trevor > Hey all, I'm pretty sure that the differences I'm noticing come from more accurate Y-extents of stems. However, if these Y-extents are in fact less accurate then we're in trouble. All of my tests have come back reporting them as more accurate, but then again, I designed my own tests (like the Y-extent printer I sent out to the list back when I was working on stems). If people could report back perceived changes in how new stem Y-extents are effecting layout (good or bad), this'd help me evaluate if there are any flaws in the Stem code. For the moment, I don't see any. I'm comfortable with pushing this version of the patch in its current form (after I re-run regtests on it and after I push other stem fixes being tossed around - these fixes may have an impact on this patch). However, if people think the gaps between the nested tuplets needs to be controlled via a separate property, a new property can be added that controls the distance between nested TupletBracket and TupletNumber grobs (currently, there is a one-property-fits-all property called "padding"). It may be a good idea, however, to get this pushed first and then to work on that later. > > http://codereview.appspot.com/4808082/diff/45009/lily/tuplet-bracket.cc > File lily/tuplet-bracket.cc (right): > > http://codereview.appspot.com/4808082/diff/45009/lily/tuplet-bracket.cc#newcode285 > lily/tuplet-bracket.cc:285: if (!scm_is_pair (scm_x_span) || > !scm_is_pair (scm_positions)) > Should issue programming error if user has specified invalid value for > 'positions. > Should there also be a programming error if X-positions is incorrectly set? Also, wouldn't this erroneous setting be blocked with a property check (positions is number-pair? in define-grob-properties.scm). Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fixes issue 1881 (cyclic dependency with beam calculations) (issue 5038042)
Mike, On Sat, Sep 17, 2011 at 10:27 AM, m...@apollinemike.com wrote: > On Sep 17, 2011, at 11:07 AM, n.putt...@gmail.com wrote: > >> LGTM. >> >> BTW, I have a few queries about stem::length: >> >> 76 (let* ((d (ly:grob-property grob 'direction)) >> >> You don't use 'direction; is it still necessary to get it to trigger >> other calculations? >> > > I doubt it - it's likely vestigial. I'll work on a patch that addresses the > issues below and remove this line. > >> 79 (beam (ly:grob-object grob 'beam))) >> >> Why do you need to access 'beam? AFAICT, the callback will never be >> triggered on beamed notes, so it's redundant. It also makes the last >> line look like a thinko: >> >> 82 (ly:grob-property grob 'length >> >> This would be a calculation-in-progress, since you're already inside the >> callback. >> > > Agreed - I can likely get rid of all this stuff. I like the term `thinko' :) > > I'll try reducing this as much as possible and will post a patch. > Are 5038042 and 5057041 both for 1881? or just 5038042 and if they are all the same 1881 can I just apply both patches to do the checks or do I need to reg test each one separately? Thanks -- -- James ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fixes issue 1881 (cyclic dependency with beam calculations) (issue 5038042)
On Sep 17, 2011, at 1:06 PM, Peekay Ex wrote: > Mike, > > On Sat, Sep 17, 2011 at 10:27 AM, m...@apollinemike.com > wrote: >> On Sep 17, 2011, at 11:07 AM, n.putt...@gmail.com wrote: >> >>> LGTM. >>> >>> BTW, I have a few queries about stem::length: >>> >>> 76 (let* ((d (ly:grob-property grob 'direction)) >>> >>> You don't use 'direction; is it still necessary to get it to trigger >>> other calculations? >>> >> >> I doubt it - it's likely vestigial. I'll work on a patch that addresses the >> issues below and remove this line. >> >>> 79 (beam (ly:grob-object grob 'beam))) >>> >>> Why do you need to access 'beam? AFAICT, the callback will never be >>> triggered on beamed notes, so it's redundant. It also makes the last >>> line look like a thinko: >>> >>> 82 (ly:grob-property grob 'length >>> >>> This would be a calculation-in-progress, since you're already inside the >>> callback. >>> >> >> Agreed - I can likely get rid of all this stuff. I like the term `thinko' :) >> >> I'll try reducing this as much as possible and will post a patch. >> > > Are 5038042 and 5057041 both for 1881? or just 5038042 and if they are > all the same 1881 can I just apply both patches to do the checks or do > I need to reg test each one separately? > > Thanks Hey James, They should be applied separately - 5038042 fixes 1881, and 5057041 prunes down bloated code. There is a chance that 5057041 is effected by 5038042 (I haven't tested them together yet) though I doubt it. After their countdowns, I'd push 5038042 first, rerun regtests on 5057041, and then either push 5057041 or modify it if necessary. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fixes issue 1881 (cyclic dependency with beam calculations) (issue 5038042)
On 17 September 2011 12:16, m...@apollinemike.com wrote: > They should be applied separately - 5038042 fixes 1881, and 5057041 prunes > down bloated code. There is a chance that 5057041 is effected by 5038042 (I > haven't tested them together yet) though I doubt it. After their countdowns, > I'd push 5038042 first, rerun regtests on 5057041, and then either push > 5057041 or modify it if necessary. I wouldn't bother with a countdown for this patch. It's simple enough to apply immediately. Cheers, Neil ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Prunes stem::length down to the bare minimum. (issue 5057041)
http://codereview.appspot.com/5057041/diff/1/scm/output-lib.scm File scm/output-lib.scm (right): http://codereview.appspot.com/5057041/diff/1/scm/output-lib.scm#newcode82 scm/output-lib.scm:82: "stem::length called but will not be used for beamed stem." remove full stop/period Have you found a case where this will be triggered? If so, it still needs to return a sane value rather than unspecified. http://codereview.appspot.com/5057041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Prunes stem::length down to the bare minimum. (issue 5057041)
On 2011/09/17 11:38:31, Neil Puttock wrote: http://codereview.appspot.com/5057041/diff/1/scm/output-lib.scm File scm/output-lib.scm (right): http://codereview.appspot.com/5057041/diff/1/scm/output-lib.scm#newcode82 scm/output-lib.scm:82: "stem::length called but will not be used for beamed stem." remove full stop/period Have you found a case where this will be triggered? If so, it still needs to return a sane value rather than unspecified. No, but you're right that it should return something. I'll have it return 0. Cheers, MS http://codereview.appspot.com/5057041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Terminates outside_staff_callback early if a grob is outside a slur's X-extent (issue 5056041)
I'm getting a lot of errors in the make check for example: /home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.log @@ -11,13 +11,13 @@ c8 cis'' -/home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.ly:11:2: programming error: cyclic dependency: calculation-in-progress encountered for #'quantized-positions (Beam) - - c8 cis'' -/home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.ly:11:2: continuing, cross fingers +/home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.ly:11:5: programming error: cyclic dependency: calculation-in-progress encountered for #'Y-extent (Stem) + c8 + cis'' +/home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.ly:11:5: continuing, cross fingers - - c8 cis'' + c8 + cis'' /home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.ly:12:2: programming error: cyclic dependency: calculation-in-progress encountered for #'quantized-positions (Beam) --snip-- This seems to be the general error. James http://codereview.appspot.com/5056041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Terminates outside_staff_callback early if a grob is outside a slur's X-extent (issue 5056041)
On 2011/09/17 11:55:51, J_lowe wrote: I'm getting a lot of errors in the make check for example: /home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.log @@ -11,13 +11,13 @@ c8 cis'' -/home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.ly:11:2: programming error: cyclic dependency: calculation-in-progress encountered for #'quantized-positions (Beam) - - c8 cis'' -/home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.ly:11:2: continuing, cross fingers +/home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.ly:11:5: programming error: cyclic dependency: calculation-in-progress encountered for #'Y-extent (Stem) + c8 + cis'' +/home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.ly:11:5: continuing, cross fingers - - c8 cis'' + c8 + cis'' /home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.ly:12:2: programming error: cyclic dependency: calculation-in-progress encountered for #'quantized-positions (Beam) --snip-- This seems to be the general error. James I'd hold fire on the regtest checking until Mike's applied the patch for issue #1881 (that's where these errors come from). Cheers, Neil http://codereview.appspot.com/5056041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Terminates outside_staff_callback early if a grob is outside a slur's X-extent (issue 5056041)
On Sep 17, 2011, at 1:55 PM, pkx1...@gmail.com wrote: > I'm getting a lot of errors in the make check for example: > > > /home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.log > > > @@ -11,13 +11,13 @@ > > > c8 cis'' > -/home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.ly:11:2: > programming error: cyclic dependency: calculation-in-progress > encountered for #'quantized-positions (Beam) > - > - c8 cis'' > -/home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.ly:11:2: > continuing, cross fingers > +/home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.ly:11:5: > programming error: cyclic dependency: calculation-in-progress > encountered for #'Y-extent (Stem) > + c8 > + cis'' > +/home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.ly:11:5: > continuing, cross fingers > > - > - c8 cis'' > + c8 > + cis'' > > /home/jlowe/lilypond-git/input/regression/spacing-correction-accidentals.ly:12:2: > programming error: cyclic dependency: calculation-in-progress > encountered for #'quantized-positions (Beam) > > --snip-- > > This seems to be the general error. > > James > > http://codereview.appspot.com/5056041/ James, If you git pull and rerun, it should be clean now. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fixes issue 1881 (cyclic dependency with beam calculations) (issue 5038042)
On Sep 17, 2011, at 1:28 PM, Neil Puttock wrote: > On 17 September 2011 12:16, m...@apollinemike.com > wrote: > >> They should be applied separately - 5038042 fixes 1881, and 5057041 prunes >> down bloated code. There is a chance that 5057041 is effected by 5038042 (I >> haven't tested them together yet) though I doubt it. After their >> countdowns, I'd push 5038042 first, rerun regtests on 5057041, and then >> either push 5057041 or modify it if necessary. > > I wouldn't bother with a countdown for this patch. It's simple enough > to apply immediately. > > Cheers, > Neil Pushed as 47f5cb2e80eca506ff4897b7628129597a60f5cb. Sorry for the critical regression from my work and thanks for your patience while I fixed it! Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: lilypond.org can't build website
- Original Message - From: "Graham Percival" To: "Phil Holmes" Cc: Sent: Friday, September 16, 2011 8:02 PM Subject: Re: lilypond.org can't build website On Fri, Sep 16, 2011 at 07:09:53PM +0100, Phil Holmes wrote: - Original Message - From: "Graham Percival" >>As you say, an alternative that _should_ work is to define >>PYTHONPATH in website.make. Would need to test that, though, and >>off rehearsing in about 10 minutes. > >I'd rather go that route. I'll add an issue so we don't forget. OK. Probably we should also copy langdefs.py to your "safe" scripts directory, too? oh, good point, since we're running that file. ick, this is getting messy. Cheers, - Graham Patch attached which runs on a standalone website build on my system. I believe this should fix your website build problem. If it's OK, I'll update the CG for the local build instructions to take the PYTHONPATH bit out. A further comment on the "copy the langdefs" script to safe-scripts. If we do that, we'll need to amend website.make again... -- Phil Holmes 0001-Adds-PYTHONPATH-to-website.make.patch Description: Binary data ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Fix 155. (issue 5032047)
Passes make and reg tests http://codereview.appspot.com/5032047/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fix 155. (issue 5032047)
Am Saturday, 17. September 2011, 14:30:23 schrieb pkx1...@gmail.com: > Passes make and reg tests > > http://codereview.appspot.com/5032047/ It would be really nice to have a better summary than "Fix 155". This does not tell me ANYTHING about what this is all about, so I'll have to click on the link to find out what this patch is for (and either remember this, or have to visit the rietveld page again when I can't remember what all the lilypond bug numbers are about). Or of course I simply ignore that mail, which is usually what I do. So, please add a short summary to the patch summary! This is also preferred for the actual git commit, since having a patch summary of "Fix 155" is just as un-helpful in the the git history as it is here. 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 https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Progress on loose columns. (issue 4841052)
passes make and reg tests I get some changes but they look ok - although one (spacing-strict-notespacing.ly) does show the bass clef colliding 'more' than it did before. All reg tests that showed up are attached here : http://code.google.com/p/lilypond/issues/detail?id=1301#c6 http://codereview.appspot.com/4841052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Progress on loose columns. (issue 4841052)
On Sep 17, 2011, at 2:49 PM, pkx1...@gmail.com wrote: > > > passes make and reg tests I get some changes but they look ok - although > one (spacing-strict-notespacing.ly) does show the bass clef colliding > 'more' than it did before. All reg tests that showed up are attached > here : > > http://code.google.com/p/lilypond/issues/detail?id=1301#c6 > > http://codereview.appspot.com/4841052/ I've spotted this change throughout my work on this patch. I'm not exactly sure how strict note spacing plays with loose columns and if this somehow makes collisions inevitable - if so, this doesn't bug me, but if not, I'll work some more on the patch. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Terminates outside_staff_callback early if a grob is outside a slur's X-extent (issue 5056041)
passes make and reg tests. James http://codereview.appspot.com/5056041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Prunes stem::length down to the bare minimum. (issue 5057041)
Passes make and reg tests http://codereview.appspot.com/5057041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add support for custom ledger positions, using two new staff-symbol properties (issue 4974075)
Passes make and reg tests http://codereview.appspot.com/4974075/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Work on 1890: Turn some int vars/funcs into vsize to prevent conversion warnings on 64-bit systems (issue 5039043)
passes make and reg tests http://codereview.appspot.com/5039043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
musicxml2ly: proposals concerning tagline, conversion info, -element and midi-output
Hey all,
I'd like to change (and add) some minor things in musicxml2ly, but before I
make a patch I wanted to make sure that no-one objects:
1) tagline: musicxml2ly currently uses the contents of the -element
for the LilyPond-tagline. So the tagline at the end of a piece engraved by
LilyPond actually contains information as to the encoding software of the
.xml-file. As this information is also saved in a custom LilyPond-variable
named "encodingsoftware" and the converted file is actually engraved by
LilyPond I'd suggest to comment out/delete the following line in musicxml2ly in
order to print out the standard LilyPond-tagline (Music engraving by
LilyPond...):
set_if_exists ('tagline', ids.get_encoding_software ())
2) conversion info: right now the conversion info at the top of the generated
.ly-file says e.g.:
% automatically converted from filename
I'd like to change that to:
% automatically converted with musicxml2ly from filename
3) -element: musicxml2ly currently doesn't know the -element.
I'd like to add it.
4) MIDI: musicxml2ly currently automatically comments out the midi-output in
the generated .ly-files. Why not leave it in by default?
So what's your opinion?
Cheers
patrick
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
[PATCH] Fixing some docs build warnings
I'm currentlly trying to clean up some build warnings. 1) The xref-map generation didn't use the same includes as texi2html, so we got several warnings about files not found. The following patch should fix those: http://codereview.appspot.com/5038044 2) A normal "make" tries to load the translations for the gettext domain "lilypond-doc" from the directory /home/reinhold/lilypond/lilypond/Documentation/po/out-www As they are not created during a normal build, we get one warning for each file processed: langdefs.py: warning: lilypond-doc gettext domain not found. Can't we add the lilypond-doc gettext creation to the begin of a normal build, too? 3) A normal build also tries to create several info files with images in pictures, for which we do not have any *.txt files, resulting in warnings: ./contributor/programming-work.itexi:67: Warnung: @image-Datei „lilypond/pictures/architecture-diagram.txt“ (für Text) nicht lesbar: Datei oder Verzeichnis nicht gefunden. ./contributor/programming-work.itexi:67: Warnung: @image-Datei „lilypond/pictures/architecture-diagram.txt“ (für Text) nicht lesbar: Datei oder Verzeichnis nicht gefunden. First, why do we hardcode the lilypond/ directory? AFAICS, we don't create or use that directory at all. All snippets link their images to something a0/... and not to lilypond/a0/... Only explicit image links have lilypond/ hardcoded in the macro. And second, should we create a .txt file for each static .png image we use, so makinfo is happy? 4) The snippets texinfo has the problem that each snippets adds a @node with its name. If a snippet is included twice, we have duplicated nodes. Also, in each snippet category, we do not create any menu, but create a node for each snippet, so we have lots of notes that are not inside any menu. Any idea how to proceed with those? 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 https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: musicxml2ly: proposals concerning tagline, conversion info, -element and midi-output
Am Saturday, 17. September 2011, 16:50:33 schrieben Sie:
> I'd like to change (and add) some minor things in musicxml2ly, but before I
> make a patch I wanted to make sure that no-one objects:
>
> 1) tagline: musicxml2ly currently uses the contents of the
> -element for the LilyPond-tagline. So the tagline at the end of
> a piece engraved by LilyPond actually contains information as to the
> encoding software of the .xml-file. As this information is also saved in a
> custom LilyPond-variable named "encodingsoftware" and the converted file
> is actually engraved by LilyPond I'd suggest to comment out/delete the
> following line in musicxml2ly in order to print out the standard
> LilyPond-tagline (Music engraving by LilyPond...):
>
> set_if_exists ('tagline', ids.get_encoding_software ())
Okay.
> 2) conversion info: right now the conversion info at the top of the
> generated .ly-file says e.g.:
>
> % automatically converted from filename
>
> I'd like to change that to:
>
> % automatically converted with musicxml2ly from filename
Sounds fine.
> 3) -element: musicxml2ly currently doesn't know the
> -element. I'd like to add it.
What would be the conversion result of a element?
> 4) MIDI: musicxml2ly currently automatically comments out the midi-output
> in the generated .ly-files. Why not leave it in by default?
If the user is interested, he can always add \midi{} himself. By default,
lilypond doesn't create midi, so it makes sense to me to not add midi by
default in musicxml2ly, either.
You might add a cmd line option to create a midi block (and a layout block to
make sure a pdf is created, too).
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
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improves horizontal spacing of axis groups that SpanBar grobs traverse. (issue 4917046)
On Sep 9, 2011, at 7:09 PM, tdanielsmu...@googlemail.com wrote:
> Hi Mike
> I found a visual difference with one of my scores. I finally tracked
> this down to a dependence on the stem direction. Try this:
>
> SopranoMusic = \relative g' {
> b2 b
> \once \override Staff . BarLine #'allow-span-bar = ##f
> b2 b
> }
>
> TenorMusic = \relative a' {
> b2 b |
> b2 b |
> }
>
> \score {
> \new GrandStaff <<
> \new Staff {
> \new Voice = "Soprano" {
> \stemDown
> \SopranoMusic
> }
> }
> \new Lyrics \lyricsto "Soprano" {
> a b long-syllable a
> }
> \new Staff {
> \new Voice = "Tenor" {
> \TenorMusic
> }
> }
>>>
> }
>
> \score {
> \new GrandStaff <<
> \new Staff {
> \new Voice = "Soprano" {
> \stemUp
> \SopranoMusic
> }
> }
> \new Lyrics \lyricsto "Soprano" {
> a b long-syllable a
> }
> \new Staff {
> \new Voice = "Tenor" {
> \TenorMusic
> }
> }
>>>
> }
Good catch!
The problem comes not from this patch but from the calculation of the
horizontal skylines for NonMusicalPaperColumns. Try adding:
\override Score . NonMusicalPaperColumn #'stencil = #ly:separation-item::print
To the beginning of SopranoMusic and then running LilyPond with
-ddebug-skylines. You'll see that in the first example, the downward stem
pushes the lyrics under the end point of the NonMusicalPaperColumn, whereas
this does not happen in the second example.
Thus, the dependency is not stems (drop the soprano an octave and you'll see
that) but NonMusicalPaperColumn grobs.
I'd recommend fixing this in a separate patch. I also think it should be put
on the tracker, although I don't feel comfortable reporting it because I'm not
sure how to describe this bug (my gut reaction would be to say that
NonMusicalPaperColumn horizontal skylines are sometimes incorrect, but I'm not
sure if these overshoots are in fact incorrect or if they need to be there to
prevent other problems from happening, in which case the description would be
more like "Over-extended NonMusicalPaper columns must be present for X but
interfere with lyric spacing in certain cases").
So, as it has passed a countdown and I've gotten no responses saying that the
SpanBars look different with the patch applied, I'll push the SpanBarStub patch
w/in the next 24ish hours.
Cheers,
MS
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: PATCH: 48-HOur countdown to 22:00 20110918
Am Saturday, 17. September 2011, 05:35:30 schrieb Colin Campbell: > For 22: MDT Sunday Sept 18: [...] > Issue 1888: Introduce a maximum depth for markup evaluation - R 5032041 > > and the closely related > > Issue 380: Try to auto-detect cyclic references in header fields (was > 'cycling markup reference segfaults') - R 5027042: Fix 380: Auto-detect > all cyclic references in markups There's another patch for this issue: http://codereview.appspot.com/4951073/ As the general solution (R 5027042) is not able to print out nice error messages (i.e. tell the user where exactly the problem lies), this patch handles only the cyclic \fromproperty case and prints a warning that tells the user exactly which header property has a recursive definition. I'd like both patches to go in, the first, because it fixes the problem in general, and the second because it's a more user-friendly special case. What I'm still missing, though, is how I can nicely print out the contents of the recursive markup. So far, I'm only able to print the name of the markup function, but not its contents (i.e. I can only print somehting like "recursive line-markup encountered", but the user won't know which of his hundreds of markups in a score causes it). 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 https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: musicxml2ly: proposals concerning tagline, conversion info, -element and midi-output
Am 17.09.2011 um 17:01 schrieb Reinhold Kainhofer:
> Am Saturday, 17. September 2011, 16:50:33 schrieben Sie:
>> I'd like to change (and add) some minor things in musicxml2ly, but before I
>> make a patch I wanted to make sure that no-one objects:
>>
>> 1) tagline: musicxml2ly currently uses the contents of the
>> -element for the LilyPond-tagline. So the tagline at the end of
>> a piece engraved by LilyPond actually contains information as to the
>> encoding software of the .xml-file. As this information is also saved in a
>> custom LilyPond-variable named "encodingsoftware" and the converted file
>> is actually engraved by LilyPond I'd suggest to comment out/delete the
>> following line in musicxml2ly in order to print out the standard
>> LilyPond-tagline (Music engraving by LilyPond...):
>>
>>set_if_exists ('tagline', ids.get_encoding_software ())
>
> Okay.
>
>> 2) conversion info: right now the conversion info at the top of the
>> generated .ly-file says e.g.:
>>
>>% automatically converted from filename
>>
>> I'd like to change that to:
>>
>>% automatically converted with musicxml2ly from filename
>
> Sounds fine.
>
>> 3) -element: musicxml2ly currently doesn't know the
>> -element. I'd like to add it.
>
> What would be the conversion result of a element?
Michael Good uses the element for publishing information as to the
original edition. (See MusicXML 3.0 Tutorial, pp 16 and identity.mod)
I'd suggest a custom LilyPond-variable named "source" or "publisherinfo", e.g.:
\header {
publisherinfo = "(Based on) Bach-Gesellschaft Ausgabe, Band 19
Leipzig: Breitkopf & Härtel, 1871. Plate B.W. XIX."
}
>
>> 4) MIDI: musicxml2ly currently automatically comments out the midi-output
>> in the generated .ly-files. Why not leave it in by default?
>
> If the user is interested, he can always add \midi{} himself. By default,
> lilypond doesn't create midi, so it makes sense to me to not add midi by
> default in musicxml2ly, either.
Okay.
>
> You might add a cmd line option to create a midi block (and a layout block to
> make sure a pdf is created, too).
Sounds good! I'll see what I can do.
Thanks,
patrick
>
> 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
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: PATCH: 48-HOur countdown to 22:00 20110918
On Sep 17, 2011, at 5:21 PM, Reinhold Kainhofer wrote: > Am Saturday, 17. September 2011, 05:35:30 schrieb Colin Campbell: >> For 22: MDT Sunday Sept 18: > [...] >> Issue 1888: Introduce a maximum depth for markup evaluation - R 5032041 >> >> and the closely related >> >> Issue 380: Try to auto-detect cyclic references in header fields (was >> 'cycling markup reference segfaults') - R 5027042: Fix 380: Auto-detect >> all cyclic references in markups > > There's another patch for this issue: > http://codereview.appspot.com/4951073/ > > As the general solution (R 5027042) is not able to print out nice error > messages (i.e. tell the user where exactly the problem lies), this patch > handles only the cyclic \fromproperty case and prints a warning that tells > the > user exactly which header property has a recursive definition. > > I'd like both patches to go in, the first, because it fixes the problem in > general, and the second because it's a more user-friendly special case. > > What I'm still missing, though, is how I can nicely print out the contents of > the recursive markup. So far, I'm only able to print the name of the markup > function, but not its contents (i.e. I can only print somehting like > "recursive line-markup encountered", but the user won't know which of his > hundreds of markups in a score causes it). > I've been toying with the idea for some time now of making markups at all levels behave more like grobs. It would require a massive code rewrite, but it'd allow a much more uniform approach to exactly this sort of thing (markups could, for example, issue errors like grobs, which gives the user the place in the code where the bad markup is). It'd also allow for some code-duping in the footnote code to go away and would allow for object-oriented references between markups (which would help with spacing, advanced inter-markup communication for layout stuff, etc.). Would anyone be interested in co-taking this on? Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: lilypond.org can't build website
On Sat, Sep 17, 2011 at 01:27:24PM +0100, Phil Holmes wrote: > > Patch attached which runs on a standalone website build on my > system. I believe this should fix your website build problem. That patch hard-codes the lilypond-git directory unnecessarily; we have already defined TOP_SRC_DIR, so why not use $TOP_SRC_DIR/python ? > A further comment on the "copy the langdefs" script to safe-scripts. > If we do that, we'll need to amend website.make again... oops, I forgot about that. So actually we don't want to point to TOP_SRC_DIR/python/ at all, since that's untrusted material. ... I can't shake the feeling that we've taken a wrong turn somewhere. The original motivation behind "avoid hard-coding/duplicating the list of languages" was to simplify the build process, right? But all this "run a python script with an argument to get a list of languages" stuff seems to be un-simple. I'm wondering if a radically different approach would be good. What about a plain old text file? If we had a languages.txt (just like VERSION), we could cat that file in website.make with no security concerns. And langdefs.py could read that file with no problems. Or... do we need an actual hard-coded list of languages at all? What if we just make a list from all two-character sub-directories of Documentation/ ? actually, maybe that's a silly idea; updating a single list of languages in one place isn't hard, and that could avoid some potential problems if somebody wanted to make a non-language two-character subdirectory of Documentation/ ... ok, I'm rambling (I have a mild fever), and this isn't helping to solve the immediate problem. 1. amend the CG page to include langdefs.py in the "trusted scripts" stuff. I'll copy that over. 2. set PYTHONPATH to be $TRUSTED_SCRIPTS_DIR (or whatever it's called) in website.make. I'm going to set up a git repo for lilypond-web-media -- not on savannah right now, just on github until the relevant GOP-PROP is officially passed -- and take an initial gander at setting that up using the LILYPOND_GIT and LILYPOND_WEB_MEDIA_GIT environment variables. Once that's done, it should be possible to vastly simplify the CG page and maybe even website.make, and then it should be much easier to actually see all the bits and pieces that need to go together, and then it should be easier to simplify the whole thing. "everything should be as simple as possible, but no simpler" - somebody (maybe Einstien)? Cheers, - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: lilypond.org can't build website
- Original Message - From: "Graham Percival" To: "Phil Holmes" Cc: Sent: Saturday, September 17, 2011 5:15 PM Subject: Re: lilypond.org can't build website On Sat, Sep 17, 2011 at 01:27:24PM +0100, Phil Holmes wrote: Patch attached which runs on a standalone website build on my system. I believe this should fix your website build problem. That patch hard-codes the lilypond-git directory unnecessarily; we have already defined TOP_SRC_DIR, so why not use $TOP_SRC_DIR/python ? Good point. I've slightly lost track of who's doing what on this now, though, so I'll not change my local website.make. A further comment on the "copy the langdefs" script to safe-scripts. If we do that, we'll need to amend website.make again... oops, I forgot about that. So actually we don't want to point to TOP_SRC_DIR/python/ at all, since that's untrusted material. I think this is your point 1. below. I can't shake the feeling that we've taken a wrong turn somewhere. The original motivation behind "avoid hard-coding/duplicating the list of languages" was to simplify the build process, right? But all this "run a python script with an argument to get a list of languages" stuff seems to be un-simple. I'm wondering if a radically different approach would be good. What about a plain old text file? If we had a languages.txt (just like VERSION), we could cat that file in website.make with no security concerns. And langdefs.py could read that file with no problems. Or... do we need an actual hard-coded list of languages at all? What if we just make a list from all two-character sub-directories of Documentation/ ? actually, maybe that's a silly idea; updating a single list of languages in one place isn't hard, and that could avoid some potential problems if somebody wanted to make a non-language two-character subdirectory of Documentation/ I must admit to finding langdefs.py rather over-kill for what we actually use it for. It seems to me the functional definition is quite simple - return a list of language abbreviations to either a make file or a python file which needs it. However, we do need to ensure that 2 lists are available: web and doc. Note Reinhold's mail about the errors from langdefs. Making this simpler would stop those, too. ... ok, I'm rambling (I have a mild fever), and this isn't helping to solve the immediate problem. I'm struggling with a visual migraine, so a bit ditto. 1. amend the CG page to include langdefs.py in the "trusted scripts" stuff. I'll copy that over. 2. set PYTHONPATH to be $TRUSTED_SCRIPTS_DIR (or whatever it's called) in website.make. See above - you gonna do this? -- Phil Holmes ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fix 155. (issue 5032047)
On Sat, Sep 17, 2011 at 02:36:04PM +0200, Reinhold Kainhofer wrote: > It would be really nice to have a better summary than "Fix 155". This does > not > tell me ANYTHING about what this is all about, Agreed. James, please take another look at: http://lilypond.org/doc/v2.15/Documentation/contributor/patch-handling No patch should get "patch-review" unless it has a descriptive subject line. By all means test to see if a patch has any regtest changes, but if the subject is not good, make a note of that and explain that you cannot mark it -review for that reason. Cheers, - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improves some parmesan noteheads. (issue 4639065)
Pushed as: 0dcc93c0a5a97d048db2f7631966f41ae0059ab5 and 0e31cd90e44673eca7ac59705ce4bed33dd8e80e Thank you all for this review! Bertrand http://codereview.appspot.com/4639065/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: lilypond.org can't build website
On Sat, Sep 17, 2011 at 05:39:35PM +0100, Phil Holmes wrote: > > > >1. amend the CG page to include langdefs.py in the "trusted > >scripts" stuff. I'll copy that over. > >2. set PYTHONPATH to be $TRUSTED_SCRIPTS_DIR (or whatever it's > >called) in website.make. > > See above - you gonna do this? sure, I only just got to university, and I'm looking for a simple task with immediate dopamine payoff. Cheers, - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
doc build: Use all includes for texinfo also for the xref-map generation (issue 5038044)
LGTM http://codereview.appspot.com/5038044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [PATCH] Fixing some docs build warnings
On Sat, Sep 17, 2011 at 04:53:00PM +0200, Reinhold Kainhofer wrote: > 2) A normal "make" tries to load the translations for the gettext domain > "lilypond-doc" from the directory > /home/reinhold/lilypond/lilypond/Documentation/po/out-www > As they are not created during a normal build, we get one warning for each > file processed: > > langdefs.py: warning: lilypond-doc gettext domain not found. > > Can't we add the lilypond-doc gettext creation to the begin of a normal > build, > too? Would they be built in out/ or out-www/ ? I have no objection to either. Oh, and you might need to build these after building stuff in python/ or scripts/ , instead of doing them at the very beginning. > 3) A normal build also tries to create several info files with images in > pictures, for which we do not have any *.txt files, resulting in warnings: > > ./contributor/programming-work.itexi:67: Warnung: @image-Datei > „lilypond/pictures/architecture-diagram.txt“ (für Text) nicht lesbar: Datei > oder Verzeichnis nicht gefunden. > ./contributor/programming-work.itexi:67: Warnung: @image-Datei > „lilypond/pictures/architecture-diagram.txt“ (für Text) nicht lesbar: Datei > oder Verzeichnis nicht gefunden. > > First, why do we hardcode the lilypond/ directory? AFAICS, we don't create or > use that directory at all. All snippets link their images to something a0/... > and not to lilypond/a0/... Only explicit image links have lilypond/ hardcoded > in the macro. I recall that there was some weirdness with makeinfo. Documentation/lilypond is a symlink to ./out-www , but when I tried to remove it 3-5 years ago, some info people complained. IIRC it was John Mandereau, who was working on displaying the images in info in emacs? > And second, should we create a .txt file for each static .png image we use, > so > makinfo is happy? If that's required to make makeinfo, then I guess so. > 4) The snippets texinfo has the problem that each snippets adds a @node with > its name. If a snippet is included twice, we have duplicated nodes. Also, in > each snippet category, we do not create any menu, but create a node for each > snippet, so we have lots of notes that are not inside any menu. Any idea how > to proceed with those? The only thing that comes to mind is asking on bug-texinfo, sorry. Cheers, - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Markup implementation
Le 17 sept. 2011 à 17:41, m...@apollinemike.com a écrit : > > I've been toying with the idea for some time now of making markups at all > levels behave more like grobs. It would require a massive code rewrite, but > it'd allow a much more uniform approach to exactly this sort of thing > (markups could, for example, issue errors like grobs, which gives the user > the place in the code where the bad markup is). It'd also allow for some > code-duping in the footnote code to go away and would allow for > object-oriented references between markups (which would help with spacing, > advanced inter-markup communication for layout stuff, etc.). > > Would anyone be interested in co-taking this on? Hi Mike, Just writing quickly, I hope I don't write nonsense. I see several problems wrt to markups: 1) the markup / markup-list duality sucks. I tend to implement every markup command twice (with its markup-lines counterparts). ==> markup and markuplines should be unified, somewhat. 2) For toplevel markups, there is no way to change the padding or spacing of a single markup. This is only possible globally at \paper level. Some markups need to be attached to the score before, some to the score after, other should stick together (away from surrounding scores), etc. ==> it shall be possible to set some characteristics to toplevel markups (e.g. before/after padding and spacing stuff.) 3) mixing several lines of text and music in a single paragraph is just not possible. One or two years ago, someone provided an implementation sketch, where a markup is given some extra context when interpreted, to figure out where on the current line it appears. I'm sorry I can't remember his name. This may be the inter-markup communication thing you're talking about? Currently, the internal representation of a markup is a simple list: (markup-command arg1 arg2 ...) And a markup a interpreted by applying the markup-command on the args (plus the layout and properties special arguments). This is not expressive enough. A more elaborate data structure shall be used. A first step could be to move the internal representation of markups to C++, like grobs, music expressions, etc, with similar accessors. At this point the functionnality would be unchanged. Then we could add some logics to deal with the mentionned problems. But this pass most probably leads to backward incompatibility at some point. Nicolas ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Markup implementation
Hi Nicolas This is very interesting. I always wanted to fix these issues. Tell me if I can do something to help this work. (I'm also ready to give at least 100€ to the one that would fix this.) Bertrand ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: lilypond.org can't build website
Am Saturday, 17. September 2011, 18:39:35 schrieb Phil Holmes: > I must admit to finding langdefs.py rather over-kill for what we actually > use it for. It seems to me the functional definition is quite simple - > return a list of language abbreviations to either a make file or a python > file which needs it. However, we do need to ensure that 2 lists are > available: web and doc. > > Note Reinhold's mail about the errors from langdefs. Making this simpler > would stop those, too. Actually, in lilypond-book, langdefs is used to load all languages so they are available in lilypond-book. The problem is that the languages are not built before they are used in a normal build, so I don't think the problem I reported can be solved in this way. 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 https://lists.gnu.org/mailman/listinfo/lilypond-devel
Fix 155: parentheses include accidentals and dots. (issue 5047048)
Hi Joe, At last, a fix for that! But this looks unfinished. Are you working on a solution that would avoid having to specify the X-extent? A fix that calculates the font size of the ParenthesesItem according to the Y-extent of the parenthesized grobs would also be awesome :) Bertrand http://codereview.appspot.com/5047048/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improves horizontal spacing of axis groups that SpanBar grobs traverse. (issue 4917046)
http://codereview.appspot.com/4917046/diff/18001/input/regression/span-bar-allow-span-bar.ly
File input/regression/span-bar-allow-span-bar.ly (right):
http://codereview.appspot.com/4917046/diff/18001/input/regression/span-bar-allow-span-bar.ly#newcode2
input/regression/span-bar-allow-span-bar.ly:2: \version "2.15.10"
2.15.12
http://codereview.appspot.com/4917046/diff/18001/input/regression/span-bar-allow-span-bar.ly#newcode5
input/regression/span-bar-allow-span-bar.ly:5: texidoc = "The
SpanBarStub grob takes care of horizontal spacing for
@code{SpanBarStub}
http://codereview.appspot.com/4917046/diff/18001/input/regression/span-bar-allow-span-bar.ly#newcode6
input/regression/span-bar-allow-span-bar.ly:6: SpanBar grobs. When the
SpanBar is disallowed, objects in contexts that
@code{SpanBar}
http://codereview.appspot.com/4917046/diff/18001/input/regression/span-bar-allow-span-bar.ly#newcode14
input/regression/span-bar-allow-span-bar.ly:14: \repeat unfold 64 { c''8
} }
fix indentation
http://codereview.appspot.com/4917046/diff/18001/lily/grob.cc
File lily/grob.cc (right):
http://codereview.appspot.com/4917046/diff/18001/lily/grob.cc#newcode617
lily/grob.cc:617: g->programming_error ("could not find this grob's
vertical axis group in the vertical alignment.");
remove full stop/period
http://codereview.appspot.com/4917046/diff/18001/lily/grob.cc#newcode627
lily/grob.cc:627: g1->programming_error ("grob does not belong to a
Vertical Alignment?");
VerticalAlignment
http://codereview.appspot.com/4917046/diff/18001/lily/grob.cc#newcode643
lily/grob.cc:643: g1->programming_error ("could not situate this grob in
its axis group");
prefer `place' to `situate'
http://codereview.appspot.com/4917046/diff/18001/lily/pure-from-neighbor-engraver.cc
File lily/pure-from-neighbor-engraver.cc (right):
http://codereview.appspot.com/4917046/diff/18001/lily/pure-from-neighbor-engraver.cc#newcode46
lily/pure-from-neighbor-engraver.cc:46: pure_relevant_p_ =
ly_lily_module_constant ("pure-relevant?");
you only use this once, so I'd get rid of it
http://codereview.appspot.com/4917046/diff/18001/lily/pure-from-neighbor-engraver.cc#newcode52
lily/pure-from-neighbor-engraver.cc:52: if (to_boolean (scm_apply_1
(pure_relevant_p_, i.item ()->self_scm (), SCM_EOL))
scm_call_1 ?
http://codereview.appspot.com/4917046/diff/18001/lily/pure-from-neighbor-engraver.cc#newcode53
lily/pure-from-neighbor-engraver.cc:53: && !i.grob
()->internal_has_interface (ly_symbol2scm
("pure-from-neighbor-interface")))
swap
http://codereview.appspot.com/4917046/diff/18001/lily/pure-from-neighbor-engraver.cc#newcode57
lily/pure-from-neighbor-engraver.cc:57: // note that this can get outta
hand if there are lots of vertical axis groups...
out of
http://codereview.appspot.com/4917046/diff/18001/lily/pure-from-neighbor-interface.cc
File lily/pure-from-neighbor-interface.cc (right):
http://codereview.appspot.com/4917046/diff/18001/lily/pure-from-neighbor-interface.cc#newcode34
lily/pure-from-neighbor-interface.cc:34: MAKE_SCHEME_CALLBACK
(Pure_from_neighbor_interface, elements_callback, 1);
this sounds like it returns elements, whereas you're just processing the
elements array
http://codereview.appspot.com/4917046/diff/18001/lily/span-bar-engraver.cc
File lily/span-bar-engraver.cc (right):
http://codereview.appspot.com/4917046/diff/18001/lily/span-bar-engraver.cc#newcode72
lily/span-bar-engraver.cc:72: Grob *vag =
Grob::get_root_vertical_alignment (bars_[0]);
is this safe?
http://codereview.appspot.com/4917046/diff/18001/lily/span-bar-stub-engraver.cc
File lily/span-bar-stub-engraver.cc (right):
http://codereview.appspot.com/4917046/diff/18001/lily/span-bar-stub-engraver.cc#newcode55
lily/span-bar-stub-engraver.cc:55: // note that this can get outta hand
if there are lots of vertical axis groups...
out of
http://codereview.appspot.com/4917046/diff/18001/lily/span-bar-stub-engraver.cc#newcode116
lily/span-bar-stub-engraver.cc:116: //it->set_parent (y_parents[j],
Y_AXIS);
remove?
http://codereview.appspot.com/4917046/diff/18001/lily/span-bar-stub-engraver.cc#newcode122
lily/span-bar-stub-engraver.cc:122: it->set_property ("Y-extent",
ly_interval2scm (Interval (infinity_f, -infinity_f)));
SCM_BOOL_F
http://codereview.appspot.com/4917046/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
[PATCH] Automatically set the eps backend in lilypond-book-preamble.ly
If a file includes lilypond-book-preamble.ly (like all of the snippets included in lilypond-book), we require the use of the eps backend anyway, so why not set it directly in the preamble rather than telling the user to set it manually? http://codereview.appspot.com/5038045 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 https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: modifying default behaviour of tremolo slashes (issue 4636081)
2011/9/16 m...@apollinemike.com : > On Sep 5, 2011, at 10:31 PM, Janek Warchoł wrote: > >> Hey Mike, >> >> 2011/8/27 : >>> http://codereview.appspot.com/4636081/diff/42001/scm/define-grobs.scm#newcode1944 >>> scm/define-grobs.scm:1944: (X-extent . ,ly:stem-tremolo::width) >>> On 2011/08/23 07:48:39, MikeSol wrote: For consistency's sake (I just went through this with flags), keep this as a style property and document the possible values in the Stem_tremolo docstring in stem-tremolo.cc (the part with ADD_INTERFACE). >>> >>> I'm not sure if i understand. Do you say that i should rename shape to >>> style? >> >> I'm not sure whether i should push this patch or not. Can you explain >> what is your concern? > > Sorry - I meant that the property name should be `style.' But there is 'style' property, only it's used for something else. Before my patch, there was 'style' property which was about the slashes being rectangular or not. My patch renames this to 'shape' property and intoduces 'style' property that does something else. So, if we keep 'style' property doing what it was doing already, how should we name the new property? And BTW, maybe i forgot to add something somewhere? Maybe it should be listed in define-grobs.scm? > Could you send me a quick update stating where you're at w/ this patch? It is ready to go except for the above. I hope this is clear now. cheers, Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improves horizontal spacing of axis groups that SpanBar grobs traverse. (issue 4917046)
On 17 September 2011 15:45, Mike Solomon wrote: > The problem comes not from this patch but from the calculation of the > horizontal skylines for NonMusicalPaperColumns. Try adding: > > \override Score . NonMusicalPaperColumn #'stencil = #ly:separation-item::print > > To the beginning of SopranoMusic and then running LilyPond with > -ddebug-skylines. You'll see that in the first example, the downward stem > pushes the lyrics under the end point of the NonMusicalPaperColumn, whereas > this does not happen in the second example. > > Thus, the dependency is not stems (drop the soprano an octave and you'll see > that) but NonMusicalPaperColumn grobs. This isn't really a bug. NonMusicalPaperColumn has a default setting for skyline-vertical-padding which extends the skyline slightly. Cheers, Neil ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Unifies mensural ligatures with blot-diameter. (issue 5030053)
Reviewers: benko.pal, Message: Hi, The new mensural style introduced with commit 0dcc93c0a5a97d048db2f7631966f41ae0059ab5 created some ugliness's in mensural ligatures. This patch fix these. I also added serifs to flexas. Regards, Bertrand Description: Unifies mensural ligatures with blot-diameter. Reverts some changes of commit 0dcc93c0a5a97d048db2f7631966f41ae0059ab5 Use blot-diameter either in C and MetaFont. Please review this at http://codereview.appspot.com/5030053/ Affected files: M lily/mensural-ligature.cc M mf/parmesan-noteheads.mf ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: change longas similarly to how breves were changed (issue 4962072)
pushed as a2d8779a847dab1b6622da59be3a0e9247ff2ab2 thanks, Janek http://codereview.appspot.com/4962072/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fix issue #1852: manuals needs more explicit dependencies. (issue 4996044)
pushed by Graham as 1b99f1907fb77b0f3a0e65725776782c3eeaa025 (thanks - i didn't have access to mi Lilydev machine lately). I close Rietveld issue. Thanks Julien! Janek http://codereview.appspot.com/4996044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Unifies mensural ligatures with blot-diameter. (issue 5030053)
Passes make and reg tests - I get differences but they look ok see: http://code.google.com/p/lilypond/issues/detail?id=1898#c1 http://codereview.appspot.com/5030053/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: adding doc strings for \...DashPattern and \harmonicBy... (1887) (issue 5019042)
Pushed as 5de09e74c407e8fd8654562df11ef48a94e8a186 and closed. Thanks, Janek http://codereview.appspot.com/5019042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New short and long lyric ties. (issue 4912041)
New patch set. After thinking about that during three weeks, I decided to remove the long tie and even reduce the length of the default tie. Janek, thank you for showing me that overshooting vowels was useless and maybe disturbing. I also changed the "è" for a "e"... Thanks, Bertrand http://codereview.appspot.com/4912041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
dev/website-build
I'm working on simplications to the website build, mainly involving a potential web-media repository on a separate branch called dev/website-build. Feel free to try it out and try to break it. For a complete build, you'll need my lilypond-extra git repository from github https://github.com/gperciva/lilypond-extra and you'll need a LILYPOND_WEB_MEDIA_GIT environment variable set up (pointing at that repo), but if you don't have anything set up, it should give you a decent error message. Anybody should feel free to push to this branch directly with no qualms or review. When the dust settles, I'll propose formal patches to master on a case-by-case basis, or something like that. Cheers, - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Message-handling: Make the Input class use the functions in warn.cc (issue 5032048)
Reviewers: ,
Message:
Please review this patch, which moves all message formatting from the
Input class to the warn.cc functions. So far, there was some duplication
in those two places.
Description:
Message-handling: Make the Input class use the functions in warn.cc
This allows us to handle all warnings/error really similar in warn.cc.
For example, we can then
simply suppress some warnings by only checking in warn.cc. This patch
makes the whole handling
much more consistent, as now everything is in warn.cc
Please review this at http://codereview.appspot.com/5032048/
Affected files:
M lily/include/input.hh
M lily/input.cc
Index: lily/include/input.hh
diff --git a/lily/include/input.hh b/lily/include/input.hh
index
91c15df3299dba369b234c1e2de9d84ff0ffb1f0..c319de0829e8c425b2c66a5b24ca77da88d91df9
100644
--- a/lily/include/input.hh
+++ b/lily/include/input.hh
@@ -62,7 +62,8 @@ public:
Input (Input const &i);
Input ();
protected:
- void print_message (int level, string s) const;
+ string message_location () const;
+ string message_string (string msg) const;
};
#include "smobs.hh"
Index: lily/input.cc
diff --git a/lily/input.cc b/lily/input.cc
index
292e06a44950fc623eab74d985faacd6f9534a39..563724584e0f2536793fcac35ff68a01f29e8007
100644
--- a/lily/input.cc
+++ b/lily/input.cc
@@ -78,21 +78,24 @@ Input::set_location (Input const &i_start, Input const
&i_end)
[file:line:column:][warning:]message
*/
-void
-Input::print_message (int level, string s) const
+string
+Input::message_string (string msg) const
{
- string location;
if (source_file_)
-::print_message (level, location_string (),
- s + "\n" + source_file_->quote_input (start_) + "\n");
+return msg + "\n" + source_file_->quote_input (start_) + "\n";
else
-::print_message (level, "", s);
+return msg;
}
+string
+Input::message_location () const
+{
+ return (source_file_) ? location_string () : "";
+}
void
Input::error (string s) const
{
- print_message (LOG_ERROR, _f ("error: %s", s));
+ ::non_fatal_error (message_string (s), message_location ());
// UGH, fix naming or usage (use non_fatal_error in most places, instead)
// exit (1);
}
@@ -101,39 +104,36 @@ void
Input::programming_error (string s) const
{
if (get_program_option ("warning-as-error"))
-error (s);
+::error (message_string (s), message_location ());
else
-{
- print_message (LOG_ERROR, _f ("programming error: %s", s));
- print_message (LOG_ERROR, _ ("continuing, cross fingers") + "\n");
-}
+::programming_error (message_string (s), message_location ());
}
void
Input::non_fatal_error (string s) const
{
- print_message (LOG_ERROR, _f ("error: %s", s));
+ ::non_fatal_error (message_string (s), message_location ());
}
void
Input::warning (string s) const
{
if (get_program_option ("warning-as-error"))
-error (s);
+::non_fatal_error (message_string (s), message_location ());
else
-print_message (LOG_WARN, _f ("warning: %s", s));
+::warning (message_string (s), message_location ());
}
void
Input::message (string s) const
{
- print_message (LOG_INFO, s);
+ ::message (message_string (s), true, message_location ());
}
void
Input::debug_output (string s) const
{
- print_message (LOG_DEBUG, s);
+ ::debug_output (message_string (s), true, message_location ());
}
string
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
[PATCH] Fix 1477: Add (ly:suppress-warning...) to suppress expected warnings
This patch adds (ly:suppress-warning ...) to suppress the given warnings from
the output (i.e. if a regtest is supposed to trigger a warning):
http://codereview.appspot.com/5037046/
I have also updates some regtests as examples to suppress expected warnings.
Usage:
#(ly:suppress-warning (_ "unterminated tie"))
#(ly:suppress-warning (_ "Two simultaneous ~a events, junking this one") "key-
change")
#(ly:suppress-warning (_ "Previous ~a event here") "key-change")
Some issues:
1) Currently, I simply collect all suppressed warnings into a list and compare
each warning to the suppressed strings (only the beginning of the real warning
message, since the Input class will append the actual file contents, too).
This approach has the disadvantage that we can't check if there is really a
warning or not.
A better approach might be to understand (ly:suppress-warning ...) for just
one warning and remove a matched string from the list of suppressed warnings.
If there are two identical warnings expected, simply call ly:suppress-warning
twice with the same message.
With that approach, one can then check at the end of a file if each expected
warning was really triggered or not. In the later case, we could print a
warning about missing warnings.
2) The suppressed warnings are currently NOT reset after each file. What's the
best way to reset them after each file? Explicitly call the clear suppressions
function in the loop lilypond-all in lily.scm? Or is there some better hook
for end-of-single-file cleanup?
3) Warnings are translated, so ly:suppress-warning also needs to be called on
the translated message, so it should be used as:
#(ly:suppress-warning (_ "message text") args)
However, many warnings are called from C++ and use the C sprintf syntax (%s
rather than ~a placeholder). suppress-warning on the other hand is a function
with SCM arguments, so we don't have immediate access to _f. My idea was to
add a second function
#(ly:suppress-c-warning "Two simultaneous %s events, junking this one" "key-
change")
That function does not get a translated string, but the original English
sprintf format string, calls ly_scm2string and then _f to translate it in the
warn-scheme.cc file. Unfortunately, I don't know an easy way to handle the
optional arguments. As the arguments can have any type, the only way I can
think of is like the following (ugly) pseudocode:
string msg;
if (argument_count == 0) {
msg = _ (str);
} else if (argument_count == 1) {
if (scm_is_string (arg1)) {
msg = _f (str, ly_scm2string (arg1));
} else if (scm_is_integer (arg1)) {
msg = _f (str, robust_scm2int (arg1));
} else if
...
} else if (argument_count == 2) {
// Here we have quadratically many different type combinations!
if (scm_is_string (arg1)) {
if (scm_is_string (arg2)) {
// format string - string
} else if (scm_is_ (arg2) {
...
} else ...
..
} else if (scm_is_integer (arg1)) {
}
} else if (argument_count == 3) {
// Doesn't really make sense effort-wise to handle this case!
}
That's of course insane, but due to the un-typedness of guile, I see no other
way to pass translate the C++ format string and the arguments from a scheme
and insert the correct arguments then in C++...
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
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improves horizontal spacing of axis groups that SpanBar grobs traverse. (issue 4917046)
Hey all,
Responses to Neil and Joe below, and a new patchset posted.
I'd like this patch to go on another countdown so that we can fully sort
out the implementation of get_root_vertical_alignment.
Cheers,
MS
http://codereview.appspot.com/4917046/diff/18001/input/regression/span-bar-allow-span-bar.ly
File input/regression/span-bar-allow-span-bar.ly (right):
http://codereview.appspot.com/4917046/diff/18001/input/regression/span-bar-allow-span-bar.ly#newcode2
input/regression/span-bar-allow-span-bar.ly:2: \version "2.15.10"
On 2011/09/17 19:01:38, Neil Puttock wrote:
2.15.12
Done.
http://codereview.appspot.com/4917046/diff/18001/input/regression/span-bar-allow-span-bar.ly#newcode5
input/regression/span-bar-allow-span-bar.ly:5: texidoc = "The
SpanBarStub grob takes care of horizontal spacing for
On 2011/09/17 19:01:38, Neil Puttock wrote:
@code{SpanBarStub}
Done.
http://codereview.appspot.com/4917046/diff/18001/input/regression/span-bar-allow-span-bar.ly#newcode6
input/regression/span-bar-allow-span-bar.ly:6: SpanBar grobs. When the
SpanBar is disallowed, objects in contexts that
On 2011/09/17 19:01:38, Neil Puttock wrote:
@code{SpanBar}
Done.
http://codereview.appspot.com/4917046/diff/18001/input/regression/span-bar-allow-span-bar.ly#newcode14
input/regression/span-bar-allow-span-bar.ly:14: \repeat unfold 64 { c''8
} }
On 2011/09/17 19:01:38, Neil Puttock wrote:
fix indentation
Done.
http://codereview.appspot.com/4917046/diff/18001/lily/grob.cc
File lily/grob.cc (right):
http://codereview.appspot.com/4917046/diff/18001/lily/grob.cc#newcode593
lily/grob.cc:593: return get_maybe_root_vertical_alignment (g, 0);
On 2011/09/02 23:27:44, joeneeman wrote:
I still think this should say
return g->get_system ()->get_vertical_alignment ();
because there are several grobs that implement Align_interface and you
want to
make sure you get the right one.
When the grobs' root vertical alignment is accessed, get_system doesn't
work yet for BarLine grobs because their ancestry along the X_AXIS has
not yet been fully established. Grobs' Y_AXIS ancestry is established
earlier in a timestep.
Try replacing this function with:
Grob*
Grob::get_root_vertical_alignment (Grob *g)
{
System *s = g->get_system ();
return s ? s->get_vertical_alignment () : 0;
}
Span bar stubs will cease to work because get_system will, for BarLines,
always return 0.
I'm not saying that your solution is impossible - I can imagine somehow
setting BarLines' X-ancestors earlier in the translation process so that
get_system always yields a system, but my preliminary attempts to
implement this in this patch are coming up short and it seems like it
will be a larger undertaking. I'd rather push this as it is and then
have a discussion about whether get_root_vertical_alignment or
get_system->get_vertical_alignment is a cleaner way to get the root
vertical alignment. I see what you're saying that there are several
grobs that implement the Align_interface, but
get_root_vertical_alignment keeps recursing until it returns the one
that has no Y_AXIS parent. I believe that all grobs save
VerticalAlignment that implement the Align_interface have Y_AXIS
parents.
http://codereview.appspot.com/4917046/diff/18001/lily/grob.cc#newcode617
lily/grob.cc:617: g->programming_error ("could not find this grob's
vertical axis group in the vertical alignment.");
On 2011/09/17 19:01:38, Neil Puttock wrote:
remove full stop/period
Done.
http://codereview.appspot.com/4917046/diff/18001/lily/grob.cc#newcode627
lily/grob.cc:627: g1->programming_error ("grob does not belong to a
Vertical Alignment?");
On 2011/09/17 19:01:38, Neil Puttock wrote:
VerticalAlignment
Done.
http://codereview.appspot.com/4917046/diff/18001/lily/grob.cc#newcode643
lily/grob.cc:643: g1->programming_error ("could not situate this grob in
its axis group");
On 2011/09/17 19:01:38, Neil Puttock wrote:
prefer `place' to `situate'
I'm officially losing my English!
Done.
http://codereview.appspot.com/4917046/diff/18001/lily/pure-from-neighbor-engraver.cc
File lily/pure-from-neighbor-engraver.cc (right):
http://codereview.appspot.com/4917046/diff/18001/lily/pure-from-neighbor-engraver.cc#newcode46
lily/pure-from-neighbor-engraver.cc:46: pure_relevant_p_ =
ly_lily_module_constant ("pure-relevant?");
On 2011/09/17 19:01:38, Neil Puttock wrote:
you only use this once, so I'd get rid of it
Done.
http://codereview.appspot.com/4917046/diff/18001/lily/pure-from-neighbor-engraver.cc#newcode52
lily/pure-from-neighbor-engraver.cc:52: if (to_boolean (scm_apply_1
(pure_relevant_p_, i.item ()->self_scm (), SCM_EOL))
On 2011/09/17 19:01:38, Neil Puttock wrote:
scm_call_1 ?
Done.
http://codereview.appspot.com/4917046/diff/18001/lily/pure-from-neighbor-engraver.cc#newcode53
lily/pure-from-neighbor-engraver.cc:53: && !i.grob
()->internal_has_interface (ly_symbol2scm
("pure-from-neighbor-interface")))
On 2011/09/17 19:01:38, Neil Puttock wrote:
swap
Done.
http://codereview.apps
