https://codereview.appspot.com/36830045/diff/20001/lily/note-spacing.cc
File lily/note-spacing.cc (right):
https://codereview.appspot.com/36830045/diff/20001/lily/note-spacing.cc#newcode117
lily/note-spacing.cc:117: ret.set_inverse_stretch_strength (base_space);
Looked at it, but I have no idea
https://codereview.appspot.com/42190043/diff/1/lily/unpure-pure-container.cc
File lily/unpure-pure-container.cc (right):
https://codereview.appspot.com/42190043/diff/1/lily/unpure-pure-container.cc#newcode45
lily/unpure-pure-container.cc:45:
Looking good.
The only thing is - how do we verify
The point is to allow users to achieve the same snug skyline spacing
between stencils that is achieved between grobs.
I agree that it is inefficient as skylines are never cached, but as it
is not default beahvior anywhere in the code base, it allows people to
decide on the efficiency versus
https://codereview.appspot.com/12957047/diff/46001/lily/paper-system.cc
File lily/paper-system.cc (right):
https://codereview.appspot.com/12957047/diff/46001/lily/paper-system.cc#newcode55
lily/paper-system.cc:55: if (head == ly_symbol2scm (skyline-stencil))
On 2013/08/24 16:19:27, dak wrote:
I had time to implement everything but the delayed markup stuff. I'll
try to get to that soon...
https://codereview.appspot.com/12957047/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
It was easier to implement the delayed stuff than I expected, so I got
it done.
https://codereview.appspot.com/12957047/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM - just make sure to check this against the
dynamics-avoid-cross-staff-stem regtests.
https://codereview.appspot.com/9426044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
On 2013/04/19 12:40:43, Neil Puttock wrote:
Hi Mike,
Generally LGTM, but since there's nothing in the code which requires
C++ code I
favour adding a callback to output-lib.scm:
(define-public (semi-tie::calc-cross-staff grob)
(let* ((note-head (ly:grob-object grob 'note-head))
Hey, I hadn't seen this. I just finished writing an equivalent patch.
Yours is better, so keep it. You can use this for the
inherit-X-parent-visibility and eliminate the
inherit-Y-parent-visibility callback, which is cruft and can be replaced
with the X one.
https://codereview.appspot.com/8613043/diff/19001/Documentation/changes.tely
File Documentation/changes.tely (right):
https://codereview.appspot.com/8613043/diff/19001/Documentation/changes.tely#newcode153
Documentation/changes.tely:153: @item
I would add a note that, if there is too-snug
On 2013/04/16 07:51:46, dak wrote:
On 2013/04/16 07:40:19, Trevor Daniels wrote:
On 2013/04/16 07:20:59, dak wrote:
On 2013/04/16 06:05:42, MikeSol wrote:
I would add a note that, if there is too-snug spacing
for an object, setting its vertical skylines to #'()
will generally restore
I'd recommend adding a sort of padding property to the
self-alignment-interface to get it completely there. That is, imagine
that we right align to a grob and we want to be padded by 0.1 We should
be able to do that. That'd allow self-alignment-interface to be used
for grobs like
In general, I see a lot of reassigning of parents. What is the goal
with this (sorry if you've explained this already)?
https://codereview.appspot.com/7768043/diff/38001/lily/fingering-engraver.cc
File lily/fingering-engraver.cc (right):
On 2013/03/24 19:59:38, dak wrote:
On 2013/03/24 19:06:49, mike7 wrote:
On 24 mars 2013, at 14:59, mailto:d...@gnu.org wrote:
https://codereview.appspot.com/7574048/diff/5001/input/regression/instrument-name-x-align.ly
File input/regression/instrument-name-x-align.ly (right):
On 2013/03/27 21:50:02, janek wrote:
I see that these overrides solve the problem, but it would be nice to
know why
this works. Can you write an explanation? What was wrong with the
skylines
that caused this bug? Maybe this is just one instance of a more
general
problem.
Objects that
LGTM
https://codereview.appspot.com/7974046/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Hey all,
Following up to my comment on the tracker, I'd like to not push this
until everyone has had a chance to think through David's e-mails about
2.18. I anticipate this needing 2-3 months of user testing to catch and
fix any bugs. Given that this is my first big patch since August, it'll
The most recent patch set copies direction from SlurEvents and
PhrasingSlurEvents, but this doesn't seem to work as intended (it fails
silently). Everything else is operational.
https://codereview.appspot.com/7424049/
___
lilypond-devel mailing list
LGTM
https://codereview.appspot.com/7533046/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
https://codereview.appspot.com/7564044/diff/9001/lily/self-alignment-interface.cc
File lily/self-alignment-interface.cc (right):
https://codereview.appspot.com/7564044/diff/9001/lily/self-alignment-interface.cc#newcode130
lily/self-alignment-interface.cc:130: // TODO: should this function be
https://codereview.appspot.com/7768043/diff/19001/lily/self-alignment-interface.cc
File lily/self-alignment-interface.cc (right):
https://codereview.appspot.com/7768043/diff/19001/lily/self-alignment-interface.cc#newcode128
lily/self-alignment-interface.cc:128: /* LilyPond positions every grob
Reviewers: lemzwerg, dak, mike7,
Message:
On 2013/03/11 10:18:59, dak wrote:
On 2013/03/10 00:32:43, mike7 wrote:
Why is this override needed for the regtest? The other
overrides
are
obvious user-accessible overrides for triggering the tested
functionality.
But should
On 2013/03/13 21:38:59, thomasmorley65 wrote:
Hi Mike,
sorry to be such an inch pincher.
https://codereview.appspot.com/7615043/diff/23001/scm/output-lib.scm
File scm/output-lib.scm (right):
https://codereview.appspot.com/7615043/diff/23001/scm/output-lib.scm#newcode1061
https://codereview.appspot.com/7615043/diff/1/scm/output-lib.scm
File scm/output-lib.scm (right):
https://codereview.appspot.com/7615043/diff/1/scm/output-lib.scm#newcode929
scm/output-lib.scm:929: form. @code{x} is the portion of the width
consumed for a given line
On 2013/03/08 06:26:29,
Thanks for the review!
https://codereview.appspot.com/7310075/diff/53001/input/regression/skyline-point-extent.ly
File input/regression/skyline-point-extent.ly (right):
https://codereview.appspot.com/7310075/diff/53001/input/regression/skyline-point-extent.ly#newcode5
Some code from my patch for issue 3161 is copied over to this patch.
The fact that Grob::pure_height returns Interval (0,0) as default causes
point stencils to be placed in the skyline, potentially effecting
vertical spacing.
Separation_item::boxes uses this (0,0) extent to add extra spacing
LGTM, with one suggestion: the difference between unpure and pure
functions in LilyPond is that unpure functions accept n arguments
whereas pure functions accept n+2. This shortcut you're proposing only
works when n=1, which you document, but it'd be nice if it worked for an
arbitrary number of
https://codereview.appspot.com/4515158/diff/2009/lily/align-interface.cc
File lily/align-interface.cc (right):
https://codereview.appspot.com/4515158/diff/2009/lily/align-interface.cc#newcode222
lily/align-interface.cc:222: dy = max (dy, min_distance);
On 2011/06/09 02:55:55, Keith wrote:
On
New patch set coming after I finish running the regtests.
https://codereview.appspot.com/7310075/diff/10/input/regression/skyline-point-extent.ly
File input/regression/skyline-point-extent.ly (right):
Hey all,
After letting this newest round sit for a few days, I'm pretty confident
that the patch is ready for review so I'd ask people to review it.
First, if anyone needs more comments in the code to let them know what's
going on, lemme know where and I'll add it. Second, if people have
People can review this if they have time...it is a lot of code, but it
is more a progress report than anything else.
To resume a bit of what's been said before: once translate_axis is
called, the dim_cache of grobs is set and it is very difficult to work
with approximations. This makes
sneaky callback is not a real concern...it's a pedantic concern.
If we're following the model to a T, grob properties should not be read
in engravers, just set. The idea is that the engravers are creating the
grobs and fixing them up with properties if need be. I prefer
duplicating a context
Reviewers: ,
Message:
This doesn't crash in the test case Keith proposed:
\new StaffGroup \with { \RemoveEmptyStaves
} { b1 b1 b1}
{ b1 b1 \break b1 }
Description:
Eliminates the Hara_kiri_engraver.
Uses a haraKiri context property in the Axis_group_engraver to
acheive the same
Reviewers: Keith,
Message:
You're right...it was mostly out of laziness and desperation that I did
the quick fix...it worked for a piece I was typesetting. I have a long
plane ride on the 28th to fix this correctly.
https://codereview.appspot.com/6972044/diff/1/lily/metronome-engraver.cc
File
Reviewers: ,
Message:
Hey all,
I'm ok w/ this on the countdown but can someone check out David's
suspicion that this slows stuff down by O(n^3)? I definitely won't push
this if it slows LilyPond down to a crawl.
Cheers,
MS
Description:
Checks for recursive element behavior
Please review
Reviewers: aleksandr.andreev,
Message:
I think a user should be able to use Kievan notation and normal
stems/flags/beams if she so chooses. I'll define something like
startKievanNotation = {
%% Set glyph styles.
\override NoteHead #'style = #'kievan
\override Rest #'style = #'mensural
Reviewers: ,
Message:
This slows down LilyPond - I haven't done comprehensive tests of how
much. I'm pretty sure it works (the regtest works as expected).
Irrespective of how multiple passes are done, this seems like a
necessary first step.
Note that this patch would not have a time impact if
LGTM - looking forward to the skyline version, as that'll more
accurately reflect where columns are overhanging.
http://codereview.appspot.com/6489107/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
Reviewers: Keith,
Message:
I'm gonna hold off on posting to this one as it evolves constantly with
my work on issue 2801, which is revealing several issues in cross-staff
stem calculations. Irrespective of how issue 2801 pans out, I'll take
all the relevant code from stem.cc and either push it
Reviewers: dak,
http://codereview.appspot.com/6496085/diff/1/lily/chord-name-engraver.cc
File lily/chord-name-engraver.cc (right):
http://codereview.appspot.com/6496085/diff/1/lily/chord-name-engraver.cc#newcode83
lily/chord-name-engraver.cc:83: || ly_is_procedure
(chord_name_-get_property
The speed problem was twofold - some cruft in callbacks coupled with the
fact that I wasn't doing make cleans, so something about the way that
gcc was putting together the old .o files was slowing things down. I
learned my lesson: always do make clean before testing a patch.
A full make clean
On 2012/09/03 13:46:33, mike7 wrote:
On 3 sept. 2012, at 07:07, mailto:mts...@gmail.com wrote:
On 2012/09/02 20:38:28, Keith wrote:
On 2012/09/02 06:25:58, MikeSol wrote:
It's not a copy of the original slur because it is using
pure heights and offsets.
I saw you interrogating SlurStub
Reviewers: Keith,
Message:
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 figuring system
Thanks for the review!
http://codereview.appspot.com/6498077/diff/21/lily/axis-group-interface.cc
File lily/axis-group-interface.cc (right):
http://codereview.appspot.com/6498077/diff/21/lily/axis-group-interface.cc#newcode419
lily/axis-group-interface.cc:419: */
On 2012/09/02 15:59:00, dak
Reviewers: dak,
Message:
Thanks for the review!
http://codereview.appspot.com/6493073/diff/1/lily/self-alignment-interface.cc
File lily/self-alignment-interface.cc (right):
http://codereview.appspot.com/6493073/diff/1/lily/self-alignment-interface.cc#newcode213
On 2012/09/02 20:38:28, Keith wrote:
On 2012/09/02 06:25:58, MikeSol wrote:
It's not a copy of the original slur because it is using
pure heights and offsets.
I saw you interrogating SlurStub regarding its purity, but did not
notice that
SlurStub took any different shape based on
LGTM
http://codereview.appspot.com/6498052/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
http://codereview.appspot.com/6490043/diff/7001/lily/ledger-line-spanner.cc
File lily/ledger-line-spanner.cc (right):
http://codereview.appspot.com/6490043/diff/7001/lily/ledger-line-spanner.cc#newcode50
lily/ledger-line-spanner.cc:50: Paper_column::get_rank
(previous_column)))
I'm having
Thanks for the performance tests! I have no problem changing the
function avoid_outside_staff_collisions to be faster - it's just that I
haven't wrapped my head around how distance alone can do it.
http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc
File
I've forgotten why I added the horizontal skyline stuff so I've taken it
out in a new version that I'll post after regtests.
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/105001/lily/axis-group-interface.cc
File lily/axis-group-interface.cc (right):
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc#newcode778
lily/axis-group-interface.cc:778: Note we only use 75% of padding and
apply the
Thanks for the review!
http://codereview.appspot.com/5626052/diff/105001/input/regression/text-script-vertical-skylines.ly
File input/regression/text-script-vertical-skylines.ly (right):
Hey Phil!
First and foremost, congrats on this work. I'm thrilled to see you
venturing into the C++ side. You're tackling an issue that, while just
a few lines of code, uses a lot of advanced LilyPond structures, so it's
not easy. My suggestions don't have to do with the math (all of it is
It's back...
The only thing that doesn't the work the way I'd expect it is
Skyline::padded. It seems to not be adding the padding correctly (Joe -
any ideas)? This causes tighter than desired spacing in
alignment-order.ly. Other than that it is all reviewable! For those
who were keeping
http://codereview.appspot.com/5626052/diff/78001/lily/skyline.cc
File lily/skyline.cc (right):
http://codereview.appspot.com/5626052/diff/78001/lily/skyline.cc#newcode693
lily/skyline.cc:693: Skyline::padded (Real horizon_padding) const
On 2012/07/01 23:04:17, Keith wrote:
The horizon_padding
Reviewers: Keith,
Message:
Thanks for the review!
http://codereview.appspot.com/6302097/diff/1/lily/line-spanner.cc
File lily/line-spanner.cc (right):
http://codereview.appspot.com/6302097/diff/1/lily/line-spanner.cc#newcode373
lily/line-spanner.cc:373: me-warning (_ (Line spanner's left
LGTM
http://codereview.appspot.com/6310065/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Good work - two overall things.
1) In Scheme, try to avoid `do'. Use map and reduce.
2) Have you tested performance hits on large scores? It may be worth it
to leave bar-line.cc as the default and have your script as a Scheme
implementation, not unlike the way we do bezier curves.
Cheers,
MS
On 2012/06/12 13:22:10, dak wrote:
On 2012/06/12 12:54:40, MikeSol wrote:
On 2012/06/12 12:49:45, dak wrote:
http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc
File lily/grob.cc (right):
http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc#newcode472
On 2012/06/11 07:10:48, dak wrote:
On 2012/06/11 05:34:23, MikeSol wrote:
This shows a case where stem height cannot be determined by stem
stencil
presence. The first version of the patch works under the assumption
that
information about height cannot be gleaned from the stencil and
On 2012/06/11 07:48:34, dak wrote:
On 2012/06/11 07:24:33, MikeSol wrote:
On 2012/06/11 07:10:48, dak wrote:
Personally, I consider it an accident waiting to happen if downing
the
stencil
is not enough to suppress its consideration.
_Iff_ there is a situation where it is really
Reviewers: ,
Message:
This fixes Issue 2574 but also deals with the footnote-break-visibility
regtest, which currently does not register the property change (this may
have something to do with the footnote engraver being on the Score
level).
Cheers,
MS
Description:
Footnotes correctly printed
On 2012/06/10 16:09:03, dak wrote:
On 2012/06/10 15:58:25, MikeSol wrote:
This fixes Issue 2574 but also deals with the
footnote-break-visibility
regtest,
which currently does not register the property change (this may have
something
to do with the footnote engraver being on the Score
Reviewers: ,
Message:
Another way of going about this would be changing the Stem::height
function so that it returned an empty interval for stencil-less stems.
Then the overrides wouldn't be necessary. It's a design question more
than anything else.
Description:
Sets TabVoice Stem height to
On 2012/06/11 03:44:32, Keith wrote:
http://codereview.appspot.com/6303065/diff/4001/lily/stem.cc
File lily/stem.cc (right):
http://codereview.appspot.com/6303065/diff/4001/lily/stem.cc#newcode703
lily/stem.cc:703: if (calc_beam !unsmob_stencil (me-get_property
(stencil)))
On 2012/05/11 08:11:18, dak wrote:
m...@apollinemike.com mailto:m...@apollinemike.com writes:
On 11 mai 2012, at 09:09, mailto:d...@gnu.org wrote:
http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver.cc
File lily/pure-from-neighbor-engraver.cc (right):
Reviewers: janek, carl.d.sorensen_gmail.com,
http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver.cc
File lily/pure-from-neighbor-engraver.cc (right):
http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver.cc#newcode56
Reviewers: ,
Message:
Hey all,
My ensemble is launching a Kickstarter project in a day or two to
support our tour in France and Ireland.
We have a sweet plug in the project video for GNU LilyPond and I was
wondering if I could strike up a partnership with LilyPond to put a link
to the project
http://codereview.appspot.com/5934050/diff/7001/lily/stem.cc
File lily/stem.cc (right):
http://codereview.appspot.com/5934050/diff/7001/lily/stem.cc#newcode131
lily/stem.cc:131: Real stem_beg = internal_calc_stem_begin_position (me,
false);
On 2012/04/02 07:42:55, Keith wrote:
I'm assuming
Reviewers: Keith,
Message:
There is a nasty bug in LilyPond that this patch only exacerbates - as a
rule of thumb, X positioning functions should not set Y positioning
variables and vice versa. note-collision.cc, which works from an X_AXIS
callback, sets the stem attachment, which is a both X
Reviewers: Keith, mike_apollinemike.com,
Message:
Running regtests. Will post new patch once it gets a clean bill of
health.
http://codereview.appspot.com/5843063/diff/7/input/regression/pure-from-neighbor-interface-pure-height.ly
File
http://codereview.appspot.com/5844052/diff/3001/lily/beaming-pattern.cc
File lily/beaming-pattern.cc (right):
http://codereview.appspot.com/5844052/diff/3001/lily/beaming-pattern.cc#newcode361
lily/beaming-pattern.cc:361: }
My comment has nothing to do with your patch but with this function -
Reviewers: ,
Message:
Meh...not my best work, but it is a first step towards fixing this
problem. A full solution would do tuplet avoidance for scripts in the
same way they're done for slurs. Takers?
Description:
Turns off TupletBracket collision avoidance if Script avoids slur
Please review
http://codereview.appspot.com/5729051/diff/6001/lily/span-bar-stub-engraver.cc
File lily/span-bar-stub-engraver.cc (right):
http://codereview.appspot.com/5729051/diff/6001/lily/span-bar-stub-engraver.cc#newcode35
lily/span-bar-stub-engraver.cc:35: these four contexts.
On 2012/03/04 21:35:57,
http://codereview.appspot.com/5755058/diff/1/lily/page-breaking.cc
File lily/page-breaking.cc (right):
http://codereview.appspot.com/5755058/diff/1/lily/page-breaking.cc#newcode584
lily/page-breaking.cc:584: }
Just a C++ question - do these lines implicitly call the copy
constructor to create
http://codereview.appspot.com/5729051/diff/1001/lily/span-bar-stub-engraver.cc
File lily/span-bar-stub-engraver.cc (right):
http://codereview.appspot.com/5729051/diff/1001/lily/span-bar-stub-engraver.cc#newcode71
lily/span-bar-stub-engraver.cc:71: axis_groups_ = scm_cons (scm_cons
(i.grob
On 2012/03/04 20:59:22, Keith wrote:
http://codereview.appspot.com/5729051/diff/6001/lily/span-bar-stub-engraver.cc
File lily/span-bar-stub-engraver.cc (right):
http://codereview.appspot.com/5729051/diff/6001/lily/span-bar-stub-engraver.cc#newcode46
lily/span-bar-stub-engraver.cc:46: be
Reviewers: ,
Message:
Hey all,
I don't have time to test this patch tonight (David sent me to bed) but
it should do the trick. The map was overkill - a simple list of pairs
does the trick w/o anymore overhead and allows the use of derived_mark.
Cheers,
MS
Description:
Uses derived_mark to
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)
On 2012/02/22 09:34:43,
http://codereview.appspot.com/5626052/diff/40001/lily/stencil-integral.cc
File lily/stencil-integral.cc (right):
http://codereview.appspot.com/5626052/diff/40001/lily/stencil-integral.cc#newcode932
lily/stencil-integral.cc:932: MAKE_SCHEME_CALLBACK (Grob,
On 2012/02/23 13:35:29, dak wrote:
On 2012/02/23 12:37:04, MikeSol wrote:
http://codereview.appspot.com/5626052/diff/40001/lily/stencil-integral.cc
File lily/stencil-integral.cc (right):
http://codereview.appspot.com/5626052/diff/40001/lily/stencil-integral.cc#newcode932
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)
On 2012/02/16 08:09:10,
Sorry for whitespace problems.
After receiving several good general comments regarding the structure of
this patch, I have incorporated them all into my work and am now putting
it on Reitveld for more technical review.
Please run:
make clean
./autogen.sh --disable-optimising
make all
To make
Hey all,
Could people please try this patch out on a couple real world scores and
do some benchmarking compared to current master? It adds a lot of
calculations to lilypond and a lot of them happen in Scheme, so I wanna
make sure lilypond doesn't take a large processing hit.
Cheers,
MS
Should have added before: the most recent patch set is not bug free.
I'm fixing all of the regtest issues, but what I need most from other
people who have a few minutes are benchmarks.
Cheers,
MS
http://codereview.appspot.com/5626052/
___
Reviewers: ,
Message:
Sorry in advance for the whitespace errors.
This patch provides a generic framework for building vertical skylines
out of boxes by traversing through a stencil. It currently only
implements skylines for three stencil types (draw-line, named-glyph, and
glyph-string) and
Hey all,
The newest version of this misses up some of the hairpin regtests
(hairpin-barline-break.ly, for example). Still not sure why - I don't
have time to check it out for a bit, but if anyone has intuition as to
why, lemme know!
Cheers,
MS
http://codereview.appspot.com/5602054/
Haven't had a chance to test it, but the code looks good.
Have you tested this out yet with nested tuplets?
Cheers,
MS
http://codereview.appspot.com/5623051/diff/1/lily/beaming-pattern.cc
File lily/beaming-pattern.cc (right):
Reviewers: Neil Puttock,
http://codereview.appspot.com/5602054/diff/2001/input/regression/metronome-mark-broken-bound.ly
File input/regression/metronome-mark-broken-bound.ly (right):
http://codereview.appspot.com/5602054/diff/2001/input/regression/metronome-mark-broken-bound.ly#newcode1
Reviewers: ,
Message:
The idea here is to create a generic framework that allows for
modifications to note lengths (i.e. swing) in the MIDI without having a
typographical impact on the score.
Description:
Creates a MIDI note length formatter
Please review this at
Reviewers: Patrick McCarty, dak, mike_apollinemike.com, janek,
http://codereview.appspot.com/5504106/diff/11013/lily/grob.cc
File lily/grob.cc (right):
http://codereview.appspot.com/5504106/diff/11013/lily/grob.cc#newcode177
lily/grob.cc:177: SCM expr = scm_list_3 (ly_symbol2scm (id),
On
Looks good! Much more user-understandable than anything I could have
ever come up with :)
http://codereview.appspot.com/5569050/diff/1/Documentation/notation/changing-defaults.itely
File Documentation/notation/changing-defaults.itely (right):
http://codereview.appspot.com/5440084/diff/11001/lily/rhythmic-music-iterator.cc
File lily/rhythmic-music-iterator.cc (right):
http://codereview.appspot.com/5440084/diff/11001/lily/rhythmic-music-iterator.cc#newcode62
lily/rhythmic-music-iterator.cc:62: if (scm_is_true
ly_lily_module_constant
On 2012/01/20 15:33:07, dak wrote:
Should I be forking the rhythmic-music-iterator stuff into the
separate review
I'd say leave it with this - it's easy to forget that two patches need
to be tested in concert.
http://codereview.appspot.com/5440084/
I still think this patch goes outside the model of the way these things
are usually designed in LilyPond. I'm not saying that LilyPond's design
paradigm is objectively good or bad (I truly have no idea, as I know
nothing about any other piece of software), but I think it's best to
follow it
Reviewers: dak,
http://codereview.appspot.com/5554048/diff/2001/lily/rhythmic-music-iterator.cc
File lily/rhythmic-music-iterator.cc (right):
http://codereview.appspot.com/5554048/diff/2001/lily/rhythmic-music-iterator.cc#newcode41
lily/rhythmic-music-iterator.cc:41: report_event (get_music
On 2012/01/18 14:27:53, dak wrote:
http://codereview.appspot.com/5554048/diff/5001/lily/rhythmic-music-iterator.cc
File lily/rhythmic-music-iterator.cc (right):
http://codereview.appspot.com/5554048/diff/5001/lily/rhythmic-music-iterator.cc#newcode42
lily/rhythmic-music-iterator.cc:42:
The tuplet-iterator.cc has a process method that uses similar logic as
that above: it does parts of what the report_music function does,
because as you correctly state, the function does very little.
LGTM - if you can, please adopt this patch so that you can coordinate
getting it and others
I would advise not handling it this way - the function to_event is
supposed to take music and return an event. In this patch, it is taking
music, returning an event, and maybe broadcasting music and/or sending
it to a context.
I'd recommend creating a new iterator, rhythmic-music-iterator, that
Reviewers: lemzwerg, carl.d.sorensen_gmail.com, Keith,
http://codereview.appspot.com/5538049/diff/1/lily/staff-symbol-referencer.cc
File lily/staff-symbol-referencer.cc (right):
http://codereview.appspot.com/5538049/diff/1/lily/staff-symbol-referencer.cc#newcode95
1 - 100 of 409 matches
Mail list logo
