Updates to fret-diagrams

Reviewers: Carl.D.Sorensen, http://codereview.appspot.com/11857/diff/1/2 File input/regression/fret-diagrams.ly (right): http://codereview.appspot.com/11857/diff/1/2#newcode1 Line 1: \version "2.12.0" This regtest is getting quite large. Is there a logical way to split it up (eg. fret-diagrams-

Implement \eyeglasses as markup command

Could you add the command to appendix B.8.3 of the manual? And maybe change the example for the \postscript command. Otherwise, lgtm http://codereview.appspot.com/12660 ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailma

Add chordChanges capability to FretBoard grobs

lgtm http://codereview.appspot.com/24044 ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Fix key signatures with accidentals in specific octave.

lgtm, except for the description "move check_pitch_against_signature () to SCM," since ly:check-pitch-against-signature is still implemented in C++. http://codereview.appspot.com/11052 ___ lilypond-devel mailing list lilypond-devel@gnu.org http://list

Re: Fix spacing for accidentals in tied chords

http://codereview.appspot.com/28134/diff/1/3 File lily/accidental-engraver.cc (right): http://codereview.appspot.com/28134/diff/1/3#newcode431 Line 431: Grob *newa = 0; Please use a more descriptive name. Also, add a comment regarding why you need two copies of the accidental. http://codereview

Fix #670: Chained trills

lgtm http://codereview.appspot.com/32142 ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Move left-broken line-spanner check to callback.

One minor comment, otherwise lgtm http://codereview.appspot.com/32148/diff/1/7 File lily/spanner.cc (right): http://codereview.appspot.com/32148/diff/1/7#newcode426 Line 426: Spanner::after_line_breaking (SCM grob) Can you think of a name that describes what the function actually does? Like Spa

Re: Move left-broken line-spanner check to callback.

On 2009/04/11 12:31:10, Neil Puttock wrote: On 2009/04/08 20:42:51, joeneeman wrote: > http://codereview.appspot.com/32148/diff/1/7#newcode426 > Line 426: Spanner::after_line_breaking (SCM grob) > Can you think of a name that describes what the function actually does? Like

Re: Fix key signatures with accidentals in specific octave.

http://codereview.appspot.com/11052/diff/3409/2410 File scm/music-functions.scm (right): http://codereview.appspot.com/11052/diff/3409/2410#newcode1047 Line 1047: ((and (equal? ignore-octave #f) I think eq? is more appropriate here http://codereview.appspot.com/11052/diff/3409/2410#newcode1048

Re: Improve implementation of dashed slurs

Very pretty slurs! http://codereview.appspot.com/41099/diff/1021/59 File lily/bezier.cc (right): http://codereview.appspot.com/41099/diff/1021/59#newcode275 Line 275: Bezier::subdivide (Real t, Bezier &left_part, Bezier &right_part) We only use references if they are const (for clarity), so ple

First release of ly/tablature.ly

I don't know much about the issues here, but here's a review, fwiw. Of course, you'll need to update the docs also (and a regression test or 2 would be nice). http://codereview.appspot.com/67174/diff/1/2 File ly/tablature.ly (right): http://codereview.appspot.com/67174/diff/1/2#newcode205 Line

Re: Fix crash when a stencil routine is missing

http://codereview.appspot.com/83046/diff/1/3 File scm/output-ps.scm (right): http://codereview.appspot.com/83046/diff/1/3#newcode58 Line 58: (ly:all-output-backend-commands) Perhaps this could be a macro (so that you don't need to c&p for every backend). And if you add a -d option like Han-W

Improvements for the SVG backend

http://codereview.appspot.com/91075/diff/1/3 File scm/output-svg.scm (right): http://codereview.appspot.com/91075/diff/1/3#newcode140 Line 140: (match:substring match 1))) Why not just (set-attribute 'font-weight "bold")? Similarly below. http://codereview.appspot.com/91075/diff/1/3#newcode155

Re: Fix crash when a stencil routine is missing

lgtm http://codereview.appspot.com/83046 ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

New instrument name positioning in Scheme.

