Re: Avoid Scheme-computed Skyline_pair values to get collected before use (issue 12708048)

2013-08-11 Thread dak
Reviewers: Keith, https://codereview.appspot.com/12708048/diff/1/lily/side-position-interface.cc File lily/side-position-interface.cc (left): https://codereview.appspot.com/12708048/diff/1/lily/side-position-interface.cc#oldcode216 lily/side-position-interface.cc:216: Skyline my_dim; On 2013/08

Re: Avoid Scheme-computed Skyline_pair values to get collected before use (issue 12708048)

2013-08-11 Thread dak
On 2013/08/11 20:41:48, Keith wrote: On Sun, 11 Aug 2013 12:46:04 -0700, wrote: > https://codereview.appspot.com/12708048/diff/1/lily/side-position-interface.cc > File lily/side-position-interface.cc (left): > > https://codereview.appspot.com/12708048/diff/1/lily/sid

Re: Avoid Scheme-computed Skyline_pair values to get collected before use (issue 12708048)

2013-08-12 Thread dak
On 2013/08/12 05:50:10, Keith wrote: On 2013/08/11 21:07:03, dak wrote: > It's conceivable that this change helped, > though there don't seem to be a lot of opportunities > for garbage collection between return of the SCM > value and use of the Skyline_pair. There is

Re: Make several special characters with or without backslash "non-special" (issue 12432043)

2013-08-14 Thread dak
On 2013/08/14 11:43:08, janek wrote: On 2013/08/14 11:41:43, janek wrote: > A general question: what about using -' or -, for staccatissimo? For me they > seem to be somewhat more intuitive. Additional advantage would be that they don't require Shift to produce on standard keyboard. Jan

Re: trill-pitch-engraver: acknowledge stems and flags; issue 3465 (issue 12844043)

2013-08-14 Thread dak
I'm in a rush right now so can't test. But would that even help with whole notes and breves? https://codereview.appspot.com/12844043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Make several special characters with or without backslash "non-special" (issue 12432043)

2013-08-14 Thread dak
On 2013/08/14 12:18:12, janek wrote: 2013/8/14 : > I crosschecked with the parser and it would not object to either. The > only argument _for_ such a change would be more visual similarity rather > than mnemonic value and that rules out -, pretty definitely. One could > talk about -' a

Re: Remove 'thickness from LedgerLineSpanner interface. (issue 12945044)

2013-08-15 Thread dak
On 2013/08/15 06:59:58, Mark Polesky wrote: On 2013/08/14 21:35:16, Trevor Daniels wrote: > Should this not be @ref{...}? No. @rinternals{...}. See http://lilypond.org/doc/v2.17/Documentation/contributor/syntax-survey#cross-references - Mark That documentation states as first entry: @

Re: Make several special characters with or without backslash "non-special" (issue 12432043)

2013-08-15 Thread dak
On 2013/08/15 09:22:12, Ian Hulin wrote: Hi David and Janek, On 14/08/13 14:30, mailto:d...@gnu.org wrote: > On 2013/08/14 12:18:12, janek wrote: >> 2013/8/14 : > >> would there be a technical problem to have /both/ -! and -' as >> shorthands for staccato? > > Technical? Not really except that

Re: Issue 3491: Add \displayMarkup command. (issue 12732043)

2013-08-15 Thread dak
https://codereview.appspot.com/12732043/diff/7001/Documentation/extending/programming-interface.itely File Documentation/extending/programming-interface.itely (right): https://codereview.appspot.com/12732043/diff/7001/Documentation/extending/programming-interface.itely#newcode650 Documentation/e

Re: Issue 3491: Add \displayMarkup command. (issue 12732043)

2013-08-15 Thread dak
On 2013/08/15 11:24:15, thomasmorley651 wrote: On 2013/08/15 10:36:06, dak wrote: > Also, the only thing that makes this "\displayMarkup" rather than > "\displayScheme" is the restriction of the argument type to "markup?". Perhaps > it woul

Re: Issue 3491: Add \displayMarkup command. (issue 12732043)

2013-08-15 Thread dak
ianhuli...@gmail.com writes: Use David's wording for EM with some tweaks. Re \displayMarkup \displayScheme: Markup needs some special-casing as we have our own home-brew customisable/extensible interpreter in there for interpreting \markup arguments. The markup interpreter sometimes does som

Re: Let Skyline's copy constructor use whole-sale copy construction of members (issue 12747043)

2013-08-15 Thread dak
Reviewers: benko.pal, Message: On 2013/08/15 19:00:39, benko.pal wrote: LGTM, but then why not let the compiler generate the copy constructor? Good point. Description: Let Skyline's copy constructor use whole-sale copy construction of members Originally connected with issue 3490, now separat

Re: Make several special characters with or without backslash "non-special" (issue 12432043)

