Flags cross-staff semi ties. (issue 8857043)

2013-04-19 Thread n . puttock
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)) (stem (ly:grob-object note-head 'stem))) (and (

Allows slurs to break at barlines. (issue 7424049)

2013-02-28 Thread n . puttock
https://codereview.appspot.com/7424049/diff/1/input/regression/repeat-slur.ly File input/regression/repeat-slur.ly (right): https://codereview.appspot.com/7424049/diff/1/input/regression/repeat-slur.ly#newcode14 input/regression/repeat-slur.ly:14: \alternative { { a' ) b' \set breakSlurAtBarLine

Cleanups in one-line-page-breaking. (issue 6248056)

2012-05-31 Thread n . puttock
LGTM. http://codereview.appspot.com/6248056/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Control mid-measure beams in 3/4 time; Issue 2566 and 2246 (issue 6247055)

2012-05-31 Thread n . puttock
LGTM. http://codereview.appspot.com/6247055/diff/9002/input/regression/autobeam-3-4-rules.ly File input/regression/autobeam-3-4-rules.ly (right): http://codereview.appspot.com/6247055/diff/9002/input/regression/autobeam-3-4-rules.ly#newcode17 input/regression/autobeam-3-4-rules.ly:17: \set Timi

Re: add \shape (issue 6255056)

2012-05-31 Thread n . puttock
http://codereview.appspot.com/6255056/diff/5001/input/regression/shape-slurs.ly File input/regression/shape-slurs.ly (right): http://codereview.appspot.com/6255056/diff/5001/input/regression/shape-slurs.ly#newcode5 input/regression/shape-slurs.ly:5: @code{\\shape}. The blue slurs are modified fr

Fix a number of display-lily shortcomings (issue 6203056)

2012-05-09 Thread n . puttock
LGTM. Just some redundant spaces in the ArticulationEvent display method needs removing, but then they were already there. Cheers, Neil http://codereview.appspot.com/6203056/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.or

Re: hideNotes should hide also TabNoteHead (issue 2480). (issue 6105049)

2012-04-23 Thread n . puttock
On 2012/04/23 12:09:15, dak wrote: \voiceOne is already used for the upper voice. The problem is that the voice settings are totally messed up by the standard grace synchronization problem. Remove the graces, and the score typesets fine. Ah, I didn't scroll down that far. :) Nevertheless,

Re: hideNotes should hide also TabNoteHead (issue 2480). (issue 6105049)

2012-04-23 Thread n . puttock
On 2012/04/23 04:50:21, Keith wrote: I need an \override NoteColumn #'ignore-collision = ##t here or else I get a warning (either before or after this patch). I mention this only because I broke the doc build not too long ago by causing a warning from Lilypond input in the docs. (patchy

Re: Uses derived_mark to avoid segfault (issue 5729051)

2012-03-04 Thread n . puttock
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#newcode160 lily/span-bar-stub-engraver.cc:160: // removes all unused contexts Is it feasible

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

2012-02-23 Thread n . puttock
Hi Mike, I've tested the latest patch on a few real-world scores and there appears to be a bit of weirdness going on with TupletNumbers when a DynamicText's nearby: \relative c' { \times 2/3 { c8^\p c c } } Cheers, Neil http://codereview.appspot.com/5626052/

Overhauls broken bound coordinate checking (issue 5602054)

2012-02-01 Thread n . 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 input/regression/metronome-mark-b

Re: Don't wrap EventChord around rhythmic events by default. (issue 5440084)

2012-01-24 Thread n . puttock
Great work David. I like how \harmonic works properly for single notes now. :) http://codereview.appspot.com/5440084/diff/14005/lily/music-scheme.cc File lily/music-scheme.cc (right): http://codereview.appspot.com/5440084/diff/14005/lily/music-scheme.cc#newcode79 lily/music-scheme.cc:79: "Is

Re: Don't wrap EventChord around rhythmic events by default. (issue 5440084)

2012-01-20 Thread n . puttock
Hi David, Should I wait for a new patch or can I test using the latest one here? I've tried it out briefly on a real music example, and have a problem with identifiers: foo = \mark \default \relative c' { \foo c1 } -> error: syntax error, unexpected EVENT_IDENTIFIER \foo If I add a note

Re: Glyphs for Kievan Notation (issue 4951062)

2012-01-18 Thread n . puttock
http://codereview.appspot.com/4951062/diff/99002/lily/stem.cc File lily/stem.cc (right): http://codereview.appspot.com/4951062/diff/99002/lily/stem.cc#newcode284 lily/stem.cc:284: string style = robust_symbol2string (heads[0]->get_property ("style"), "default"); A bit fussy. You can safely chec

Re: changes.tely: mention Flag changes, remove duplicate "does" (issue 5540058)

2012-01-18 Thread n . puttock
http://codereview.appspot.com/5540058/diff/3003/Documentation/changes.tely File Documentation/changes.tely (right): http://codereview.appspot.com/5540058/diff/3003/Documentation/changes.tely#newcode67 Documentation/changes.tely:67: \override Flag #'color = #red g8 Please put the note on a new li

Re: Doc: NR Section on Upbeats made clearer (issue 5520056)

2012-01-09 Thread n . puttock
Note that this is misleading: \relative c' { \partial 4 c4 \applyContext #(lambda (ctx) (newline) (display (ly:context-current-moment ctx))) c1 } -> # The Rational class doesn't display negative rationals unless they're infinite. http://coderevi

Re: Doc: NR Section on Upbeats made clearer (issue 5520056)

2012-01-09 Thread n . puttock
On 2012/01/09 12:58:58, Neil Puttock wrote: http://codereview.appspot.com/5520056/diff/1/Documentation/notation/rhythms.itely File Documentation/notation/rhythms.itely (right): http://codereview.appspot.com/5520056/diff/1/Documentation/notation/rhythms.itely#newcode1366 Documentation/notatio

Re: Doc: NR Section on Upbeats made clearer (issue 5520056)

2012-01-09 Thread n . puttock
http://codereview.appspot.com/5520056/diff/1/Documentation/notation/rhythms.itely File Documentation/notation/rhythms.itely (right): http://codereview.appspot.com/5520056/diff/1/Documentation/notation/rhythms.itely#newcode1366 Documentation/notation/rhythms.itely:1366: The @code{\partial @var{du

Re: Give slurs skylines in outside-staff-priority calculations. (issue 5504055)

2011-12-28 Thread n . puttock
On 2011/12/28 17:41:03, mike_apollinemike.com wrote: I added it precisely to un-silently prevent negative numbers. It's true that it has no analogue in Scheme, but I didn't think this was a prerequisite for adding a predicate. In lily.scm, there are several predicates that don't have Scheme

Re: Give slurs skylines in outside-staff-priority calculations. (issue 5504055)

2011-12-28 Thread n . puttock
http://codereview.appspot.com/5504055/diff/3002/scm/define-grob-properties.scm File scm/define-grob-properties.scm (right): http://codereview.appspot.com/5504055/diff/3002/scm/define-grob-properties.scm#newcode1101 scm/define-grob-properties.scm:1101: (skyline-quantizing ,ly:vsize? "The number o

Give slurs skylines in outside-staff-priority calculations. (issue 5504055)

2011-12-21 Thread n . puttock
http://codereview.appspot.com/5504055/diff/1/lily/slur.cc File lily/slur.cc (right): http://codereview.appspot.com/5504055/diff/1/lily/slur.cc#newcode388 lily/slur.cc:388: vsize N_BOXES = 100; Don't you think this is a bit extravagant? At least, this shouldn't be hard-coded. A coarser integral

Re: Adds parenthesized measure numbers at mid-bar breaks. (issue 5452057)

2011-12-12 Thread n . puttock
http://codereview.appspot.com/5452057/diff/7002/scm/define-context-properties.scm File scm/define-context-properties.scm (left): http://codereview.appspot.com/5452057/diff/7002/scm/define-context-properties.scm#oldcode127 scm/define-context-properties.scm:127: (barNumberVisibility ,procedure? "A

Re: Allows for automatic renumbering of measure numbers at volta repeats. (issue 5440049)

2011-12-12 Thread n . puttock
On 2011/12/06 12:07:03, mike_apollinemike.com wrote: Good to see you back on the list! Cheers mate. :) I'm afraid I'm only passing through temporarily though. Hopefully will get back into the swing of things after Christmas. Cheers, Neil http://codereview.appspot.com/5440049/

