Re: Web: Easier Editing - added bwwtolily (issue 7098055)

2013-01-29 Thread dak
On 2013/01/28 22:18:57, benko.pal wrote: bwwtolilly is a typo, isn't it? (diff view doesn't work for some reason.) Typo fixed as commit 1b15a2096770f0393c799097afa4a2dcf28ed213 Author: David Kastrup Date: Tue Jan 29 15:01:05 2013 +0100 Typo bwwtolilly -> bwwtolily https://codereview.a

Re: Make documentation reflect presence of \tuplet (issue 7220052)

2013-01-30 Thread dak
On 2013/01/29 17:25:19, Keith wrote: https://codereview.appspot.com/7220052/diff/4001/Documentation/learning/common-notation.itely#newcode506 Documentation/learning/common-notation.itely:506: ratio of the number of notes to play in relation to the nominal "... The fraction is the number of no

Re: Make documentation reflect presence of \tuplet (issue 7220052)

2013-01-30 Thread dak
On 2013/01/30 10:41:50, dak wrote: We have a fundamental difference in our terminology. For you, a triplet consists of three notes. For me, a triplet consists of a single note, three of which make up a triplet group. I've mostly skirted the issue in the current wording.

Re: Caches the interior skylines of vertical axis groups and systems. (issue 7185044)

2013-02-01 Thread dak
As previously, there are no comments whatsoever putting the code into perspective. This is an amorphous heap of code without an attempt of explaining its design or inner logic. There is a single function comment giving a glimpse of what that function is supposed to do. However, there is no expla

Re: \note-by-number supports flag-styles (issue 7231072)

2013-02-02 Thread dak
https://codereview.appspot.com/7231072/diff/1/scm/define-markup-commands.scm File scm/define-markup-commands.scm (right): https://codereview.appspot.com/7231072/diff/1/scm/define-markup-commands.scm#newcode3366 scm/define-markup-commands.scm:3366: (string-append "flags." I'd rather use (format

Re: Caches the interior skylines of vertical axis groups and systems. (issue 7185044)

2013-02-03 Thread dak
On 2013/02/03 08:45:13, mike7 wrote: On 1 févr. 2013, at 16:04, mailto:d...@gnu.org wrote: > As previously, there are no comments whatsoever putting the code into > perspective. This is an amorphous heap of code without an attempt of > explaining its design or inner logic. There is a sin

Re: Issue 754: \transpose should not affect \transposition (issue 7304044)