Just one corner case, otherwise lgtm http://codereview.appspot.com/91119/diff/1/10 File scm/output-lib.scm (right): http://codereview.appspot.com/91119/diff/1/10#newcode833 Line 833: (interval-center extent If (not (pair? live-elts)) then (interval-center extent) will be NaN, instead of 0 w

Re: New format for autobeaming rules

very nice! http://codereview.appspot.com/88155/diff/3092/2039 File ly/music-functions-init.ly (right): http://codereview.appspot.com/88155/diff/3092/2039#newcode699 Line 699: (make-music 'SequentialMusic 'void #t)) I'd feel better if this were finished. At least add FIXME so it can be easily gr

SVG backend: On-the-fly conversion of Emmentaler/Aybabtu glyphs to paths

http://codereview.appspot.com/96083/diff/1/8 File lily/pango-font.cc (right): http://codereview.appspot.com/96083/diff/1/8#newcode351 Line 351: // options. Could you please have a look at this? (after applying this patch, if you prefer). The more special cases we add for different backends, the

Re: New format for autobeaming rules

I think this is pretty much ready to commit http://codereview.appspot.com/88155/diff/3101/4032 File lily/beam-scheme.cc (right): http://codereview.appspot.com/88155/diff/3101/4032#newcode2 Line 2: beam-scheme.cc -- Retrieving beam settings could you call this beam-grouping-scheme.cc or somethin

Re: SVG backend: On-the-fly conversion of Emmentaler/Aybabtu glyphs to paths

On 2009/07/24 23:30:00, Patrick McCarty wrote: On 2009/07/21 18:43:10, joeneeman wrote: > I think it would be helpful if you gave an example or 2 of the sort of string > you expect to match here. I'm a bit worried also about the fact that you require > the attributes to be

Re: SVG backend: On-the-fly conversion of Emmentaler/Aybabtu glyphs to paths

It seems as though rietveld wants to send my code-comments and my reply-to-comments comments in two separate emails. Sorry for the noise. http://codereview.appspot.com/96083/diff/1015/20 File lily/pango-font.cc (right): http://codereview.appspot.com/96083/diff/1015/20#newcode354 Line 354: bool

SVG backend: Output a single SVG file for each page

lgtm. You can ignore my nitpicking if you disagree. http://codereview.appspot.com/105045/diff/5/1005 File scm/framework-svg.scm (right): http://codereview.appspot.com/105045/diff/5/1005#newcode60 Line 60: (dump (ec 'svg)) Just a very very minor thing: IWBN to have eo and ec closer together. For

Implement new handling for margin and line-width settings.

