lgtm
http://codereview.appspot.com/6551050/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
lgtm
http://codereview.appspot.com/6542057/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
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
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 when
On 2012/08/17 19:25:29, Keith wrote:
On Fri, 17 Aug 2012 10:16:25 -0700, mailto:mts...@gmail.com 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:
I am
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 comment
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
lgtm
http://codereview.appspot.com/6450113/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
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
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,
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));
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
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: vectorGrob * *riders)
I don't understand why riders
, MikeSol 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 children (say
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
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
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://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
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...
Ok, lgtm
http://codereview.appspot.com/5371050/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
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;
lgtm
http://codereview.appspot.com/5121042/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reviewers: J_lowe, Neil Puttock,
Message:
Thanks, all fixed.
Description:
Several fixes for annotate-spacing.
- Fixes annotate-spacing to use the new spacing names.
- Annotates spacing between staves as well as spacing between systems.
- Fixes some collisions between annotations.
- Padding
http://codereview.appspot.com/4951073/diff/3001/scm/define-markup-commands.scm
File scm/define-markup-commands.scm (right):
http://codereview.appspot.com/4951073/diff/3001/scm/define-markup-commands.scm#newcode1897
scm/define-markup-commands.scm:1897: (interpret-markup layout (cons
(list (list
http://codereview.appspot.com/4974075/diff/1/lily/ledger-line-spanner.cc
File lily/ledger-line-spanner.cc (right):
http://codereview.appspot.com/4974075/diff/1/lily/ledger-line-spanner.cc#newcode362
lily/ledger-line-spanner.cc:362: ledgers.add_stencil (ledger_line);
Since you no longer seem to
lgtm
http://codereview.appspot.com/4969069/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reviewers: lemzwerg, Neil Puttock, Reinhold,
http://codereview.appspot.com/4724041/diff/1/scm/paper-system.scm
File scm/paper-system.scm (right):
http://codereview.appspot.com/4724041/diff/1/scm/paper-system.scm#newcode112
scm/paper-system.scm:112: (ly:output-def-lookup layout
lgtm
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);
I still think this should say
return g-get_system
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 _
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
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
: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] is returned
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?
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
:
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'll just return 'default.
http://codereview.appspot.com
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 ==
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
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
BH's urtext edition
ok, lgtm
http://codereview.appspot.com/4517136/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
);
On 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 calculated
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
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],
:
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
(vsize
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 removed the 3 lines
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
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
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
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
lgtm
http://codereview.appspot.com/4553060/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
On 2011/03/20 07:45:11, Keith wrote:
On Sun, 20 Mar 2011 00:17:48 -0700, Joe Neeman
mailto:joenee...@gmail.com wrote:
On Sat, Mar 19, 2011 at 11:58 PM, mailto:k-ohara5...@oco.net
wrote:
In that case, is there any need to set after_affinity at all?
I could ask the author of the original
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
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 know
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,
:
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 works that way, but it does.
That's very strange
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).
lgtm
http://codereview.appspot.com/3585042/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
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
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
/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 with CENTER affinity
might be placed closer to the lower staff.
Also, by equidistant between the ... staves, you mean
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
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
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
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
lgtm
http://codereview.appspot.com/1665053/show
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
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
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 =
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
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
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
___
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
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
lgtm
http://codereview.appspot.com/186112
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
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_program_option
(music
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.
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 ) )
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
lgtm
http://codereview.appspot.com/120060
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
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 wrote:
Don't forget to escape $. Or else
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):
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.
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 in a specific order
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
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
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
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
lgtm
http://codereview.appspot.com/83046
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
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
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
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 cp for every
backend). And if you add a -d option like
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
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
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
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
Spanner
lgtm
http://codereview.appspot.com/32142
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
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
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.
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
lgtm
http://codereview.appspot.com/24044
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
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
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.
1 - 100 of 106 matches
Mail list logo