2013-02-06 Thread dak
On 2013/02/06 07:11:34, Keith wrote: This works, including midi and cues between transposing instruments. I somewhat recommend doing only patch set 1 and the update to the regtest documentation in set 3. (Or, at least put the change in patchset 2 as a separate commit, with a convert-ly rul

Re: Issue 754: \transpose should not affect \transposition (issue 7304044)

2013-02-06 Thread dak
https://codereview.appspot.com/7304044/diff/6001/ly/music-functions-init.ly File ly/music-functions-init.ly (right): https://codereview.appspot.com/7304044/diff/6001/ly/music-functions-init.ly#newcode491 ly/music-functions-init.ly:491: instrumentSwitch = On 2013/02/06 07:11:34, Keith wrote: If

Re: Issue 754: \transpose should not affect \transposition (issue 7304044)

2013-02-07 Thread dak
On 2013/02/07 07:05:34, Keith wrote: On 2013/02/06 11:15:41, dak wrote: > I don't see > the point in _not_ inverting the instrumentTransposition sign: > our documentation gets it wrong, and all regtests are wrong. I cannot follow the multiple negatives, but I can see some poi

Re: Issue 754: don't transpose generic property-setting music commands (issue 7303057)

2013-02-08 Thread dak
On 2013/02/08 04:46:27, Keith wrote: Yesterday's patch was better. I beg to differ. It did not address instrument definitions, or manual settings of instrumentTransposition. The 'untransposable property is more closely related to the source of the events (\transposition or \instrumentSwitc

Re: Issue 754: don't transpose generic property-setting music commands (issue 7303057)

2013-02-08 Thread dak
On 2013/02/08 08:40:29, dak wrote: On 2013/02/08 04:46:27, Keith wrote: > Yesterday's patch was better. I beg to differ. It did not address instrument definitions, or manual settings Concretely: can you show an example of not-just-academical LilyPond source that would be chang

Re: Issue 754: don't transpose generic property-setting music commands (issue 7303057)

2013-02-09 Thread dak
On 2013/02/09 05:46:45, Keith wrote: On Fri, 08 Feb 2013 01:20:59 -0800, <mailto:d...@gnu.org> wrote: > On 2013/02/08 08:40:29, dak wrote: >> On 2013/02/08 04:46:27, Keith wrote: >> > Yesterday's patch was better. > >> I beg to differ. It did not addr

Re: This is actually a whole bunch of commits: (issue 7303067)

2013-02-11 Thread dak
Reviewers: Keith, Message: On 2013/02/10 20:42:35, Keith wrote: https://codereview.appspot.com/7303067/diff/1/Documentation/notation/editorial.itely File Documentation/notation/editorial.itely (right): https://codereview.appspot.com/7303067/diff/1/Documentation/notation/editorial.itely#newco

Re: Issue 754: don't transpose generic property-setting music commands (issue 7303057)

2013-02-11 Thread dak
On 2013/02/11 06:18:57, Keith wrote: On 2013/02/09 08:54:03, dak wrote: > It doesn't matter. LilyPond is still being actively maintained. If a > feature is required, it can be implemented properly, with a proper > interface, _without_ resorting to exploits. > I was hop

Re: Issue 754: don't transpose generic property-setting music commands (issue 7303057)

2013-02-11 Thread dak
On 2013/02/11 09:25:20, dak wrote: On 2013/02/11 06:18:57, Keith wrote: > On 2013/02/09 08:54:03, dak wrote: > > > It doesn't matter. LilyPond is still being actively maintained. If a > > feature is required, it can be implemented properly, with a proper > > inte

Eliminates pure-print-callbacks list (issue 7300082)

2013-02-11 Thread dak
https://codereview.appspot.com/7300082/diff/1/scm/define-grobs.scm File scm/define-grobs.scm (right): https://codereview.appspot.com/7300082/diff/1/scm/define-grobs.scm#newcode2570 scm/define-grobs.scm:2570: (Y-extent . ,pure-safe-stencil-height) Are there grobs that don't have this definition o

Re: Eliminates pure-print-callbacks list (issue 7300082)

2013-02-11 Thread dak
On 2013/02/11 17:01:17, mike7 wrote: On 11 févr. 2013, at 16:29, mailto:d...@gnu.org wrote: > scm/output-lib.scm:61: (define-public pure-safe-stencil-height > Perhaps add any information about the name? Its name claims to produce > a pure value, but it actually outputs callbacks both for pu

Re: Issue 754: don't transpose generic property-setting music commands (issue 7303057)

2013-02-11 Thread dak
On 2013/02/11 20:53:53, Keith wrote: On 2013/02/11 12:39:02, dak wrote: > On 2013/02/11 09:25:20, dak wrote: > > On 2013/02/11 06:18:57, Keith wrote: > > > > > > I was hoping to be able to provide a backward-compatibility function, as in > > I did not even

Re: Issue 3174: Implement \accidental markup and \pitched command for transposable accidental markups (issue 7323060)

2013-02-14 Thread dak
Reviewers: Trevor Daniels, thomasmorley65, Message: On 2013/02/13 23:27:55, thomasmorley65 wrote: On 2013/02/13 22:56:30, Trevor Daniels wrote: > I definitely prefer the alternative you suggest in para 3. > You would not then need the warning, which is only necessary due to the > complicated 2

Re: Caches the interior skylines of vertical axis groups and systems. (issue 7185044)

2013-02-14 Thread dak
I've not really reviewed everything here, just highlights. Regarding the commenting problems: it is important for the reviewer or maintainer or rereader to get the gist of the story. Much of the LilyPond codebase has been written with a total disregard to people not present during the writing.

Re: let beam thickness depend on line thickness (fix 3173) (issue 7312091)

2013-02-14 Thread dak
https://codereview.appspot.com/7312091/diff/1/scm/define-grob-properties.scm File scm/define-grob-properties.scm (right): https://codereview.appspot.com/7312091/diff/1/scm/define-grob-properties.scm#newcode93 scm/define-grob-properties.scm:93: (beam-thickness ,number-pair? "Beam thickness. It i

Re: Standardizes use of empty extents in pure heights and skylines. (issue 7310075)

2013-02-16 Thread dak
https://codereview.appspot.com/7310075/diff/12001/lily/separation-item.cc File lily/separation-item.cc (right): https://codereview.appspot.com/7310075/diff/12001/lily/separation-item.cc#newcode156 lily/separation-item.cc:156: // If these two uses of inf combine, leave the empty extent. The descr

Re: Clean up bar-line.scm some more (issue 7350044)

2013-02-17 Thread dak
On 2013/02/17 18:57:13, marc wrote: On 2013/02/17 18:51:23, dak wrote: > Remove unused variable LGTM, looks much more scheme-ish than before! Well, there is not a single set!, there _is_ a named let for building two lists in tandem (unfold does not work really for building more than

Re: Caches the interior skylines of vertical axis groups and systems. (issue 7185044)

2013-02-18 Thread dak
On 2013/02/15 06:37:20, mike7 wrote: https://codereview.appspot.com/7185044/diff/32003/input/regression/tuplet-number-outside-staff-positioning.ly#newcode15 > input/regression/tuplet-number-outside-staff-positioning.ly:15: > \override TupletBracket.Y-offset = > #ly:side-position-interface::calc

Re: Issue 3137: 2 Errors in german translation of lilypond.org (issue 7338047)

2013-02-19 Thread dak
https://codereview.appspot.com/7338047/diff/1/Documentation/de/web/community.itexi File Documentation/de/web/community.itexi (right): https://codereview.appspot.com/7338047/diff/1/Documentation/de/web/community.itexi#newcode307 Documentation/de/web/community.itexi:307: Fehlermeldungen, die nicht

Re: Avoid excessive number of dots in chords (#3179). (issue 7319049)

2013-02-19 Thread dak
On 2013/02/19 11:01:16, lemzwerg wrote: > Well, Gould is showing the usual case, where notes have > Dots.staff-position = 0. You were asking for opinions > on the optimal way to behave for users who override > Dots.staff-position. You could generalize by using the > requested Dots position 'p'

Re: Caches the interior skylines of vertical axis groups and systems. (issue 7185044)

2013-02-19 Thread dak
On 2013/02/19 14:57:48, mike7 wrote: The current documentation on outside-staff grobs reads: "Intuitively, there are some objects in musical notation that belong to the staff and there are other objects that should be placed outside the staff. Objects belonging outside the staff include things s

Re: Uses single algorithm for side-position spacing. (issue 6827072)

2013-02-19 Thread dak
https://codereview.appspot.com/6827072/diff/38003/lily/multi-measure-rest.cc File lily/multi-measure-rest.cc (right): https://codereview.appspot.com/6827072/diff/38003/lily/multi-measure-rest.cc#newcode123 lily/multi-measure-rest.cc:123: Spanner *sp = dynamic_cast (me); The compiler complains th

Re: Eliminates pure-print-callbacks list (issue 7300082)

2013-02-20 Thread dak
On 2013/02/11 17:15:47, dak wrote: On 2013/02/11 17:01:17, mike7 wrote: > On 11 févr. 2013, at 16:29, mailto:d...@gnu.org wrote: > > scm/output-lib.scm:61: (define-public pure-safe-stencil-height > > Perhaps add any information about the name? Its name claims to produce &

Fixes annotate spacing (issue 7313082)

2013-02-20 Thread dak
https://codereview.appspot.com/7313082/diff/1/scm/skyline.scm File scm/skyline.scm (right): https://codereview.appspot.com/7313082/diff/1/scm/skyline.scm#newcode23 scm/skyline.scm:23: (define-public (skyline-pair::useable? skyp) So why is this not non-empty-skyline-pair? or skyline-pair-and-non-

Re: Fixes annotate spacing (issue 7313082)

2013-02-20 Thread dak
On 2013/02/20 15:27:49, dak wrote: https://codereview.appspot.com/7313082/diff/1/scm/skyline.scm File scm/skyline.scm (right): https://codereview.appspot.com/7313082/diff/1/scm/skyline.scm#newcode23 scm/skyline.scm:23: (define-public (skyline-pair::useable? skyp) So why is this not non-empty

Re: Make ly:make-unpure-pure-container accept a single callback (issue 7382046)

2013-02-21 Thread dak
Reviewers: MikeSol, Message: On 2013/02/21 19:35:35, MikeSol wrote: 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

Re: Standardizes use of empty extents in pure heights and skylines. (issue 7310075)

2013-02-22 Thread dak
On 2013/02/18 02:59:58, Keith wrote: On Sun, 17 Feb 2013 02:07:19 -0800, mailto:m...@mikesolomon.org wrote: The classic error with floating point is to do math on the pair, something like (left . right) + shift/3.0 , where left==right, and then ask is_empty().

Re: Eliminates pure-print-callbacks list (issue 7300082)

2013-02-22 Thread dak
On 2013/02/16 22:16:28, Keith wrote: It does looks reasonable to put the promise that a stencil does not change with line-spacing, near the definiton fo the stencil, rather than in a separate list. See http://code.google.com/p/lilypond/issues/detail?id=3200> for a more transparent way to put

Re: Uses only unpure-pure containers to articulate unpure-pure relationships. (issue 7377046)

2013-02-23 Thread dak
On 2013/02/23 05:35:50, MikeSol wrote: 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 us

Re: Caches the interior skylines of vertical axis groups and systems. (issue 7185044)

2013-02-23 Thread dak
https://codereview.appspot.com/7185044/diff/45018/Documentation/changes.tely File Documentation/changes.tely (right): https://codereview.appspot.com/7185044/diff/45018/Documentation/changes.tely#newcode68 Documentation/changes.tely:68: @code{ly:side-position-interface::calc-outside-staff-y-offse

Re: Caches the interior skylines of vertical axis groups and systems. (issue 7185044)

2013-02-23 Thread dak
https://codereview.appspot.com/7185044/diff/45018/input/regression/tuplet-bracket-outside-staff-priority.ly File input/regression/tuplet-bracket-outside-staff-priority.ly (right): https://codereview.appspot.com/7185044/diff/45018/input/regression/tuplet-bracket-outside-staff-priority.ly#newcode1

Re: Caches the interior skylines of vertical axis groups and systems. (issue 7185044)

2013-02-24 Thread dak
Stupid question: could you not just check whether outside-staff-priority has been set, and if it has, pass the buck to the right kind of callback automatically? That way, the user does not need to meddle with callbacks himself. https://codereview.appspot.com/7185044/ ___

Re: Caches the interior skylines of vertical axis groups and systems. (issue 7185044)

2013-02-24 Thread dak
On 2013/02/24 13:27:39, mike7 wrote: On 24 févr. 2013, at 12:40, mailto:d...@gnu.org wrote: > Stupid question: could you not just check whether > outside-staff-priority has been set, and if it has, pass the buck > to the right kind of callback automatically? That way, the user > does not need

Re: changes.tely: deal with \transposition and instrumentTransposition changes (issue 7404046)

2013-02-24 Thread dak
Hope I interpreted Trevor's comment correctly. The roposed "new" order did not occur to me since it violates causation: the (now) first change is dependent on the second one. However, that is on an implementation level and probably not interesting to the user. https://codereview.appspot.com/74

Re: Caches the interior skylines of vertical axis groups and systems. (issue 7185044)

2013-02-25 Thread dak
On 2013/02/24 16:20:53, mike7 wrote: On 24 févr. 2013, at 17:18, David Kastrup wrote: > "m...@mikesolomon.org" writes: > >> On 24 févr. 2013, at 16:37, mailto:d...@gnu.org wrote: >> >>> On 2013/02/24 13:27:39, mike7 wrote: On 24 févr. 20

Re: changes.tely: deal with \transposition and instrumentTransposition changes (issue 7404046)

2013-02-25 Thread dak
On 2013/02/25 16:02:41, Trevor Daniels wrote: Not quite, but it is hardly a point worth labouring over. Changes are listed with the most recent at the top, and 'previously' means 'earlier in time', so it ought to refer to items lower in the list. Our changes list is not really ordered in stri

Re: Issue 3205: opening bar check causes crash if \score contains the \midi block (issue 7377057)

2013-02-27 Thread dak
Reviewers: Keith, Message: On 2013/02/27 07:07:58, Keith wrote: I tried skipTypesetting and showFirstLength in different situations. They seem to work. It would be nice to know at least /how/ this change causes Score_performer::prepare() to be called. On the other hand, it seems that the

Re: changes.tely: deal with \transposition and instrumentTransposition changes (issue 7404046)

2013-02-27 Thread dak
On 2013/02/27 07:40:59, Keith wrote: On 2013/02/27 05:58:58, Keith wrote: > (and @code{\whateverItWasCalled f'} is available Oops. It was \oldTransposition but it was not put into LilyPond. Since the sign of instrumentTransposition has been inverted, it would require serious trickery or

Re: Uses only unpure-pure containers to articulate unpure-pure relationships. (issue 7377046)

2013-02-27 Thread dak
https://codereview.appspot.com/7377046/diff/17001/input/regression/scheme-text-spanner.ly File input/regression/scheme-text-spanner.ly (right): https://codereview.appspot.com/7377046/diff/17001/input/regression/scheme-text-spanner.ly#newcode129 input/regression/scheme-text-spanner.ly:129: side-p

Re: Uses only unpure-pure containers to articulate unpure-pure relationships. (issue 7377046)

2013-02-27 Thread dak
On 2013/02/27 23:00:48, mike7 wrote: On 27 févr. 2013, at 19:06, mailto:d...@gnu.org wrote: > > https://codereview.appspot.com/7377046/diff/17001/input/regression/scheme-text-spanner.ly > File input/regression/scheme-text-spanner.ly (right): > > https://codereview.appspot.com/7377046/diff

Re: Make test-output-distance regtest contain span bars (issue 7400057)

2013-02-27 Thread dak
Reviewers: lemzwerg, thomasmorley65, Message: On 2013/02/28 00:45:57, thomasmorley65 wrote: LGTM One suggestion: https://codereview.appspot.com/7400057/diff/1/input/regression/test-output-distance.ly File input/regression/test-output-distance.ly (right): https://codereview.appspot.com/

Re: Allows slurs to break at barlines. (issue 7424049)

2013-03-01 Thread dak
On 2013/03/01 10:56:57, MikeSol wrote: Implements inheritence in slur engravers to avoid code dups Mike, can you make that a separate review placed on issue 2689? Things become too tangled up to review on their respective merits otherwise. https://codereview.appspot.com/7424049/

Allows inheritence for slur engravers (issue 7437048)

2013-03-01 Thread dak
Thanks for the rebase! https://codereview.appspot.com/7437048/diff/1/lily/slur-engraver.cc File lily/slur-engraver.cc (right): https://codereview.appspot.com/7437048/diff/1/lily/slur-engraver.cc#newcode51 lily/slur-engraver.cc:51: event_name_ = "slur-event"; As a matter of style, I'd declare al

Re: Allows inheritence for slur engravers (issue 7437048)

2013-03-01 Thread dak
On 2013/03/01 14:45:27, mike7 wrote: > "doubleSlurs" is not generic but rather specific. I'd use something > like > if (double_property_name_ && to_boolean (get_property > (double_property_name_))) > instead, then we can, if desired, have a separate doublePhrasingSlur > property at some point o

Re: Allows inheritence for slur engravers (issue 7437048)

2013-03-01 Thread dak
On 2013/03/01 17:00:28, mike7 wrote: On 1 mars 2013, at 17:30, mailto:d...@gnu.org wrote: > On 2013/03/01 14:45:27, mike7 wrote: > >> > "doubleSlurs" is not generic but rather specific. I'd use something >> > like >> > if (double_property_name_ && to_boolean (get_property >> > (double_prope

Re: Includes empty skylines in minimum translation calculations (issue 7304068)

2013-03-05 Thread dak
https://codereview.appspot.com/7304068/diff/27001/lily/align-interface.cc File lily/align-interface.cc (right): https://codereview.appspot.com/7304068/diff/27001/lily/align-interface.cc#newcode74 lily/align-interface.cc:74: vector *const ret, Usage of const here is "dear compiler, I promise that

Re: Make test-output-distance regtest contain span bars (issue 7400057)

2013-03-05 Thread dak
On 2013/03/02 07:30:11, Keith wrote: LGTM. On 2013/02/28 05:57:45, dak wrote: > I think we have a special span bar at the start, The bar at the start is a SystemStartBar, which was not affected by the recent span-bar bug. You might want an extra measure, and a final bar, then. A

Re: Be more explicit about footnotes on chord constituents (issue 7038047)

2013-03-05 Thread dak
Reviewers: J_lowe, Trevor Daniels, Message: I hate it when I discover I had unpublished draft comments on an issue that has been closed long ago. I am flushing out these comments nevertheless for the record so that reviewers don't think I ignored their feedback (rather than being of different op

Change name of Patch Handler to Patch Meister in CG (issue 7498045)

2013-03-06 Thread dak
It's a mystery to me how anybody could interpret this text in any other manner, but there is no point in being less clear than possible, so LGTM. https://codereview.appspot.com/7498045/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lis

Re: Change name of Patch Handler to Patch Meister in CG (issue 7498045)

2013-03-06 Thread dak
On 2013/03/06 09:22:21, mike7 wrote: On 6 mars 2013, at 10:13, mailto:d...@gnu.org wrote: > It's a mystery to me how anybody could interpret this text in any other > manner, but there is no point in being less clear than possible, so > LGTM. > > https://codereview.appspot.com/7498045/ A

Re: Make \relative { ... } interpret the first pitch as an absolute one (issue 7546044)

2013-03-06 Thread dak
On 2013/03/07 05:58:46, Keith wrote: In order from reasonable to ridiculous, Actually, you'll find me in almost full agreement. Converting \relative{g b d} -> \relative c' {g b d} is reasonable, Yes and no. If the meaning of \relative { ... } remains unchanged, this conversion is gratui

Re: Make \relative { ... } interpret the first pitch as an absolute one (issue 7546044)

2013-03-07 Thread dak
On 2013/03/07 07:19:01, janek wrote: Hi, i like the idea of \relative interpreting the first pitch "in absolute" if there's no explicit reference pitch. However, i don't think we should promote this way of doing things: it's a shorthand, and with LilyPond it's usually better to be explicit

Re: Make \relative { ... } interpret the first pitch as an absolute one (issue 7546044)

2013-03-07 Thread dak
On 2013/03/07 07:19:01, janek wrote: Hi, i like the idea of \relative interpreting the first pitch "in absolute" if there's no explicit reference pitch. However, i don't think we should promote this way of doing things: it's a shorthand, and with LilyPond it's usually better to be expli

Re: Make \relative { ... } interpret the first pitch as an absolute one (issue 7546044)

2013-03-07 Thread dak
On 2013/03/07 09:25:11, dak wrote: Sorry for the repetition. My browser very much made it appear like it had lost the previous comment, so I rewrote it from scratch. I see no way to delete comments from Rietveld (as opposed to Codeview). https://codereview.appspot.com/7546044

Re: Make \relative { ... } interpret the first pitch as an absolute one (issue 7546044)

2013-03-07 Thread dak
On 2013/03/07 15:56:52, Trevor Daniels wrote: Looking good. Obviously the text which describes \relative will need attention too, Not just that. So far all humanly written changes are constrained to python/convertrules.ly and ly/music-functions-init.ly (the actual change of the definition).

Re: Make \relative { ... } interpret the first pitch as an absolute one (issue 7546044)

2013-03-07 Thread dak
On 2013/03/07 18:10:46, J_lowe wrote: On 2013/03/07 17:31:08, dak wrote: > On 2013/03/07 15:56:52, Trevor Daniels wrote: > > Looking good. Obviously the text which describes > > \relative will need attention too, > > Not just that. So far all humanly written change

Re: Change \transpose to \relative in ancient.itely (issue 7538043)

2013-03-07 Thread dak
On 2013/03/07 21:25:04, benko.pal wrote: https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancient.itely File Documentation/notation/ancient.itely (right): https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancient.itely#newcode952 Documentation/not

Re: Allows slurs to break at barlines. (issue 7424049)

2013-03-08 Thread dak
https://codereview.appspot.com/7424049/diff/19022/ly/spanners-init.ly File ly/spanners-init.ly (right): https://codereview.appspot.com/7424049/diff/19022/ly/spanners-init.ly#newcode114 ly/spanners-init.ly:114: breakSlur = #(make-music 'BreakSlurEvent) I am not happy with a new event type here.

Re: Allows inheritence for slur engravers (issue 7437048)

2013-03-08 Thread dak
LGTM https://codereview.appspot.com/7437048/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Allows minimum-length to work for end-of-line spanners. (issue 7453046)

2013-03-08 Thread dak
https://codereview.appspot.com/7453046/diff/1/input/regression/minimum-length-end-line.ly File input/regression/minimum-length-end-line.ly (right): https://codereview.appspot.com/7453046/diff/1/input/regression/minimum-length-end-line.ly#newcode10 input/regression/minimum-length-end-line.ly:10:

Re: Standardizes use of empty extents in pure heights and skylines. (issue 7310075)

2013-03-08 Thread dak
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 input/regression/skyline-point-extent.ly:5: even t

Re: Allows minimum-length to work for end-of-line spanners. (issue 7453046)

2013-03-09 Thread dak
On 2013/03/09 07:18:50, mike7 wrote: On 8 mars 2013, at 14:10, mailto:d...@gnu.org wrote: > > https://codereview.appspot.com/7453046/diff/1/input/regression/minimum-length-end-line.ly > File input/regression/minimum-length-end-line.ly (right): > > https://codereview.appspot.com/7453046/di

Re: Allows minimum-length to work for end-of-line spanners. (issue 7453046)

2013-03-11 Thread dak
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 _this_ override not be the default? >> > >> > https://codereview.appspot.com

Re: Harmonize point-and-click information for #xxx and $xxx (issue 7501046)

2013-03-12 Thread dak
On 2013/03/12 22:12:34, janek wrote: As usual, i have a dumb question about the commit message. What is the relation of \xxx to #xxx and $xxx? I can't quite grok the commit message :/ Uh, \xxx is something starting with a backslash, $xxx is something starting with a dollar sign, #xxx is somethi

Re: Harmonize point-and-click information for #xxx and $xxx (issue 7501046)

2013-03-13 Thread dak
On 2013/03/13 07:03:16, janek wrote: On Wed, Mar 13, 2013 at 7:06 AM, wrote: > On 2013/03/12 22:12:34, janek wrote: >> >> As usual, i have a dumb question about the commit message. >> What is the relation of \xxx to #xxx and $xxx? >> I can't quite grok the commit message :/

Re: Standardizes use of empty extents in pure heights and skylines. (issue 7310075)

2013-03-13 Thread dak
Just sent this to developer list only, so here a copy for the record: "m...@mikesolomon.org" writes: The concern before was a comment about numerical inaccuracy, but after having tested the patch, this seems not to be an issue. Like Keith pointed out, it could become one if more than one ope

Re: Harmonize point-and-click information for #xxx and $xxx (issue 7501046)

2013-03-13 Thread dak
On 2013/03/13 15:44:20, janek wrote: On Wed, Mar 13, 2013 at 9:11 AM, wrote: > I don't get it. Seriously. All of $, #, and \ are lexical elements > combining with something following behind them (and all can take an > identifier) and producing an expression or a copy of i

Re: Change \transpose to \relative in ancient.itely (issue 7538043)

2013-03-16 Thread dak
So many pitches are wrong in this patch that I think we should revert it and have it redone carefully. Possibly using some automatism (like Frescobaldi's absolute/relative conversions, making sure that one gets the starting pitch right). As it stands, this just changes too much. https://codere

Re: Make a PostEvents container class for packaging several postevents (issue 7742044)

2013-03-16 Thread dak
Reviewers: lemzwerg, Message: On 2013/03/16 19:19:57, lemzwerg wrote: LGTM. The combination `\' `\n' `\(' is probably worth a comment in the contributors' manual. If \( was defined, things would be so much easier: one would just put \( in the first column. \ \n( is the best I could manage.

Re: Allows minimum-length to work for end-of-line spanners. (issue 7453046)

2013-03-17 Thread dak
On 2013/03/17 07:10:23, MikeSol wrote: On 2013/03/11 10:18:59, dak wrote: > > There is no point in hiding the symptoms of a problem away. That only > makes things even harder in future. I don't think this is a problem blocking the current patch. It is a problem making the

Re: Change \transpose to \relative in ancient.itely (issue 7538043)

2013-03-17 Thread dak
On 2013/03/18 00:00:13, aleksandr.andreev wrote: Sorry, I should have checked the output more carefully. (Then again, Pál said above that the changes "didn't matter"). Ancient music usually tries very hard to avoid ledger lines which is one of the reasons for its multitude of clefs. So bein

Add warning about using \relative with tagged music (3253) (issue 7798045)

2013-03-18 Thread dak
https://codereview.appspot.com/7798045/diff/1/Documentation/notation/input.itely File Documentation/notation/input.itely (right): https://codereview.appspot.com/7798045/diff/1/Documentation/notation/input.itely#newcode2247 Documentation/notation/input.itely:2247: @end example That's \relative wi

Re: Allows slurs to break at barlines. (issue 7424049)

2013-03-19 Thread dak
On 2013/03/19 15:22:41, MikeSol wrote: Two backslashes After some consideration, I consider the name \broken suboptimal since it implies two pieces. Two other possibilities would be \detached and \fake. https://codereview.appspot.com/7424049/ ___ l

Re: Allows slurs to break at barlines. (issue 7424049)

2013-03-19 Thread dak
On 2013/03/19 22:37:14, wl_gnu.org wrote: >> After some consideration, I consider the name \broken suboptimal since >> it implies two pieces. Two other possibilities would be \detached and >> \fake. > > I vote for detached. I vote for \broken. For me, it doesn't imply two pieces. This w

Re: Allows slurs to break at barlines. (issue 7424049)

2013-03-20 Thread dak
On 2013/03/20 12:35:37, Ian Hulin (gmail) wrote: \fake and \broken are concise but "feel" wrong, implying something's wrong with something else but the name doesn't describe it. Without pointing out what makes you think they feel wrong, that's not helpful. I think a function name where we've

Re: Allows slurs to break at barlines. (issue 7424049)

2013-03-20 Thread dak
Yet another possibility would be \inner but that implies a hierarchy. More accurate would be \continued. https://codereview.appspot.com/7424049/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-deve

Re: Allows slurs to break at barlines. (issue 7424049)

2013-03-20 Thread dak
And yet another would be \silent as a sort of negative variant of \visual. https://codereview.appspot.com/7424049/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: report a programming error when trying to align on empty parent (issue 7533046)

2013-03-20 Thread dak
https://codereview.appspot.com/7533046/diff/3001/lily/self-alignment-interface.cc File lily/self-alignment-interface.cc (right): https://codereview.appspot.com/7533046/diff/3001/lily/self-alignment-interface.cc#newcode63 lily/self-alignment-interface.cc:63: programming_error ("cannot align on se

Re: report a programming error when trying to align on empty parent (issue 7533046)

2013-03-21 Thread dak
On 2013/03/21 08:04:42, janek wrote: So, you think that we shouldn't report any programming error in these alignment funcitons since an empty extent doesn't prevent us from doing our job (i.e. aligning)? Well, with an empty extent it would appear our job of aligning _self_ is already done

Re: Don't report a programming error when trying to align grob with an empty extent (issue 7533046)

2013-03-24 Thread dak
On 2013/03/24 12:37:14, Trevor Daniels wrote: I was a little concerned that problems might result when a non-empty stencil was given an empty extent, but as this passes all tests it looks like this fear was unfounded. I don't think your fear is unfounded: there just does not seem to be any sens

Re: Eliminates side poisition calculations for system-start-text grobs. (issue 7574048)

2013-03-24 Thread dak
https://codereview.appspot.com/7574048/diff/5001/input/regression/instrument-name-x-align.ly File input/regression/instrument-name-x-align.ly (right): https://codereview.appspot.com/7574048/diff/5001/input/regression/instrument-name-x-align.ly#newcode33 input/regression/instrument-name-x-align.l

Re: Make a PostEvents container class for packaging several postevents (issue 7742044)

2013-03-24 Thread dak
https://codereview.appspot.com/7742044/diff/5001/scm/define-music-types.scm File scm/define-music-types.scm (right): https://codereview.appspot.com/7742044/diff/5001/scm/define-music-types.scm#newcode77 scm/define-music-types.scm:77: \n(no direction specified), and where @code{y} is an articulat

Re: Fix composition of markup lists containing markup command list calls (issue 7799048)

2013-03-24 Thread dak
I just changed the summary of the commit messages since one commit in the middle referred to a syntax that was only provided by a commit later in the sequence. Just a technicality. https://codereview.appspot.com/7799048/ ___ lilypond-devel mailing lis

Re: Eliminates side poisition calculations for system-start-text grobs. (issue 7574048)

2013-03-24 Thread dak
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): > > https://codereview.appspot.com/757404

Re: Removes '-signs in vectors - follow-up (issue 7834043)

2013-03-24 Thread dak
https://codereview.appspot.com/7834043/diff/2001/Documentation/snippets/preventing-final-mark-from-removing-final-tuplet.ly File Documentation/snippets/preventing-final-mark-from-removing-final-tuplet.ly (right): https://codereview.appspot.com/7834043/diff/2001/Documentation/snippets/preventing-

Re: Removes '-signs in vectors - follow-up (issue 7834043)

2013-03-24 Thread dak
https://codereview.appspot.com/7834043/diff/2001/Documentation/snippets/new/applying-note-head-styles-depending-on-the-step-of-the-scale.ly File Documentation/snippets/new/applying-note-head-styles-depending-on-the-step-of-the-scale.ly (right): https://codereview.appspot.com/7834043/diff/2001/Do

Re: Issue 2922: \keepWithTag - update the existing docs in the manual to reflect new changes (issue 7494049)

2013-03-25 Thread dak
Reviewers: Trevor Daniels, Message: On 2013/03/24 21:36:42, Trevor Daniels wrote: I can still learn something new about LilyPond! Thanks David - LGTM (with a further suggestion) https://codereview.appspot.com/7494049/diff/1/Documentation/notation/input.itely File Documentation/notation/input

Re: Removes '-signs in vectors - follow-up (issue 7834043)

2013-03-26 Thread dak
https://codereview.appspot.com/7834043/diff/13001/Documentation/snippets/new/applying-note-head-styles-depending-on-the-step-of-the-scale.ly File Documentation/snippets/new/applying-note-head-styles-depending-on-the-step-of-the-scale.ly (right): https://codereview.appspot.com/7834043/diff/13001/

Re: Removes '-signs from vectors (issue 6923043)

2013-03-27 Thread dak
On 2013/03/27 21:53:39, janek wrote: LGTM. I suppose that one of David's changes made this patch possible; if you know which one it is it would be good to note this (but if you don't know i think it's not worth the trouble to search). No thanks to me: #(1 2 3) always has been the Scheme

Re: Additions in event-listener.ly (issue 8165043)

2013-03-29 Thread dak
https://codereview.appspot.com/8165043/diff/2001/ly/event-listener.ly File ly/event-listener.ly (right): https://codereview.appspot.com/8165043/diff/2001/ly/event-listener.ly#newcode69 ly/event-listener.ly:69: (eq? 0 (ly:moment-grace-numerator moment)) On 2013/03/29 19:21:31, janek wrote: Out o

Re: Add Changes entries for \temporary, \omit, \hide, \single, multiple tags (issue 8187044)

2013-03-30 Thread dak
Reviewers: janek, thomasmorley65, Message: I'll address the other points presently, or on Tuesday, depending on my connectivity. https://codereview.appspot.com/8187044/diff/1/Documentation/changes.tely File Documentation/changes.tely (right): https://codereview.appspot.com/8187044/diff/1/Docum

Re: Restructure parsing of reverts to avoid ambiguities in relation to BACKUP (issue 8103043)

2013-04-02 Thread dak
Reviewers: janek, Message: On 2013/04/02 21:23:16, janek wrote: I'm not sure what the previous ambiguities were A music expression could start with BACKUP (an artificially inserted token never present in the source code itself) due to these rules. I needed BACKUP in a different context and go

Re: Restructure parsing of reverts to avoid ambiguities in relation to BACKUP (issue 8103043)

2013-04-02 Thread dak
On 2013/04/02 21:51:20, janek wrote: Thanks fot the explanation! Could you paste it into commit message? Have you taken a look at the actual diff? It is just factoring out one subexpressions of revert into revert_arg_backup, making for about three trivial changes. Commit messages are cheap,

Re: Allows minimum-length to work for end-of-line spanners. (issue 7453046)

2013-04-02 Thread dak
On 2013/04/02 22:34:53, janek wrote: I'll risk joining the discussion. I see valid points from both of you. I agree that it's better to fix a broken design than to patch it with red tape. It isn't patched. minimum-length is used in multiple contexts/interfaces, and Mike's patch muddies t

<    3   4   5   6   7   8   9   10   11   12   >