Could you add some regression tests (doesn't have to be in this commit) that demonstrate some of the possible combinations of margin settings? There should also be some tests that demonstrate the warnings. http://codereview.appspot.com/104085/diff/1/4 File lily/output-def.cc (right): http://cod

Use our own ~s ly:format placeholder, since guile is broken with wide chars

http://codereview.appspot.com/109070/diff/1/2 File lily/general-scheme.cc (right): http://codereview.appspot.com/109070/diff/1/2#newcode437 Line 437: replace_all (&s, "\"", "\\\""); Don't forget to escape $. Or else use single quotes rather than double quotes. http://codereview.appspot.com/1090

Re: Use our own ~s ly:format placeholder, since guile is broken with wide chars

On 2009/08/22 00:11:34, Reinhold wrote: http://codereview.appspot.com/109070/diff/1/2 File lily/general-scheme.cc (right): http://codereview.appspot.com/109070/diff/1/2#newcode437 Line 437: replace_all (&s, "\"", "\\\""); On 2009/08/21 22:59:33, joeneeman wro

Manual beaming forced directions (^[ & _[).

lgtm http://codereview.appspot.com/120060 ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: New twoside mode.

http://codereview.appspot.com/144049/diff/1/4 File ly/paper-defaults-init.ly (right): http://codereview.appspot.com/144049/diff/1/4#newcode90 Line 90: twoside = ##f On 2009/10/30 22:19:50, Neil Puttock wrote: I'm not too fond of this name, but can't think of anything better. :) I think two-si

Re: Fixes issue 786, "Extenders in lyrics stop prematurely if a single underscore is found."

http://codereview.appspot.com/150067/diff/2003/3005 File lily/extender-engraver.cc (right): http://codereview.appspot.com/150067/diff/2003/3005#newcode83 lily/extender-engraver.cc:83: } current_lyric_is_skip_ = ly_is_equal (test, scm_from_locale_string (" ")); http://codereview.appspot.com/1500

Re: Fixes issue 786, "Extenders in lyrics stop prematurely if a single underscore is found."

lgtm http://codereview.appspot.com/150067/diff/3018/4006 File lily/extender-engraver.cc (right): http://codereview.appspot.com/150067/diff/3018/4006#newcode118 lily/extender-engraver.cc:118: Moment *start_mom = column ? unsmob_moment (column->get_property ("when") ) : 0; no space between ) ) h

Add basic scheme programmable engravers. (issue181109)

Neat! http://codereview.appspot.com/181109/diff/17/21 File lily/context-scheme.cc (right): http://codereview.appspot.com/181109/diff/17/21#newcode26 lily/context-scheme.cc:26: // TODO: naming - should use now? I think ly:context-current-moment is clearer (although of course it isn't consistent

Add -dmusic-strings-to-paths and enable for SVG backend. (issue186054)

http://codereview.appspot.com/186054/diff/1/2 File lily/pango-font.cc (right): http://codereview.appspot.com/186054/diff/1/2#newcode384 lily/pango-font.cc:384: bool to_paths = get_program_option ("music-strings-to-paths"); It would be more consistent, I think, to use a global variable here. htt

Re: Add -dmusic-strings-to-paths and enable for SVG backend. (issue186054)

On 2010/01/10 06:40:56, Patrick McCarty wrote: On 2010/01/10 05:03:30, joeneeman wrote: > http://codereview.appspot.com/186054/diff/1/2 > File lily/pango-font.cc (right): > > http://codereview.appspot.com/186054/diff/1/2#newcode384 > lily/pango-font.cc:384: bool to_paths = get

Fix DynamicTextSpanner left alignment. (issue186112)

lgtm http://codereview.appspot.com/186112 ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Avoid orphan/widow lines (issue190102)

lgtm, modulo some more formatting nitpicking. If you fix the formatting and mail me the patch, I'll push it. Also, in the future, please add lilypond-devel@gnu.org to the CC list (I should have mentioned it, sorry). http://codereview.appspot.com/190102/diff/1/2 File lily/constrained-breaking.c

Re: Fix #943 (input/regression/slur-broken-trend.ly broken) (issue203054)

http://codereview.appspot.com/203054/diff/1/2 File lily/system.cc (left): http://codereview.appspot.com/203054/diff/1/2#oldcode193 lily/system.cc:193: } For vertical positioning to work, it's important that after-line-breaking be called before Page_layout_problem does its work. Can you check tha

Re: Fix #943 (input/regression/slur-broken-trend.ly broken) (issue203054)

Just for the record, I posted to lily-devel (because I wanted to attach a file and I can't seem to do that here) to point out that this patch breaks input/regression/page-spacing-rehearsal-mark.ly. http://codereview.appspot.com/203054/show ___ lilypon

Re: Fix 1112. (issue1670042)

Reviewers: Neil Puttock, Message: Thanks, fixed. I'll push after "make check" finishes... http://codereview.appspot.com/1670042/diff/1/3 File lily/constrained-breaking.cc (right): http://codereview.appspot.com/1670042/diff/1/3#newcode384 lily/constrained-breaking.cc:384: Line_details for anyth

Patch for issue #1116 (one stencil in fill-line) (issue1689041)

Are you still waiting for someone to review this? If so, here are a couple minor things: http://codereview.appspot.com/1689041/diff/2001/3001 File scm/define-markup-commands.scm (right): http://codereview.appspot.com/1689041/diff/2001/3001#newcode848 scm/define-markup-commands.scm:848: X RIGHT

Re: Optimizations for pure-height approximations. (issue1817045)

On 2010/07/14 23:15:13, lemzwerg wrote: http://codereview.appspot.com/1817045/diff/1/4 File scm/define-grob-properties.scm (right): http://codereview.appspot.com/1817045/diff/1/4#newcode1059 scm/define-grob-properties.scm:1059: Since the properties in this file are sorted alphabetically, this

Re: Optimizations for pure-height approximations. (issue1817045)

Reviewers: lemzwerg, Neil Puttock, Message: On 2010/07/15 22:04:58, Neil Puttock wrote: http://codereview.appspot.com/1817045/diff/1/2 File lily/axis-group-interface.cc (right): http://codereview.appspot.com/1817045/diff/1/2#newcode125 lily/axis-group-interface.cc:125: pure_height_cache = sc

Fix #1192. (issue1665053)

lgtm http://codereview.appspot.com/1665053/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Fix 442. (issue1888042)

Reviewers: Neil Puttock, Message: On 2010/08/04 21:57:20, Neil Puttock wrote: Hi Joe, LGTM. I'd be quite happy with this method, even though it loses a bit of flexibility in comparison with your original patch. Thanks, fixed and pushed. I don't particularly see much use for the origina

Re: Make pure-print-callbacks public (issue1983047)

While this patch looks fine to me, there are a bunch of other lists with related functionality. You may want to make some of them public too (or add public accessor functions). http://codereview.appspot.com/1983047/ ___ lilypond-devel mailing list lily

Re: Fix 1240. (issue2065041)

Reviewers: carl.d.sorensen_gmail.com, Message: Thanks. In the end, I committed this mostly as-is, but a change I made while working on 1252 allowed me to do without the dummy Paper_columns. Description: Fix 1240. Include fixed spacings in the calculation of minimum spacings. Please review thi

Re: Doc: NR 4.4.1: Rewrite. (issue2642043)

http://codereview.appspot.com/2642043/diff/1/Documentation/notation/spacing.itely File Documentation/notation/spacing.itely (right): http://codereview.appspot.com/2642043/diff/1/Documentation/notation/spacing.itely#newcode1624 Documentation/notation/spacing.itely:1624: size) will always reset al

Re: Doc: NR 4.1: Reorganize, clarify details. (issue2758042)

http://codereview.appspot.com/2758042/diff/1/Documentation/notation/spacing.itely File Documentation/notation/spacing.itely (right): http://codereview.appspot.com/2758042/diff/1/Documentation/notation/spacing.itely#newcode428 Documentation/notation/spacing.itely:428: @c TODO: Where do headers/fo

Re: Doc: NR 4.4.1: Rewrite. (issue2642043)

cent staff-like contexts should have On 2010/10/27 08:44:41, Mark Polesky wrote: On 2010/10/26 22:28:16, joeneeman wrote: > I'm not sure how precise you want to be here, but this > isn't quite true: if the upper staff, for example, has a > very low note then a lyrics line

Re: Fix 1252 by compressing page (issue3422041)

On 2010/12/02 20:52:00, Carl wrote: Here's a patch to fix issue 1252 (music overflows page) by compressing music together. It may cause collisions, but it should solve the critical overflow error. LGTM. It would still be nice to better estimate the heights (I actually have a little code fo

Fix bookpart identifier crash with page-marker. (issue3585042)

lgtm http://codereview.appspot.com/3585042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Fix Issue 1290 (issue3832046)

I don't know if it's important, but it may be worth mentioning somewhere that skyline-horizontal-padding works differently for VerticalAxisGroup and System. (Because in VerticalAxisGroup, skyline-horizontal-padding takes effect while the outside-staff-grobs are being placed). http://codereview.a

Re: Gets vertical skylines from grob stencils (issue 5626052)

Just looked at skyline-related stuff for now... http://codereview.appspot.com/5626052/diff/85004/lily/skyline.cc File lily/skyline.cc (right): http://codereview.appspot.com/5626052/diff/85004/lily/skyline.cc#newcode292 lily/skyline.cc:292: Real p1 = left->end_ * left->slope_ + left->y_intercept

layout.cc: do not draw empty boxes (issue 6450113)

lgtm http://codereview.appspot.com/6450113/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Gets vertical skylines from grob stencils (issue 5626052)

http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc File lily/axis-group-interface.cc (right): http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode651 lily/axis-group-interface.cc:651: ---/ I've experimented

Doc: clarify page-breaking variables (2735) (issue 6453137)

lgtm http://codereview.appspot.com/6453137/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Gets vertical skylines from grob stencils (issue 5626052)

On 2012/08/17 19:25:29, Keith wrote: On Fri, 17 Aug 2012 10:16:25 -0700, wrote: > > http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode780 > lily/axis-group-interface.cc:780: while (dirty); > On 2012/08/17 08:12:56, Keith wrote: >

Re: Gets vertical skylines from grob stencils (issue 5626052)

On 2012/08/21 07:42:38, Keith wrote: On 2012/08/18 10:12:00, MikeSol wrote: > \relative c'' { >\override TupletBracket #'outside-staff-priority = #1 >\override TupletNumber #'font-size = #5 >\times 2/3 { a4\trill a\trill^"foo" a\trill } > } > > I added this as a regtest. If you co

Re: layout.cc: do not draw empty boxes (issue 6450113)

On 2012/09/03 08:16:27, dak wrote: http://codereview.appspot.com/6450113/diff/10001/lily/lookup.cc File lily/lookup.cc (right): http://codereview.appspot.com/6450113/diff/10001/lily/lookup.cc#newcode172 lily/lookup.cc:172: if (b.x ().length () < blotdiameter) On 2012/09/03 07:25:53, Keith wro

Re: Approximates cross-staff slurs in VerticalAxisGroup vertical-skylines. (issue 6498077)

On 2012/09/02 06:25:58, MikeSol wrote: On 2012/09/01 23:58:37, Keith wrote: > I might have a test case for you at > http://www.mutopiaproject.org/cgibin/piece-info.cgi?id=1776 > > It seems you copy each slur into a "slur-stub", and from those keep only the > ones with "cross-staff" set. Then wh

Set up indent-tabs-mode in lexer.ll and parser.yy (issue 6551050)

lgtm http://codereview.appspot.com/6551050/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: parser.yy: remove STRING_IDENTIFIER token (issue 6542057)

lgtm http://codereview.appspot.com/6542057/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

note-collision: retain upper voice dot when merging dots (issue 6550056)

lgtm http://codereview.appspot.com/6550056/diff/1/lily/note-collision.cc File lily/note-collision.cc (right): http://codereview.appspot.com/6550056/diff/1/lily/note-collision.cc#newcode251 lily/note-collision.cc:251: dot_wipe_head = head_down; Comment, please? (eg. it would make a nice addition

Re: lily/bezier.cc: Avoid a floating point compare (issue 5121042)

lgtm http://codereview.appspot.com/5121042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Caches pure translations for full score. (issue 5349043)

looks good! I never realized this case was such a bottleneck. http://codereview.appspot.com/5349043/diff/1/lily/align-interface.cc File lily/align-interface.cc (right): http://codereview.appspot.com/5349043/diff/1/lily/align-interface.cc#newcode320 lily/align-interface.cc:320: SCM l = SCM_EOL;

grob.cc: rewrite O(n^2) algorithm in Grob::common_refpoint algorithm to O(n) (issue 5371050)

http://codereview.appspot.com/5371050/diff/1/lily/grob.cc File lily/grob.cc (right): http://codereview.appspot.com/5371050/diff/1/lily/grob.cc#newcode536 lily/grob.cc:536: c = c->dim_cache_[a].parent_; After this loop, balance will be 0, right? So the next loop won't do anything... http://coder

Re: grob.cc: rewrite O(n^2) algorithm in Grob::common_refpoint algorithm to O(n) (issue 5371050)

Ok, lgtm http://codereview.appspot.com/5371050/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Adds some info about pure properties to the CG. (issue 5364048)

http://codereview.appspot.com/5364048/diff/4001/Documentation/contributor/programming-work.itexi File Documentation/contributor/programming-work.itexi (right): http://codereview.appspot.com/5364048/diff/4001/Documentation/contributor/programming-work.itexi#newcode1836 Documentation/contributor/p

Re: Adds some info about pure properties to the CG. (issue 5364048)

On 2011/11/21 07:44:36, mike_apollinemike.com wrote: I'm going to leave it this way if that's all right with you for the purely selfish reason that I grasp these lists better in terms of what they contain. That's definitely ok with me. Thanks for taking the time to document this! http://co

Re: some comments and complaints on the code (issue 5651069)

http://codereview.appspot.com/5651069/diff/1/lily/accidental-placement.cc File lily/accidental-placement.cc (right): http://codereview.appspot.com/5651069/diff/1/lily/accidental-placement.cc#newcode211 lily/accidental-placement.cc:211: * @return A vector of Accidental_placement_entrys Please don

Re: some comments and complaints on the code (issue 5651069)

http://codereview.appspot.com/5651069/diff/1/lily/accidental-placement.cc File lily/accidental-placement.cc (right): http://codereview.appspot.com/5651069/diff/1/lily/accidental-placement.cc#newcode211 lily/accidental-placement.cc:211: * @return A vector of Accidental_placement_entrys On 2012/02

Re: Gets vertical skylines from grob stencils (issue 5626052)

http://codereview.appspot.com/5626052/diff/23001/lily/axis-group-interface.cc File lily/axis-group-interface.cc (right): http://codereview.appspot.com/5626052/diff/23001/lily/axis-group-interface.cc#newcode659 lily/axis-group-interface.cc:659: vector *riders) I don't understand why riders are ne

Re: Gets vertical skylines from grob stencils (issue 5626052)

wrote: On 2012/02/16 08:09:10, joeneeman wrote: > I don't understand why riders are necessary. Is it because of this cyclic > dependence stuff? Not cyclical, but imagine that a grob (say tuplet bracket, for example) has its outside staff priority set whereas one of its Y_AXIS ch

Re: Gets vertical skylines from grob stencils (issue 5626052)

http://codereview.appspot.com/5626052/diff/30003/lily/skyline.cc File lily/skyline.cc (right): http://codereview.appspot.com/5626052/diff/30003/lily/skyline.cc#newcode393 lily/skyline.cc:393: Skyline::Skyline (Building b, Real start, Axis horizon_axis, Direction sky) This isn't quite what I had

Re: Gets vertical skylines from grob stencils (issue 5626052)

http://codereview.appspot.com/5626052/diff/34001/lily/skyline.cc File lily/skyline.cc (right): http://codereview.appspot.com/5626052/diff/34001/lily/skyline.cc#newcode362 lily/skyline.cc:362: result.push_front (Building (last_end, -infinity_f, -infinity_f, iv[LEFT] - 2 * horizon_padding)); push_

Re: lilypond-book: Set include path for --output option (issue 2423). (issue 5846075)

http://codereview.appspot.com/5846075/diff/1/scripts/lilypond-book.py File scripts/lilypond-book.py (right): http://codereview.appspot.com/5846075/diff/1/scripts/lilypond-book.py#newcode639 scripts/lilypond-book.py:639: global_options.include_path.insert (0, inverse_relpath (original_dir, global

Re: [XY]-core-extent and general_alignment (issue 2613) (issue 6308093)

http://codereview.appspot.com/6308093/diff/1/lily/self-alignment-interface.cc File lily/self-alignment-interface.cc (right): http://codereview.appspot.com/6308093/diff/1/lily/self-alignment-interface.cc#newcode206 lily/self-alignment-interface.cc:206: grob_alignment = scm_to_double (scm_cdr (gro

Re: Fix Issue 1290 (issue3832046)

On 2011/01/03 03:48:09, Carl wrote: On 2011/01/02 05:19:58, joeneeman wrote: > Why restrict to y_intercept_ < 0? Because if I have a y intercept that is less than zero, and create a box with that y intercept, the padded skyline comes out with all zero heights. I don't know why it wor

Re: Draws a line above footnotes (issue4167063)

http://codereview.appspot.com/4167063/diff/3018/lily/note-head.cc File lily/note-head.cc (right): http://codereview.appspot.com/4167063/diff/3018/lily/note-head.cc#newcode189 lily/note-head.cc:189: "footnote " Any particular reason this belongs to note-head-interface (rather than, say, item-inte

Re: Draws a line above footnotes (issue4167063)

On 2011/02/22 15:23:00, MikeSol wrote: On 2011/02/22 01:05:49, joeneeman wrote: > Why not make the separator a (stencil-valued) property of the paper-book? I could, but currently, this patch employs its current stencil kludge for the following (perhaps-unfounded) reason. Please let me k

Re: Reduces algorithm time by prefinding footnoted grobs (issue4213042)

I haven't had time to look at this carefully, but I'll have closer look later. What I don't understand, though, is why this problem needs such extensive changes. If it's just a matter of preventing repeated footnotes at the beginning/end of a line, it should be enough to examine footnote->get_colu

Re: Avoid repeats of 'staff-affinity' warning; change text. (issue4278058)

On 2011/03/20 07:45:11, Keith wrote: On Sun, 20 Mar 2011 00:17:48 -0700, Joe Neeman wrote: > On Sat, Mar 19, 2011 at 11:58 PM, wrote: > > In that case, is there any need to set after_affinity at all? > I could ask the author of the ori

Re: Loose-lines honor padding between systems (issue4553060)

lgtm http://codereview.appspot.com/4553060/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Spacing staves with dynamics between; issue 1442 (issue4536088)

On 2011/05/27 18:02:23, Keith wrote: Joe just pushed half of this, fixing 1442, so never mind for now. I'll merge and repost next weekend. Sorry, I didn't realize we were competing for this issue. But it seems like there's still an outstanding issue in the handling of minimum-distance together

Re: Spacing staves with dynamics between; issue 1668 (issue4536088)

lgtm http://codereview.appspot.com/4536088/diff/15003/lily/page-layout-problem.cc File lily/page-layout-problem.cc (left): http://codereview.appspot.com/4536088/diff/15003/lily/page-layout-problem.cc#oldcode543 lily/page-layout-problem.cc:543: // using a scheme similar to the one in set_column_

Re: Emit not-quite-cross-staff beams in the right context. (issue4564041)

Reviewers: Keith, Message: On 2011/05/28 18:43:19, Keith wrote: I would love to test, but don't know enough of the internals to see what this does. If you take your example from comment 5 of issue 1043, without manual beaming, the Beam's Y-parent will be the VerticalAxisGroup of the top staf

Re: Emit not-quite-cross-staff beams in the right context. (issue4564041)

Hey Joe, Lines 1207-1209 in beam.cc are a kludge that I believe your patch resolves. You may want to consider removing these lines and testing to see if it still passes regtests. If so, I think you can nix these lines permanently. After fixing my patch, I can indeed remove those lines.

Vertical spacing, distinguish stretchability/compressibility (issue4517136)

http://codereview.appspot.com/4517136/diff/1001/lily/align-interface.cc File lily/align-interface.cc (left): http://codereview.appspot.com/4517136/diff/1001/lily/align-interface.cc#oldcode230 lily/align-interface.cc:230: dy = max (dy, Page_layout_problem::get_fixed_spacing (elems[j-1], elems[j],

Re: Vertical spacing, distinguish stretchability/compressibility (issue4517136)

ote: On 2011/06/04 07:09:05, joeneeman wrote: > It seems like this loop will never see i=0, even if it is active. If springs[0] is active, the i-- above leaves i at UINT_MAX on loop exit, so ++i would be zero. I don't know a good loop idiom for a down-loop using an unsigned type (vsiz

Re: Emit not-quite-cross-staff beams in the right context. (issue4564041)

On 2011/06/01 05:59:21, Keith wrote: On 2011/05/31 08:19:42, joeneeman wrote: > the Beam's Y-parent will be the VerticalAxisGroup of the bottom staff > and Beam::calc_cross_staff will return false. Old patch didn't; but new patch does. LGTM Thanks, pushed. Also rem

Spacing with empty contexts; issue 1669 (issue4515158)

http://codereview.appspot.com/4515158/diff/2009/lily/align-interface.cc File lily/align-interface.cc (right): http://codereview.appspot.com/4515158/diff/2009/lily/align-interface.cc#newcode222 lily/align-interface.cc:222: dy = max (dy, min_distance); If pure is true, there may be some staves tha

Re: Spacing with empty contexts; issue 1669 (issue4515158)

2011/06/07 04:48:01, joeneeman wrote: > If pure is true, there may be some staves that are going to be removed. However, > these staves won't be removed until after line-breaking. ... and, as you say, these staves are not yet removed when Y-extent-estimates for the page breaker are

Re: Vertical spacing, distinguish stretchability/compressibility (issue4517136)

ok, lgtm http://codereview.appspot.com/4517136/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: modifying default behaviour of tremolo slashes (issue4636081)

Actually, non-rectangular, constant-sloped beams used to be the default, but I changed them (some years ago now) to be rectangular and parallel to the beam, since that's what most of my scores have. My music collection isn't with me right now, but I can confirm at least that B&H's urtext edition o

Re: modifying default behaviour of tremolo slashes (issue4636081)

http://codereview.appspot.com/4636081/diff/10001/lily/stem-tremolo.cc File lily/stem-tremolo.cc (right): http://codereview.appspot.com/4636081/diff/10001/lily/stem-tremolo.cc#newcode42 lily/stem-tremolo.cc:42: style = ly_symbol2scm ("constant"); You can remove these two lines and use style == ly

Re: Fix 153: \once\set properly restores the context property (issue4810042)

If you do \set #'foo = #1 c \once \set #'foo = #2 c \once \set #'foo = #3 c then the final value of foo should be 1. But with this code, I think it will be 2. http://codereview.appspot.com/4810042/ ___ lilypond-devel mailing list lilypond-devel@gnu.o

Re: modifying default behaviour of tremolo slashes (issue4636081)

wrote: On 2011/07/21 08:39:53, joeneeman wrote: > These two lines are redundant. Isn't their purpose to guard against undefined (empty) style property? If style is empty (or not a symbol) then (style == ly_symbol2scm ("varying")) will be false and you&#x

Re: Remove special case in staff-spacing (issue4188051)

http://codereview.appspot.com/4188051/diff/4001/lily/include/spacing-spanner.hh File lily/include/spacing-spanner.hh (right): http://codereview.appspot.com/4188051/diff/4001/lily/include/spacing-spanner.hh#newcode45 lily/include/spacing-spanner.hh:45: static Real cushion_; I think this should be

Re: Does better polynomial calculations for avoid objects. (issue 4860042)

http://codereview.appspot.com/4860042/diff/1/flower/polynomial.cc File flower/polynomial.cc (right): http://codereview.appspot.com/4860042/diff/1/flower/polynomial.cc#newcode65 flower/polynomial.cc:65: Polynomial::minmax (Real l, Real r, bool dir) const Perhaps "bool max" instead of "bool dir"?

Re: Does better polynomial calculations for avoid objects. (issue 4860042)

:36:45, joeneeman wrote: > If there are multiple intersections with (say) r, then Polynomial::solve doesn't > seem to return them in any useful order. So sol[RIGHT][0] is really just an > arbitrary solution, isn't it? True, but this seems no worse than line 81 where ts[0

Re: Uses Y-offset for stem tremolos instead of translated stencil. (issue 4867043)

lgtm http://codereview.appspot.com/4867043/diff/1/lily/beam.cc File lily/beam.cc (right): http://codereview.appspot.com/4867043/diff/1/lily/beam.cc#newcode1797 lily/beam.cc:1797: Beam::middle_stem_estimation (Grob *me, Direction dir) As far as I can tell, this function estimates the range of st

Re: Improves horizontal spacing of axis groups that SpanBar grobs traverse. (issue 4917046)

Correct me if I'm wrong, but this is my understanding: wherever there's a SpanBar, you're creating SpanBarStubs between every relevant pair of staves. These don't actually get printed; they're just there for the pure-height (because the SpanBar height is pretty much the whole system, so it doesn't

Re: Changes variable names in include/beam-scoring-problem.hh and beam-quanting.cc (issue 4961041)

http://codereview.appspot.com/4961041/diff/3001/lily/include/beam-scoring-problem.hh File lily/include/beam-scoring-problem.hh (right): http://codereview.appspot.com/4961041/diff/3001/lily/include/beam-scoring-problem.hh#newcode114 lily/include/beam-scoring-problem.hh:114: TODO - use trailing _

  1   2   >