2013-08-15 Thread dak
"Keith OHara" writes: '!' also looks like a staccatissimo, is a stop character so conceptually related to the '.' for staccato, conflicts less with the use of ''' in pitches: e'4-! cs-! bf-! g-! e'-! cs-! \tuplet3/2{bf-> g-> e->} e'4-' cs-' bf-' g-' e'-' cs-' \tuplet3/2{bf-> g-> e->} indi

Re: Issue 3491: Add \displayMarkup command. (issue 12732043)

2013-08-15 Thread dak
On 2013/08/16 02:38:49, Mark Polesky wrote: David Kastrup wrote: >> Any chance to join them completely? > > Not yet. It's a long-term goal. And of course, there are a > few things that are not easily reconciled: compare > \void\displayScheme -5 > with > \void\displayMusic -5 Well, I played a

Re: Issue 3491: Add \displayMarkup command. (issue 12732043)

2013-08-20 Thread dak
This looks fine for committing to me. There are several obvious followup issues/commits making a lot of sense afterwards, however. One is the obvious complement with \displayLilyScheme which makes sense to do before the others, namely providing a notice in Documentation/changes.itely and making

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-08-24 Thread dak
On 2013/08/24 08:05:13, mike7 wrote: On 24 août 2013, at 10:18, mailto:lemzw...@googlemail.com wrote: > LGTM, except of using `surrogate' in the name. Given that the stencil > dimensions are actively overridden on request, I would like rather like > having `override'. For me, the word `s

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-08-24 Thread dak
On 2013/08/24 10:05:17, mike7 wrote: The stencil command takes the skyline of stencil X and uses it as a replacement for skyline Y. We could use replacement-skyline-stencil as the command. I'd just use skyline-stencil here. If specified, it is the stencil used for skylines. https://coderev

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-08-24 Thread dak
On 2013/08/24 10:31:38, dak wrote: On 2013/08/24 10:05:17, mike7 wrote: > > The stencil command takes the skyline of stencil X and uses it as a replacement > for skyline Y. We could use replacement-skyline-stencil as the command. I'd just use skyline-stencil here. If specif

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-08-24 Thread dak
On 2013/08/24 13:19:30, mike7 wrote: The question I'm asking is "How can I allow the user to replace the skyline of a stencil with another shape?" I got that. But what is the actual use case for that? Before skylines became a thing, you could override the X-extent or the Y-extent of grobs,

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-08-24 Thread dak
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")) It seems awkward and error-prone to

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-08-25 Thread dak
On 2013/08/25 08:22:01, mike7 wrote: On 25 août 2013, at 09:15, mailto:k-ohara5...@oco.net wrote: > A second stencil is not a very good data structure for reserving space. > Skylines are better at this, and we can \once\override 'padding and > 'horizon-padding to get padding that follows the

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-08-25 Thread dak
On 2013/08/26 04:08:17, mike7 wrote: On 25 août 2013, at 16:04, mailto:d...@gnu.org wrote: > On 2013/08/25 08:22:01, mike7 wrote: >> On 25 août 2013, at 09:15, mailto:k-ohara5...@oco.net wrote: > >> > A second stencil is not a very good data structure for reserving > space. >> > Skylines are b

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-08-25 Thread dak
On 2013/08/26 04:51:50, mike7 wrote: On 26 août 2013, at 07:36, mailto:d...@gnu.org wrote: > On 2013/08/26 04:08:17, mike7 wrote: >> On 25 août 2013, at 16:04, mailto:d...@gnu.org wrote: > >> > On 2013/08/25 08:22:01, mike7 wrote: >> >> On 25 août 2013, at 09:15, mailto:k-ohara5...@oco.net wro

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-08-25 Thread dak
On 2013/08/26 05:25:42, mike7 wrote: On 26 août 2013, at 08:20, mailto:d...@gnu.org wrote: > Indeed. Instead of calling stencil-integrate on a "surrogate stencil" > or whatever for deriving a skyline, the respective stencil operation in > stencil-integrate will just plug in a a "surrogate

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-08-26 Thread dak
On 2013/08/26 06:42:38, mike7 wrote: On 26 août 2013, at 08:35, mailto:d...@gnu.org wrote: > So why do you talk about that in the first place? Because I'd rather spend more time implementing a solid system than less time implementing a kludge. In the context of this issue, we are curren

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-08-26 Thread dak
On 2013/08/26 09:11:39, mike7 wrote: On 26 août 2013, at 11:41, mailto:d...@gnu.org wrote: > On 2013/08/26 06:42:38, mike7 wrote: >> On 26 août 2013, at 08:35, mailto:d...@gnu.org wrote: > >> > So why do you talk about that in the first place? > >> Because I'd rather spend more time implementi

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-08-26 Thread dak
On 2013/08/26 09:42:46, mike7 wrote: In French there's a saying "Temporary solutions have a way of becoming permanent", which is why I'd like to avoid it if possible. Allow me to raise an eyebrow. What we don't want is for people to start using the stencil primitive if we know it's temporary

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-08-26 Thread dak
On 2013/08/26 10:05:25, mike7 wrote: On 26 août 2013, at 13:00, mailto:d...@gnu.org wrote: > That's bullshit since nobody assembles stencil expressions manually. I assemble stencil expressions manually. If you meddle with internals, that's your own business. > This is done using opaque

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-08-26 Thread dak
On 2013/08/26 10:18:04, mike7 wrote: I think Harm is right that, irrespective of documentation, users use what's available. If people meddle with internals instead of using provided API functions, they deserve whatever they get. If we're going to add something, we want to be sure that it h

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-08-26 Thread dak
On 2013/08/26 16:32:24, mike7 wrote: On 26 août 2013, at 13:31, mailto:d...@gnu.org wrote: > _Anything_ is "publicly useable". If that's supposed to be a criterion, > we may not change LilyPond ever again. We try to support programming > interfaces as long as it is feasible. But the way

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-08-27 Thread dak
On 2013/08/27 06:58:05, mike7 wrote: On 27 août 2013, at 09:01, "Keith OHara" wrote: > How are skylines asked-for, and is it possible to know if they were asked for while interpreting the markup ? > {c4-\markup \whiteout \pad-to-box #(-0.5 . 9) #(-0.5 . 1.5) \con

Re: Let parser accept symbols after \new, \context, \unset and implicit \set (issue 13180044)

2013-08-27 Thread dak
Reviewers: janek, Message: On 2013/08/27 07:48:08, janek wrote: Looks like a good thing to do, but what does this change mean? It means it lets the parser accept symbols after \new, \context, \unset and implicit \set. After the symbol list changes of issue 2883 (which made x.y equivalent to #

Re: Parse composite music in context modifications in \notemode (issue 12773047)

2013-08-27 Thread dak
Reviewers: janek, Message: On 2013/08/27 07:43:41, janek wrote: Hi, could you write a 2-sentence explanation why we want to do this? Janek \new Staff \with { \transposition f' } { g a b c' } Description: Parse composite music in context modifications in \notemode Please review this at

Allows inner-markup spacing using skylines. (issue 13341044)

2013-08-28 Thread dak
Where's the point? We don't use that kind of arrangement in our code, and it's not efficiently doable anyway. A conceivable application would be for determining systems spacing manually (for example, generating sequences of TeX \vspace and \break for LilyPond-book that will reserve normal rectan

Re: Allows inner-markup spacing using skylines. (issue 13341044)

2013-08-29 Thread dak
On 2013/08/29 06:57:53, MikeSol wrote: 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 p

Re: stencil-integral: use box extents specified in markup; issue 3255 (issue 9295044)

2013-08-30 Thread dak
On 2013/08/30 07:41:49, mike7 wrote: I'd still argue that (2) is the best way to go as it is a one-stop-shopping way to clear all these bugs (and perhaps more) as they arise. Since we are currently in the process of recovering from the last one-stop-shopping way to clear bugs galore which is

Re: parser: more specific error messages; issue 3300 (issue 8506043)

2013-08-30 Thread dak
https://codereview.appspot.com/8506043/diff/13001/lily/parser.yy File lily/parser.yy (right): https://codereview.appspot.com/8506043/diff/13001/lily/parser.yy#newcode2995 lily/parser.yy:2995: parser->parser_error (@1, _ ("unexpected markup, without any ^ _ or -, and outside of \\lyricmode")); Th

Re: stencil-integral: use box extents specified in markup; issue 3255 (issue 9295044)

2013-08-30 Thread dak
On 2013/08/30 08:55:15, mike7 wrote: On 30 août 2013, at 10:11, mailto:d...@gnu.org wrote: > On 2013/08/30 07:41:49, mike7 wrote: > >> I'd still argue that (2) is the best way to go as it is a > one-stop-shopping way >> to clear all these bugs (and perhaps more) as they arise. > > Since we are

Re: parser.yy: allow "scalar" to be a negative literal number (issue 13270048)

2013-09-01 Thread dak
Reviewers: Janek Warchol, https://codereview.appspot.com/13270048/diff/1/lily/parser.yy File lily/parser.yy (right): https://codereview.appspot.com/13270048/diff/1/lily/parser.yy#newcode2328 lily/parser.yy:2328: | SCM_IDENTIFIER On 2013/09/01 16:25:39, Janek Warchol wrote: just curious: this S

Re: Keep bison-generated files in sync. (issue 13466043)

2013-09-02 Thread dak
https://codereview.appspot.com/13466043/diff/2001/stepmake/stepmake/c++-rules.make File stepmake/stepmake/c++-rules.make (right): https://codereview.appspot.com/13466043/diff/2001/stepmake/stepmake/c++-rules.make#newcode16 stepmake/stepmake/c++-rules.make:16: $(BISON) -o $(subst .hh,.cc,$@) -d $

Re: Keep bison-generated files in sync. (issue 13466043)

2013-09-02 Thread dak
https://codereview.appspot.com/13466043/diff/2001/lily/GNUmakefile File lily/GNUmakefile (right): https://codereview.appspot.com/13466043/diff/2001/lily/GNUmakefile#newcode69 lily/GNUmakefile:69: $(outdir)/score.o: $(outdir)/parser.hh The additional dependencies here appear all wrong. All of th

Re: Keep bison-generated files in sync. (issue 13466043)

2013-09-03 Thread dak
https://codereview.appspot.com/13466043/diff/12001/stepmake/stepmake/c++-rules.make File stepmake/stepmake/c++-rules.make (right): https://codereview.appspot.com/13466043/diff/12001/stepmake/stepmake/c++-rules.make#newcode16 stepmake/stepmake/c++-rules.make:16: $(BISON) -d -o $(outdir)/$*.cc $<

Re: Adds dimension stencil command to correct with-dimension (issue 12957047)

2013-09-04 Thread dak
On 2013/09/04 18:36:16, Keith wrote: https://codereview.appspot.com/12957047/diff/88001/lily/stencil-integral.cc File lily/stencil-integral.cc (right): https://codereview.appspot.com/12957047/diff/88001/lily/stencil-integral.cc#newcode917 lily/stencil-integral.cc:917: } My best guess for the

Restore the default `make' target. (issue 13290047)

2013-09-06 Thread dak
https://codereview.appspot.com/13290047/diff/1/stepmake/stepmake/generic-targets.make File stepmake/stepmake/generic-targets.make (right): https://codereview.appspot.com/13290047/diff/1/stepmake/stepmake/generic-targets.make#newcode1 stepmake/stepmake/generic-targets.make:1: .PHONY : all clean b

Re: NR: explain that \type shouldn't be used in derived and midi contexts (issue 13578045)

2013-09-07 Thread dak
https://codereview.appspot.com/13578045/diff/4001/Documentation/notation/changing-defaults.itely File Documentation/notation/changing-defaults.itely (right): https://codereview.appspot.com/13578045/diff/4001/Documentation/notation/changing-defaults.itely#newcode1194 Documentation/notation/changi

Re: Make user-generated tab-clefs possible (issue 13612043)

2013-09-08 Thread dak
https://codereview.appspot.com/13612043/diff/6001/scm/parser-clef.scm File scm/parser-clef.scm (right): https://codereview.appspot.com/13612043/diff/6001/scm/parser-clef.scm#newcode23 scm/parser-clef.scm:23: (define-session-public supported-clefs Huh. I though we had that already. https://code

Re: replaced function argument 'string' by 'const string&' where it makes sense to avoid unnecessary co… (issue 13537043)

2013-09-09 Thread dak
https://codereview.appspot.com/13537043/diff/1/lily/file-name-map.cc File lily/file-name-map.cc (left): https://codereview.appspot.com/13537043/diff/1/lily/file-name-map.cc#oldcode32 lily/file-name-map.cc:32: s = file_name_map_global[s]; On 2013/09/07 04:44:02, fred1995 wrote: Then it would req

Re: Handle MultiMeasureRest direction in the part combiner like other elements (issue 13302048)

2013-09-10 Thread dak
https://codereview.appspot.com/13302048/diff/3001/input/regression/part-combine-mmrest-apart.ly File input/regression/part-combine-mmrest-apart.ly (right): https://codereview.appspot.com/13302048/diff/3001/input/regression/part-combine-mmrest-apart.ly#newcode5 input/regression/part-combine-mmres

Re: Metafont formatting instructions (issue 13400046)

2013-09-12 Thread dak
https://codereview.appspot.com/13400046/diff/6001/Documentation/contributor/feta-font.itexi File Documentation/contributor/feta-font.itexi (right): https://codereview.appspot.com/13400046/diff/6001/Documentation/contributor/feta-font.itexi#newcode127 Documentation/contributor/feta-font.itexi:127

Checks to see if tuplet brackets have bounds (issue 13582046)

2013-09-12 Thread dak
https://codereview.appspot.com/13582046/diff/1/lily/tuplet-bracket.cc File lily/tuplet-bracket.cc (right): https://codereview.appspot.com/13582046/diff/1/lily/tuplet-bracket.cc#newcode99 lily/tuplet-bracket.cc:99: if (!left || !right) Too many combinatorics involved unnecessarily. Just do if (!

Re: Checks to see if tuplet brackets have bounds (issue 13582046)

2013-09-13 Thread dak
On 2013/09/13 08:41:42, Trevor Daniels wrote: On 2013/09/13 07:09:44, mike7 wrote: > With respect to your point about null pointers and the nature of the patch, I > agree that there needs to be a better way to handle this. To me, the general > problem seems to be "what do we do when we as

Re: Allow identifiers or scheme expressions evaluating to music after \with (issue 13641043)

2013-09-15 Thread dak
Reviewers: lemzwerg, janek, Message: On 2013/09/15 18:56:55, janek wrote: As usual with my reviews: i definitely like the results, but i cannot say much about the code. thanks for another simplification! Janek It's not really a simplification: it's just moving one piece of code into anoth

Re: Allow identifiers or scheme expressions evaluating to music after \with (issue 13641043)

2013-09-15 Thread dak
On 2013/09/15 19:16:55, dak wrote: On 2013/09/15 18:56:55, janek wrote: > As usual with my reviews: i definitely like the results, but i cannot say much > about the code. > > thanks for another simplification! > Janek It's not really a simplification: it's just

Issue 3557: Fix some grammar mistakes. (issue 13373054)

2013-09-16 Thread dak
LGTM https://codereview.appspot.com/13373054/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Doc, EG: LilyPond's getting too smart for the "Inline Scheme code" section (issue 13661045)

2013-09-19 Thread dak
Reviewers: Trevor Daniels, Message: On 2013/09/19 16:23:54, Trevor Daniels wrote: Hhm. I think I'd prefer this whole section to be commented out (with an explanation) until someone can think of a convincing example. Why continue to explain to users how to do something they will now quite likely

Re: Stop \lyricmode { \skip 1.*3 } from failing. (issue 13256053)

2013-09-22 Thread dak
Reviewers: Vik Reykja, Message: On 2013/09/22 22:30:51, Vik Reykja wrote: I think a patch like this requires at least one regression test. Well, actually the original lexer.ll patch would more likely have called for a regression test as it covers a lot more change. Would you want to propose o

Re: Calculate wordwrapped/justified lines using the stencil stacking primitives (issue 13827044)

2013-09-24 Thread dak
Reviewers: janek, Message: On 2013/09/24 07:46:17, janek wrote: could you give an example of how it was failing before? Regtests change, and the new regtests seem pretty consistent. I was not going to measure this out in detail. The comments of the old code also spell out a failure case. Ch

Re: Stop \lyricmode { \skip 1.*3 } from failing. (issue 13256053)

2013-09-24 Thread dak
https://codereview.appspot.com/13256053/diff/1/lily/lexer.ll File lily/lexer.ll (right): https://codereview.appspot.com/13256053/diff/1/lily/lexer.ll#newcode588 lily/lexer.ll:588: [^|*.=$#{}\"\\ \t\n\r\f0-9][^$#{}\"\\ \t\n\r\f0-9]* { On 2013/09/24 07:44:44, janek wrote: Hmm. From my point of vi

Re: Calculate wordwrapped/justified lines using the stencil stacking primitives (issue 13827044)

2013-09-24 Thread dak
On 2013/09/24 22:01:17, janek wrote: > and you'll see that the old code has problems in the patterns ending on > 85 and 104. Thanks, this should help me understand what's going on. Without looking at the code, that's not all that likely. Interestingly, with current master (bfdebcb93bebe

Re: Stop \lyricmode { \skip 1.*3 } from failing. (issue 13256053)

2013-09-24 Thread dak
9/24 16:15:39, dak wrote: > On 2013/09/24 07:44:44, janek wrote: > > Hmm. From my point of view, this deserves some comment (but i don't insist). > > I have a problem thinking of a comment that would add any information that is > not immediately apparent from the p

Re: Stop \lyricmode { \skip 1.*3 } from failing. (issue 13256053)

2013-09-26 Thread dak
On 2013/09/26 10:51:34, janek wrote: > It does not belong in the code. It might belong in the issue tracker, > perhaps in the commit message. Just as a record of what went wrong. > But the code, as it is written, does not offer a temptation of changing > it back to the buggy previous versi

Re: Add backup option to convert-ly (Issue 3572) (issue 14040043)

2013-09-28 Thread dak
https://codereview.appspot.com/14040043/diff/6001/scripts/convert-ly.py File scripts/convert-ly.py (right): https://codereview.appspot.com/14040043/diff/6001/scripts/convert-ly.py#newcode243 scripts/convert-ly.py:243: back_up = file + '.~' + str(n) + '~' On 2013/09/29 01:00:49, Ian Hulin (gmail)

Re: Add backup option to convert-ly (Issue 3572) (issue 14040043)

2013-09-28 Thread dak
On 2013/09/29 05:46:39, dak wrote: https://codereview.appspot.com/14040043/diff/6001/scripts/convert-ly.py The name here was chosen to correspond with the numbered backups of Emacs. Emacs will recognize a numbered backup file (joining the backup scheme) only when there is a good match. In

Re: Add backup option to convert-ly (Issue 3572) (issue 14040043)

2013-09-30 Thread dak
On 2013/09/30 15:10:00, PhilEHolmes wrote: Julien - I'm not convinced that's a good idea. It would mean that, once you'd "turned numbering on", then you couldn't turn it off except by deleting all the numbered files. I think it's better to let the user select, based on the command line sw

Re: Make "Adding extra fingering with scheme" snippet do something useful (issue 13826047)

2013-09-30 Thread dak
Reviewers: thomasmorley651, Message: On 2013/09/27 23:53:50, thomasmorley651 wrote: LGTM Question: considering LSR-update, I recently added an earlier version of it to the LSR http://lsr.dsi.unimi.it/LSR/Item?id=887 Should I replace it with this one? I think that depends on whether it wil

Re: Build: Fix out-of-tree build from tarball. (issue 13854043)

2013-10-01 Thread dak
Basically, LGTM, but I might take a look at the German doc regarding the grammar inclusion. It would be preferable to be able to diss that. https://codereview.appspot.com/13854043/diff/7001/Documentation/GNUmakefile File Documentation/GNUmakefile (right): https://codereview.appspot.com/1385404

Re: Build: Fix out-of-tree build from tarball. (issue 13854043)

2013-10-02 Thread dak
https://codereview.appspot.com/13854043/diff/16001/Documentation/GNUmakefile File Documentation/GNUmakefile (right): https://codereview.appspot.com/13854043/diff/16001/Documentation/GNUmakefile#newcode283 Documentation/GNUmakefile:283: $(outdir)/contributor.texi: $(outdir)/ly-grammar.txt The pro

Re: scripts: improve strip-whitespace.py (issue 13720049)

2013-10-02 Thread dak
https://codereview.appspot.com/13720049/diff/1/scripts/auxiliar/strip-whitespace.py File scripts/auxiliar/strip-whitespace.py (right): https://codereview.appspot.com/13720049/diff/1/scripts/auxiliar/strip-whitespace.py#newcode15 scripts/auxiliar/strip-whitespace.py:15: print 'Stripping trailing

Re: Add the command \offset to LilyPond (issue 8647044)

2013-10-03 Thread dak
On 2013/04/25 13:48:55, david.nalesnik wrote: On 2013/04/23 22:09:04, dak wrote: > On 2013/04/23 20:24:57, dak wrote: > "If our search returns an anonymous procedure" is quite strained. We don't need > to _speculate_ about the identity of the anonymous procesure. I

Re: Add the command \offset to LilyPond (issue 8647044)

2013-10-03 Thread dak
On 2013/10/04 01:17:08, david.nalesnik wrote: https://codereview.appspot.com/8647044/diff/5001/scm/c++.scm File scm/c++.scm (right): https://codereview.appspot.com/8647044/diff/5001/scm/c++.scm#newcode30 scm/c++.scm:30: (every number-pair? x))) On 2013/04/23 20:24:57, dak wrote: > Isn&#

Re: Add backup option to convert-ly (Issue 3572) (issue 14040043)

2013-10-04 Thread dak
https://codereview.appspot.com/14040043/diff/6001/scripts/convert-ly.py File scripts/convert-ly.py (right): https://codereview.appspot.com/14040043/diff/6001/scripts/convert-ly.py#newcode241 scripts/convert-ly.py:241: while os.path.exists(back_up) and os.path.isfile(back_up): I'd do a repeat-unt

Re: Add backup option to convert-ly (Issue 3572) (issue 14040043)

2013-10-04 Thread dak
On 2013/10/04 14:41:28, email_philholmes.net wrote: - Original Message - From: To: ; ; ; Cc: ; Sent: Friday, October 04, 2013 3:24 PM Subject: Re: Add backup option to convert-ly (Issue 3572)

Re: Add backup option to convert-ly (Issue 3572) (issue 14040043)

2013-10-04 Thread dak
On 2013/10/04 15:09:48, email_philholmes.net wrote: > convert-ly -edn file.ly I'm really confused here. -n is the option for no-version. How is this related to backup? Sorry, probably confused this with -b. The rest of the comment stands. https://codereview.appspot.com/14040043/

Re: Add backup option to convert-ly (Issue 3572) (issue 14040043)

2013-10-04 Thread dak
On 2013/10/04 15:47:17, dak wrote: On 2013/10/04 15:09:48, http://email_philholmes.net wrote: > > convert-ly -edn file.ly > > I'm really confused here. -n is the option for no-version. How is this > related to backup? Sorry, probably confused this with -b. The

Issue 3572: convert-ly should produce several backup files for each invokation (issue 14383044)

2013-10-04 Thread dak
Reviewers: , https://codereview.appspot.com/14383044/diff/1/Documentation/usage/updating.itely File Documentation/usage/updating.itely (right): https://codereview.appspot.com/14383044/diff/1/Documentation/usage/updating.itely#newcode172 Documentation/usage/updating.itely:172: @item -h,--help Ac

Re: Add the command \offset to LilyPond (issue 8647044)

2013-10-06 Thread dak
Just before I forget: this is not a real comment on this issue. David N posted some update on this issue maybe a week ago (details are not all that important) after it lay dormant for months. I replied to that issue, and then figured I apparently didn't properly reply since Rietveld flagged "unp

Re: Move New_dynamic_engraver over the unused Dynamic_engraver (issue 14460043)

2013-10-06 Thread dak
On 2013/10/06 23:00:25, thomasmorley651 wrote: Although, I can't review C++, I've applied the patch for testing (hopefully without mistake) Testing this code { c''1_\mf^\> \break d''_\mp^\! } I've got: programming error: Spanner `Hairpin' is not fully contained in parent spanner.

Re: Move New_dynamic_engraver over the unused Dynamic_engraver (issue 14460043)

2013-10-06 Thread dak
On 2013/10/06 23:29:23, thomasmorley651 wrote: On 2013/10/06 23:10:16, dak wrote: > On 2013/10/06 23:00:25, thomasmorley651 wrote: [...] > > Or am I completely wrong and this patch has nothing to do with the problem > > above? > > In this case, you are completely wrong.

Re: Add the command \offset to LilyPond (issue 8647044)

2013-10-07 Thread dak
{\\offset} command. These properties are limited to immutable On 2013/04/23 20:24:57, dak wrote: > What does "immutable" mean here? Hopefully this rewrite is less opaque. https://codereview.appspot.com/8647044/diff/5001/input/regression/offsets.ly#newcode8 input/regression/off

Re: Add the command \offset to LilyPond (issue 8647044)

2013-10-07 Thread dak
https://codereview.appspot.com/8647044/diff/35001/input/regression/offsets.ly File input/regression/offsets.ly (right): https://codereview.appspot.com/8647044/diff/35001/input/regression/offsets.ly#newcode31 input/regression/offsets.ly:31: Spurious space in otherwise empty line. https://coderev

Re: Add the command \offset to LilyPond (issue 8647044)

2013-10-07 Thread dak
https://codereview.appspot.com/8647044/diff/35001/scm/c++.scm File scm/c++.scm (right): https://codereview.appspot.com/8647044/diff/35001/scm/c++.scm#newcode30 scm/c++.scm:30: (not (null? x)) Why not null? An empty list is also a list, so this makes the predicate a bit of a misnomer. https://c

Re: Add the command \offset to LilyPond (issue 8647044)

2013-10-07 Thread dak
https://codereview.appspot.com/8647044/diff/48001/ly/music-functions-init.ly File ly/music-functions-init.ly (right): https://codereview.appspot.com/8647044/diff/48001/ly/music-functions-init.ly#newcode705 ly/music-functions-init.ly:705: (if (ly:music? item) Ok, I am annoyed at the necessity for

Re: Add the command \offset to LilyPond (issue 8647044)

2013-10-15 Thread dak
On 2013/10/15 18:04:43, janek wrote: Hi, an afterthought. On 2013/10/06 01:15:12, david.nalesnik wrote: > The examples below represent my efforts to test the effects of multiple > applications of \offset. You can see that some accumulation is possible. > [...] > > \relative c' { >

Re: Make \tweak ... Grob equivalent to \override rather than \once \override (issue 14503043)

2013-10-15 Thread dak
On 2013/10/15 21:51:22, janek wrote: Do i understand correctly that with this patch one can use \tweak to achieve everything that \override does? That's not really the main point. It would have been simple previously to make \tweak be equivalent to \override rather than \once \override when

Make markup commands \super and \sub produce correct placement (issue 14438075)

2013-10-16 Thread dak
Well, one thing to note is that LilyPond should be more concerned about placing, say, flats and sharps in a reasonable relation to a chord symbol (usually uppercase) than to place mathematical super- and subscripts like expected. Of course we should go for both if feasible, but I don't see that a

Re: format-metronome-mark and metronome-markup don't support styles other than default (issue 14807043)

2013-10-17 Thread dak
The interface looks cumbersome to use. Why don't you just take the styles that are used in the Score? It might mean adding the respective properties to the font-interface (MetronomeMark has both font-interface and text-interface), but that seems reasonably straightforward. https://codereview.ap

Re: Add the command \offset to LilyPond (issue 8647044)

2013-10-20 Thread dak
On 2013/10/20 15:51:57, david.nalesnik wrote: https://codereview.appspot.com/8647044/diff/48001/scm/music-functions.scm File scm/music-functions.scm (right): https://codereview.appspot.com/8647044/diff/48001/scm/music-functions.scm#newcode2118 scm/music-functions.scm:2118: (let* ((immutable

Re: Add the command \offset to LilyPond (issue 8647044)

2013-10-20 Thread dak
On 2013/10/20 20:35:20, david.nalesnik wrote: OK, that makes sense. I've rewritten offset according to these guidelines. A side benefit is that it was no trouble to allow directed tweaks, so the following is possible: { 4 4 } Oh. Indeed. https://codereview.appspot.com/8647044

Re: Make sure slurs actually avoid stafflines. (issue 15400049)

2013-10-23 Thread dak
On 2013/10/23 07:16:36, Keith wrote: https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc File lily/slur-configuration.cc (right): https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#newcode121 lily/slur-configuration.cc:121: Real mid_pts_cor = 0.6

Re: Improvements to \offset (issue 15850044)

2013-10-23 Thread dak
On 2013/10/23 14:24:33, david.nalesnik wrote: https://codereview.appspot.com/15850044/diff/120001/ly/music-functions-init.ly File ly/music-functions-init.ly (right): https://codereview.appspot.com/15850044/diff/120001/ly/music-functions-init.ly#newcode722 ly/music-functions-init.ly:722: (mak

Re: Improvements to \offset (issue 15850044)

2013-10-23 Thread dak
On 2013/10/23 14:30:42, dak wrote: > The way the code is now, the snippet returns an error about bad grob property > path (`foo') and "post-event expected." No arpeggio appears. With a return of > item, we only get the warning about property path, and the arpeg

Re: there are actually four commits: (issue 14640043)

2013-10-25 Thread dak
On 2013/10/25 07:04:55, Keith wrote: None of the examples show \set completionFactorUnit = n/m actually helping. Is there an example where this property is more useful than a boolean switch? No, because completionFactorUnit is only used for determining the position of a boolean switch. You

Re: Document how completion engravers split scaled notes; issue 3560 (issue 14633043)

2013-10-25 Thread dak
https://codereview.appspot.com/14633043/diff/2/lily/completion-note-heads-engraver.cc File lily/completion-note-heads-engraver.cc (right): https://codereview.appspot.com/14633043/diff/2/lily/completion-note-heads-engraver.cc#newcode202 lily/completion-note-heads-engraver.cc:202: if (left_to_do_

Re: Issue 3560: Completion_heads_engraver with \scaleDurations (issue 14268043)

2013-10-25 Thread dak
https://codereview.appspot.com/14268043/diff/9001/scm/define-context-properties.scm File scm/define-context-properties.scm (right): https://codereview.appspot.com/14268043/diff/9001/scm/define-context-properties.scm#newcode218 scm/define-context-properties.scm:218: (completionFactor ,rational-or

Re: Issue 3560: Completion_heads_engraver with \scaleDurations (issue 14268043)

2013-10-25 Thread dak
https://codereview.appspot.com/14268043/diff/9001/lily/completion-note-heads-engraver.cc File lily/completion-note-heads-engraver.cc (right): https://codereview.appspot.com/14268043/diff/9001/lily/completion-note-heads-engraver.cc#newcode200 lily/completion-note-heads-engraver.cc:200: factor_ =

Define absolute-dynamics as a customizable event-function (issue 20660044)

2013-11-01 Thread dak
https://codereview.appspot.com/20660044/diff/1/ly/dynamic-scripts-init.ly File ly/dynamic-scripts-init.ly (right): https://codereview.appspot.com/20660044/diff/1/ly/dynamic-scripts-init.ly#newcode7 ly/dynamic-scripts-init.ly:7: #(define safe-dynamics ; see midi.scm What's "safe" about them? htt

Re: Build: Fix compilation with GNU make 4.0 (issue 18700043)

2013-11-01 Thread dak
LGTM https://codereview.appspot.com/18700043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Issue 3560: Completion_heads_engraver with \scaleDurations (issue 14268043)

2013-11-01 Thread dak
On 2013/10/26 06:37:13, Keith wrote: The simplicity is just lovely. Always hard to tell sarcasm from enthusiasm. https://codereview.appspot.com/14268043/diff/9001/lily/completion-note-heads-engraver.cc File lily/completion-note-heads-engraver.cc (right): https://codereview.appspot.com/142

Re: Issue 3639: tuplet of a single chord gives a syntax error (issue 20320044)

2013-11-03 Thread dak
Reviewers: Keith, Message: On 2013/11/02 22:34:51, Keith wrote: I have been reading, slowly absorbing this and the patch for issue 3618, and see no problems so far. I see now that the challenge is not so much determining that \repeat... is not a duration for \tuplet 6/4 \repeat tremolo

Re: A \score-lines markup list command for multi-lines embedded scores (issue 21280043)

2013-11-03 Thread dak
Reviewers: lemzwerg, https://codereview.appspot.com/21280043/diff/1/lily/lily-lexer.cc File lily/lily-lexer.cc (right): https://codereview.appspot.com/21280043/diff/1/lily/lily-lexer.cc#newcode79 lily/lily-lexer.cc:79: {"score-lines", SCORELINES}, On 2013/11/03 08:56:47, lemzwerg wrote: If the

<    8   9   10   11   12   13   14   15   16   17   >