Re: Allows for automatic renumbering of measure numbers at volta repeats. (issue 5440049)

2011-12-06 Thread n . puttock
http://codereview.appspot.com/5440049/diff/9003/lily/bar-number-engraver.cc File lily/bar-number-engraver.cc (right): http://codereview.appspot.com/5440049/diff/9003/lily/bar-number-engraver.cc#newcode156 lily/bar-number-engraver.cc:156: scm_string_concatenate (scm_list_2 (scm_number_to_string (

Re: Implement \once as music function able to operate on complex stuff. (issue 5322065)

2011-11-03 Thread n . puttock
LGTM. http://codereview.appspot.com/5322065/diff/3001/input/regression/complex-once.ly File input/regression/complex-once.ly (right): http://codereview.appspot.com/5322065/diff/3001/input/regression/complex-once.ly#newcode7 input/regression/complex-once.ly:7: \layout { ragged-right = ##t } redu

Re: changing-defaults.itely: correct misstatement about variables for context mods (issue 5306076)

2011-11-03 Thread n . puttock
http://codereview.appspot.com/5306076/diff/3002/Documentation/notation/changing-defaults.itely File Documentation/notation/changing-defaults.itely (left): http://codereview.appspot.com/5306076/diff/3002/Documentation/notation/changing-defaults.itely#oldcode732 Documentation/notation/changing-def

Re: Sketch for in-notes. (issue 5293053)

2011-10-28 Thread n . puttock
http://codereview.appspot.com/5293053/diff/11005/input/regression/footnote-break-visibility.ly File input/regression/footnote-break-visibility.ly (left): http://codereview.appspot.com/5293053/diff/11005/input/regression/footnote-break-visibility.ly#oldcode1 input/regression/footnote-break-visibi

Re: Sketch for in-notes. (issue 5293053)

2011-10-21 Thread n . puttock
http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc File lily/page-layout-problem.cc (right): http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc#newcode79 lily/page-layout-problem.cc:79: footnotes_added = g->get_property ("footnote-stencil") !=

Re: Adds Scheme function for spring constructor. (issue 5306050)

2011-10-21 Thread n . puttock
http://codereview.appspot.com/5306050/diff/1/lily/spring-smob.cc File lily/spring-smob.cc (right): http://codereview.appspot.com/5306050/diff/1/lily/spring-smob.cc#newcode48 lily/spring-smob.cc:48: "spring, and @code{min-dist} is the minimum distance.") @var{min-dist} http://codereview.appspot.

Re: Sketch for broken beams with consistent slopes (issue 4961041)

2011-10-20 Thread n . puttock
http://codereview.appspot.com/4961041/diff/60001/Documentation/changes.tely File Documentation/changes.tely (right): http://codereview.appspot.com/4961041/diff/60001/Documentation/changes.tely#newcode66 Documentation/changes.tely:66: \override Beam #'breakable = ##t no indent http://codereview.

Re: Make accidental styles available as context mods. (issue 4819064)

2011-10-20 Thread n . puttock
On 2011/10/20 05:44:38, dak wrote: Sorry, I always start thinking only when far too late in the game. Anyway, here is a musing: should't the _main_ interface to accidental modifications be context mods? You won't need to set them in midcontext except in unusual situations, and then \applyC

Re: Make accidental styles available as context mods. (issue 4819064)

2011-10-20 Thread n . puttock
On 2011/10/20 04:06:03, Keith wrote: Neil, don't forget to push this. I've been looking forward to it. I'll try to sort it out at the weekend. Unfortunately I don't have a working Linux install at the moment, and foolishly neglected to back it up for easier restoring. Cheers, Neil http://cod

Re: First stab at getting script offsets right. (issue 5235052)

2011-10-18 Thread n . puttock
http://codereview.appspot.com/5235052/diff/17001/lily/script-engraver.cc File lily/script-engraver.cc (right): http://codereview.appspot.com/5235052/diff/17001/lily/script-engraver.cc#newcode206 lily/script-engraver.cc:206: Script_engraver::acknowledge_inline_accidental (Grob_info info) Doesn't

Re: Creates a LeftBarStub that stops lyrics from bumping into the SystemStartBar. (issue 5201043)

2011-10-08 Thread n . puttock
http://codereview.appspot.com/5201043/diff/3001/scm/output-lib.scm File scm/output-lib.scm (right): http://codereview.appspot.com/5201043/diff/3001/scm/output-lib.scm#newcode355 scm/output-lib.scm:355: (if (eq? 0 (ly:item-break-dir grob)) Doesn't this potentially make the function unpure? If I

Re: Optimizes note-heads.cc and introduces robust_symbol2string. (issue 5233042)

2011-10-08 Thread n . puttock
Hi Betrand, This should be split into two patches: one for the optimization and another for adding robust_symbol2string lookups outside note-head.cc. Cheers, Neil http://codereview.appspot.com/5233042/diff/12001/lily/lily-guile.cc File lily/lily-guile.cc (left): http://codereview.appspot.com/

Doc: NR 1.2.6 added slashedGrace function (issue 5133043)

2011-09-25 Thread n . puttock
http://codereview.appspot.com/5133043/diff/1/Documentation/changes.tely File Documentation/changes.tely (right): http://codereview.appspot.com/5133043/diff/1/Documentation/changes.tely#newcode65 Documentation/changes.tely:65: There is a new @code{\slashedGrace} function: This is already document

Re: Terminates outside_staff_callback early if a grob is outside a slur's X-extent (issue 5056041)

2011-09-21 Thread n . puttock
Needs a regression test. This might do: \relative c'' { \set strokeFingerOrientations = #'(up) \override StrokeFinger #'avoid-slur = #'outside 16( b) } http://codereview.appspot.com/5056041/diff/1/lily/slur.cc File lily/slur.cc (right): http://codereview.appspot.com/5056041/diff/1/lily/

Re: Progress on loose columns. (issue 4841052)

2011-09-21 Thread n . puttock
LGTM. http://codereview.appspot.com/4841052/diff/25001/input/regression/spacing-loose-polyphony.ly File input/regression/spacing-loose-polyphony.ly (right): http://codereview.appspot.com/4841052/diff/25001/input/regression/spacing-loose-polyphony.ly#newcode1 input/regression/spacing-loose-polyp

Re: Implement define-event-function (issue 5083045)

2011-09-21 Thread n . puttock
http://codereview.appspot.com/5083045/diff/1/scm/c++.scm File scm/c++.scm (right): http://codereview.appspot.com/5083045/diff/1/scm/c++.scm#newcode38 scm/c++.scm:38: (define-public (event? x) I'd prefer a less vague name for this since it's going to conflict with the stream event predicate (curr

Re: Unifies mensural ligatures with blot-diameter. (issue 5030053)

2011-09-21 Thread n . puttock
http://codereview.appspot.com/5030053/diff/2004/lily/mensural-ligature.cc File lily/mensural-ligature.cc (right): http://codereview.appspot.com/5030053/diff/2004/lily/mensural-ligature.cc#newcode74 lily/mensural-ligature.cc:74: = (me->layout ()->get_dimension (ly_symbol2scm ("blot-diameter")));

Re: New alist to replace special characters. (issue 4553056)

2011-09-21 Thread n . puttock
http://codereview.appspot.com/4553056/diff/106003/Documentation/included/special-characters.ly File Documentation/included/special-characters.ly (right): http://codereview.appspot.com/4553056/diff/106003/Documentation/included/special-characters.ly#newcode1 Documentation/included/special-charact

Re: Implement define-event-function (issue 5083045)

2011-09-21 Thread n . puttock
On 2011/09/21 19:40:37, reinhold_kainhofer.com wrote: Huh? How do you verify that your feature works at all? I'm sure you are using some simple test file for this. So, simply take that file, add a \header { doctitle="some short description" } and you're done. The example David's posted abo

Re: Adds StemTremolo to the note column's element list. (issue 5067041)

2011-09-19 Thread n . puttock
On 2011/09/19 05:45:04, MikeSol wrote: While working on flags, I noticed that horizontal spacing changes when a grob is part of a NoteColumn's element list. I went back to StemTremolos and tested this out: run the code below with and without this patch: I'm getting deja vu here Mike. W

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

2011-09-17 Thread n . puttock
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

Re: Terminates outside_staff_callback early if a grob is outside a slur's X-extent (issue 5056041)

2011-09-17 Thread n . puttock
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-acciden

Re: Prunes stem::length down to the bare minimum. (issue 5057041)

2011-09-17 Thread n . puttock
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 foun

Re: Fixes issue 1881 (cyclic dependency with beam calculations) (issue 5038042)

2011-09-17 Thread n . puttock
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 cal

Re: Add scripts/auxiliar/guileindent.scm (issue 4896043)

2011-09-15 Thread n . puttock
lambda* should have same indentation as lambda comments are broken: ; (margin comment) should be indented (not sure how many spaces emacs does it though) ;;; (top-level comment) shouldn't be indented even inside a form There are a few files which use margin comments by mistake, so we should fi

Re: Add scripts/auxiliar/guileindent.scm (issue 4896043)

2011-09-15 Thread n . puttock
http://codereview.appspot.com/4896043/diff/10002/scripts/auxiliar/guileindent.scm File scripts/auxiliar/guileindent.scm (right): http://codereview.appspot.com/4896043/diff/10002/scripts/auxiliar/guileindent.scm#newcode38 scripts/auxiliar/guileindent.scm:38: (define (assf test-proc search-list) m

Re: Add some polyphonically directed grobs (issue 4387046)

2011-09-15 Thread n . puttock
LGTM. http://codereview.appspot.com/4387046/ ___ 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)

2011-09-14 Thread n . puttock
http://codereview.appspot.com/4974075/diff/8001/input/regression/staff-ledger-positions.ly File input/regression/staff-ledger-positions.ly (right): http://codereview.appspot.com/4974075/diff/8001/input/regression/staff-ledger-positions.ly#newcode4 input/regression/staff-ledger-positions.ly:4: by

Re: Fix 456: Also check for laissez-vibrer events attached to single heads inside a chord (issue 4969069)

2011-09-14 Thread n . puttock
LGTM. http://codereview.appspot.com/4969069/diff/2001/input/regression/laissez-vibrer-chords.ly File input/regression/laissez-vibrer-chords.ly (right): http://codereview.appspot.com/4969069/diff/2001/input/regression/laissez-vibrer-chords.ly#newcode1 input/regression/laissez-vibrer-chords.ly:1:

Re: Glyphs for Kievan Notation (issue 4951062)

2011-09-14 Thread n . puttock
http://codereview.appspot.com/4951062/diff/11001/input/regression/kievan-notes.ly File input/regression/kievan-notes.ly (right): http://codereview.appspot.com/4951062/diff/11001/input/regression/kievan-notes.ly#newcode11 input/regression/kievan-notes.ly:11: \override NoteHead #'stencil = #ly:tex

Re: Several fixes for annotate-spacing. (issue 4950071)

2011-09-13 Thread n . puttock
http://codereview.appspot.com/4950071/diff/1/lily/page-layout-problem-scheme.cc File lily/page-layout-problem-scheme.cc (right): http://codereview.appspot.com/4950071/diff/1/lily/page-layout-problem-scheme.cc#newcode26 lily/page-layout-problem-scheme.cc:26: "Return the spacing spec going between

Re: Doc: Added \compoundMeter function to NR (issue 4837050)

2011-09-12 Thread n . puttock
LGTM. http://codereview.appspot.com/4837050/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

parser.yy et al: turn \partial and \skip into music functions. (issue 4969076)

2011-09-12 Thread n . puttock
LGTM. http://codereview.appspot.com/4969076/diff/1/ly/music-functions-init.ly File ly/music-functions-init.ly (right): http://codereview.appspot.com/4969076/diff/1/ly/music-functions-init.ly#newcode794 ly/music-functions-init.ly:794: "Make a partial measure." (_i "Make a partial measure.") ind

Re: Fix 1805: AmbitusAccidental needs avoid-slur, needed when the notes in the ambitus have a slur (issue 4904049)

2011-09-05 Thread n . puttock
LGTM. http://codereview.appspot.com/4904049/diff/3001/input/regression/ambitus-slur.ly File input/regression/ambitus-slur.ly (right): http://codereview.appspot.com/4904049/diff/3001/input/regression/ambitus-slur.ly#newcode3 input/regression/ambitus-slur.ly:3: texidoc = "Ambitus also works with

Re: Deprecate \fermataMarkup for full-bar rests. (issue 4672059)

2011-09-05 Thread n . puttock
On 2011/08/07 19:48:58, reinhold_kainhofer.com wrote: I wouldn't go that lowlevel. I rather thought about a scheme function that prints a ly:warning and then returns the new definition (or calls the new function). Well, it turns out my idea for object properties wouldn't work anyway since

Re: Creates pure closures (issue 4894052)

2011-09-01 Thread n . puttock
http://codereview.appspot.com/4894052/diff/29001/lily/unpure-pure-container.cc File lily/unpure-pure-container.cc (right): http://codereview.appspot.com/4894052/diff/29001/lily/unpure-pure-container.cc#newcode35 lily/unpure-pure-container.cc:35: LY_ASSERT_TYPE (is_unpure_pure_container, smob, 1)

Re: Creates pure closures (issue 4894052)

2011-08-31 Thread n . puttock
http://codereview.appspot.com/4894052/diff/19001/lily/system.cc File lily/system.cc (right): http://codereview.appspot.com/4894052/diff/19001/lily/system.cc#newcode773 lily/system.cc:773: || is_unpure_pure_container (elts[i]->get_property_data ("Y-extent" move this to pure-relevant? http://

Re: Creates pure closures (issue 4894052)

2011-08-31 Thread n . puttock
On 2011/08/20 20:21:30, mikesol_ufl.edu wrote: I'll try to think of something better...if you have any suggestions in the meantime, they're certainly welcome! Something using a stencil override? > http://codereview.appspot.com/4894052/diff/9001/lily/pure-closure.cc#newcode68 > lily/pure-

Re: Creates a Flag grob. (issue 4922042)

2011-08-24 Thread n . puttock
http://codereview.appspot.com/4922042/diff/17001/lily/flag.cc File lily/flag.cc (right): http://codereview.appspot.com/4922042/diff/17001/lily/flag.cc#newcode147 lily/flag.cc:147: "what style of flag glyph is typeset on a" " what http://codereview.appspot.com/4922042/diff/17001/lily/flag.cc#new

Re: lily-guile updates and CG: "Scheme->C interface" section. (issue 4917044)

2011-08-19 Thread n . puttock
On 2011/08/19 21:08:10, Carl wrote: As an aside, I think that we should change the definition of the property align-dir. It should no longer be called a direction, since it's not limited to the values -1, 0, and 1. It's unused. I think it's superseded by stacking-dir. Cheers, Neil http

Re: Creates pure closures (issue 4894052)

2011-08-19 Thread n . puttock
http://codereview.appspot.com/4894052/diff/9001/input/regression/pure-closure.ly File input/regression/pure-closure.ly (right): http://codereview.appspot.com/4894052/diff/9001/input/regression/pure-closure.ly#newcode18 input/regression/pure-closure.ly:18: #(ly:make-pure-closure ly:stem::height '

Re: Prevents nested tuplets from colliding. (issue 4808082)

2011-08-19 Thread n . puttock
http://codereview.appspot.com/4808082/diff/13002/input/regression/tuplet-nest-broken.ly File input/regression/tuplet-nest-broken.ly (right): http://codereview.appspot.com/4808082/diff/13002/input/regression/tuplet-nest-broken.ly#newcode5 input/regression/tuplet-nest-broken.ly:5: texidoc = "Broke

Re: Doc: Added \compoundMeter function to NR (issue 4837050)

2011-08-19 Thread n . puttock
LGTM. http://codereview.appspot.com/4837050/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: lily-guile updates and CG: "Scheme->C interface" section. (issue 4917044)

2011-08-19 Thread n . puttock
On 2011/08/19 20:20:12, Neil Puttock wrote: http://codereview.appspot.com/4917044/diff/1/lily/include/lily-guile.hh File lily/include/lily-guile.hh (right): http://codereview.appspot.com/4917044/diff/1/lily/include/lily-guile.hh#newcode96 lily/include/lily-guile.hh:96: //

Re: lily-guile updates and CG: "Scheme->C interface" section. (issue 4917044)

2011-08-19 Thread n . puttock
http://codereview.appspot.com/4917044/diff/1/lily/general-scheme.cc File lily/general-scheme.cc (right): http://codereview.appspot.com/4917044/diff/1/lily/general-scheme.cc#newcode110 lily/general-scheme.cc:110: if (scm_is_integer (s)) On 2011/08/19 18:04:38, Carl wrote: I think the old code h

Re: Fixes boolean/SCM confusions, part 1. (issue 4875054)

2011-08-18 Thread n . puttock
http://codereview.appspot.com/4875054/diff/8002/lily/accidental-engraver.cc File lily/accidental-engraver.cc (right): http://codereview.appspot.com/4875054/diff/8002/lily/accidental-engraver.cc#newcode319 lily/accidental-engraver.cc:319: if (scm_to_int (left_objects_[i]->get_property ("side-axis

Re: Creates pure closures (issue 4894052)

2011-08-18 Thread n . puttock
Hi Mike, I have reservations about the naming, since you're basically creating a smob which acts as a container for a pair of callbacks; it doesn't work like a simple-closure in that you can evaluate the closure and get something useful back. Cheers, Neil http://codereview.appspot.com/4894052/

Re: Creates pure closures (issue 4894052)

2011-08-18 Thread n . puttock
On 2011/08/18 11:06:57, reinhold_kainhofer.com wrote: Wow, that would be VERY user-unfriendly. Just imagine explaining that to someone on -user... (or even to the average lilypond developer... I still haven't understood what closures are). I agree, though I do like the idea in principle si

Fix 1805: AmbitusAccidental needs avoid-slur, needed when the notes in the ambitus have a slur (issue 4904049)

2011-08-17 Thread n . puttock
http://codereview.appspot.com/4904049/diff/1/scm/define-grobs.scm File scm/define-grobs.scm (right): http://codereview.appspot.com/4904049/diff/1/scm/define-grobs.scm#newcode125 scm/define-grobs.scm:125: (avoid-slur . inside) I think it would make more sense for the engravers to ignore an Ambitu

Re: T1349 - Fix load order for running with Guile V2 (issue 4849054)

2011-08-17 Thread n . puttock
LGTM. http://codereview.appspot.com/4849054/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Fixes heights and pure heights of stems. (issue 4898044)

2011-08-14 Thread n . puttock
Hi Mike, This looks like a work in progress. What's going on with the begin/end position methods? You've removed the grob properties, but kept the exported callbacks plus an entry in pure-conversions-alist which does nothing. Cheers, Neil http://codereview.appspot.com/4898044/diff/1/scm/flag

Re: Add scripts/auxiliar/guileindent.scm (issue 4896043)

2011-08-14 Thread n . puttock
Hi Carl, This latest patch looks great. I'm not sold on the change to `else', but that's mainly due to being used to how Emacs does it. I'll do a bit more testing on other files and report back. Cheers, Neil http://codereview.appspot.com/4896043/ _

Re: Make accidental styles available as context mods. (issue 4819064)

2011-08-14 Thread n . puttock
http://codereview.appspot.com/4819064/diff/1/scm/music-functions.scm File scm/music-functions.scm (right): http://codereview.appspot.com/4819064/diff/1/scm/music-functions.scm#newcode1267 scm/music-functions.scm:1267: Staff ,(make-accidental-rule 'same-octave 0) As Carl's pointed out here, http:

Re: Fix for Issue 620. (issue 4814041)

2011-08-14 Thread n . puttock
http://codereview.appspot.com/4814041/diff/24001/lily/axis-group-interface.cc File lily/axis-group-interface.cc (right): http://codereview.appspot.com/4814041/diff/24001/lily/axis-group-interface.cc#newcode82 lily/axis-group-interface.cc:82: Axis_group_interface::relative_maybe_bound_group_exten

Re: Fix for Issue 620. (issue 4814041)

2011-08-14 Thread n . puttock
On 2011/08/05 00:21:45, MikeSol wrote: Neil - whenever you get the chance, I'd like to hear more of what you have to say about it. If you think that custom engravers would be a better idea here (or even non-custom engravers), I'd like to fully discuss it on the list. I think an engraver a

Re: Texi2HTML: don't wrap around the contents of table cells. (issue 4891044)

2011-08-14 Thread n . puttock
LGTM. http://codereview.appspot.com/4891044/diff/1/Documentation/lilypond-texi2html.init File Documentation/lilypond-texi2html.init (right): http://codereview.appspot.com/4891044/diff/1/Documentation/lilypond-texi2html.init#newcode2076 Documentation/lilypond-texi2html.init:2076: if (defined ($c

Re: Ends of barlines are hidden in staff lines. (issue 4809057)

2011-08-13 Thread n . puttock
http://codereview.appspot.com/4809057/diff/1/lily/bar-line.cc File lily/bar-line.cc (right): http://codereview.appspot.com/4809057/diff/1/lily/bar-line.cc#newcode40 lily/bar-line.cc:40: /* Due to rounding problems, barlines extending to the outermost edges bar lines http://codereview.appspot.co

Re: Adds a glyph for tied lyrics. (issue 4808074)

2011-08-13 Thread n . puttock
LGTM. http://codereview.appspot.com/4808074/diff/7001/scm/define-markup-commands.scm File scm/define-markup-commands.scm (right): http://codereview.appspot.com/4808074/diff/7001/scm/define-markup-commands.scm#newcode943 scm/define-markup-commands.scm:943: (tie-str (markup #:hspace (/ word-space

Re: Prevents nested tuplets from colliding. (issue4808082)

2011-08-11 Thread n . puttock
Hi Mike, Have you tested this with broken tuplets? I've tried adding breaks at random in tuplet-nest.ly and get collisions in some cases. Cheers, Neil http://codereview.appspot.com/4808082/diff/2001/input/regression/tuplet-nest.ly File input/regression/tuplet-nest.ly (right): http://coderevi

Re: T1349 - Fix load order for running with Guile V2 (issue4849054)

2011-08-11 Thread n . puttock
http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm File scm/lily.scm (right): http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode324 scm/lily.scm:324: (> file-name-length 2) tab->space conversion has broken indentation here (and lines below) http://codereview.appspo

Re: Fixes bad slur heights by limiting fit_factor to the interior of slurs. (issue4810072)

2011-08-11 Thread n . puttock
Needs a rebase. http://codereview.appspot.com/4810072/diff/1003/input/regression/slur-height-capping.ly File input/regression/slur-height-capping.ly (right): http://codereview.appspot.com/4810072/diff/1003/input/regression/slur-height-capping.ly#newcode4 input/regression/slur-height-capping.ly:

Re: modifying default behaviour of tremolo slashes (issue4636081)

2011-08-11 Thread n . puttock
Needs a regression test exercising the different settings. http://codereview.appspot.com/4636081/diff/38001/lily/stem-tremolo.cc File lily/stem-tremolo.cc (right): http://codereview.appspot.com/4636081/diff/38001/lily/stem-tremolo.cc#newcode118 lily/stem-tremolo.cc:118: shape = ly_symbol2scm ("

Re: lily/auto-beam-engraver: keep a Context_handle to starting Staff (issue4830064)

2011-08-11 Thread n . puttock
LGTM. Just needs a regtest. http://codereview.appspot.com/4830064/diff/5001/lily/auto-beam-engraver.cc File lily/auto-beam-engraver.cc (right): http://codereview.appspot.com/4830064/diff/5001/lily/auto-beam-engraver.cc#newcode85 lily/auto-beam-engraver.cc:85: Handle on the starting staff keeps

Re: T1349 - Fix load order for running with Guile V2 (issue4849054)

2011-08-11 Thread n . puttock
On 2011/08/11 14:36:24, Ian Hulin (gmail) wrote: New patch-set available for review. Please rebase against master. Cheers, Neil http://codereview.appspot.com/4849054/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/ma

Re: Better pure height approximations for beamed rests. (issue4860043)

2011-08-10 Thread n . puttock
LGTM. http://codereview.appspot.com/4860043/diff/11001/lily/beam.cc File lily/beam.cc (right): http://codereview.appspot.com/4860043/diff/11001/lily/beam.cc#newcode1736 lily/beam.cc:1736: Below, the prev_offset parameter is intentionally unused. This is comment it out then :) http://codereview

Re: Better pure height approximations for beamed rests. (issue4860043)

2011-08-10 Thread n . puttock
http://codereview.appspot.com/4860043/diff/4002/lily/rest.cc File lily/rest.cc (right): http://codereview.appspot.com/4860043/diff/4002/lily/rest.cc#newcode251 lily/rest.cc:251: Interval rest_max_pos = Staff_symbol::line_span (Staff_symbol_referencer::get_staff_symbol (me)); unsafe with \stopSta

Re: Fix 1214: cueDuring and quoteDuring should also quote voices that create subvoices (issue4816044)

2011-08-10 Thread n . puttock
LGTM. http://codereview.appspot.com/4816044/diff/15001/input/regression/quote-during-subvoice.ly File input/regression/quote-during-subvoice.ly (right): http://codereview.appspot.com/4816044/diff/15001/input/regression/quote-during-subvoice.ly#newcode1 input/regression/quote-during-subvoice.ly:

Re: Better pure height approximations for beamed rests. (issue4860043)

2011-08-10 Thread n . puttock
http://codereview.appspot.com/4860043/diff/1/lily/rest.cc File lily/rest.cc (right): http://codereview.appspot.com/4860043/diff/1/lily/rest.cc#newcode238 lily/rest.cc:238: // Wont play nicely with weird staves where individual line positions are set... Don't use line_count then :) (see Breathin

Re: Treats multi measure rest staff position like rest staff position. (issue4822046)

2011-08-10 Thread n . puttock
LGTM. http://codereview.appspot.com/4822046/diff/4001/input/regression/multi-measure-rest-staff-position.ly File input/regression/multi-measure-rest-staff-position.ly (right): http://codereview.appspot.com/4822046/diff/4001/input/regression/multi-measure-rest-staff-position.ly#newcode1 input/re

Re: Progress on loose columns. (issue4841052)

2011-08-09 Thread n . puttock
Hi Mike, The latest patch set breaks spacing-empty-bar.ly (we want left-loose-column-padding to be 0 in this case), but apart from that it looks great. Cheers, Neil http://codereview.appspot.com/4841052/diff/14002/input/regression/loose-column-spacing.ly File input/regression/loose-column-spac

Re: Fixes issue 40. (issue4801083)

2011-08-09 Thread n . puttock
LGTM. http://codereview.appspot.com/4801083/diff/14002/scm/define-grobs.scm File scm/define-grobs.scm (right): http://codereview.appspot.com/4801083/diff/14002/scm/define-grobs.scm#newcode933 scm/define-grobs.scm:933: (end-on-accidental . #t) trailing whitespace http://codereview.appspot.com/4

Re: Fixes issue 40. (issue4801083)

2011-08-09 Thread n . puttock
On 2011/08/09 18:23:42, mike_apollinemike.com wrote: Good call - I've uploaded a new patch that uses "accidental-grob" (don't know why I didn't think of that before...too much code on the brain!). The latest patch set needs rebasing against master. Cheers, Neil http://codereview.appspot.co

Re: Prevents lilypond from segfaulting with add-footer = ##f (issue4832046)

2011-08-09 Thread n . puttock
On 2011/08/09 06:47:11, MikeSol wrote: Fix pushed as 435b36a3e6576cebf794d815ae7cc78b652180f9. The warning message still needs tweaking: warning (_ ("must have a footer to add footnotes")) http://codereview.appspot.com/4832046/ ___ lilypond-devel

Re: Fixes issue 40. (issue4801083)

2011-08-09 Thread n . puttock
http://codereview.appspot.com/4801083/diff/8001/input/regression/glissando-accidental.ly File input/regression/glissando-accidental.ly (right): http://codereview.appspot.com/4801083/diff/8001/input/regression/glissando-accidental.ly#newcode8 input/regression/glissando-accidental.ly:8: a1 \glissa

Deprecate \fermataMarkup for full-bar rests. (issue4672059)

2011-08-07 Thread n . puttock
Reviewers: , Message: Hi, I've always been a bit annoyed by this, so I though it would be a nice enhancement to allow scripts to work on full-bar rests just like ordinary markup. The syntax constructor already makes a half-hearted attempt to add scripts, but fails in trying to set the 'text pro

Make accidental styles available as context mods. (issue4819064)

2011-08-07 Thread n . puttock
Reviewers: , Message: Hi, This patch adds context modification identifiers for all supported accidental styles. This should make it much easier for users to set a style globally, since there's no need to work out what each style sets internally. I've followed Kieren's example here, http://list

Re: Add Notation appendix for context mod identifiers. (issue4794057)

2011-08-07 Thread n . puttock
On 2011/08/03 20:06:49, Graham Percival wrote: LGTM, fantastic work as always. Thanks! Pushed: 33f71ee2c062b4e377e146920ef48ea8c41d6fe8 Cheers, Neil http://codereview.appspot.com/4794057/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http

  1   2   3   4   5   6   >