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

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 } On 2011/11/

Re: Patch series (available at branch dev/syntax) for more argument list power. (issue 5333051)

Reviewers: J_lowe, Reinhold, Message: On 2011/11/04 23:43:04, Reinhold wrote: On 2011/11/03 22:59:52, J_lowe wrote: > Passes make and make check, No reg test diffs. Is Patch Set 3 the final version, or are the three patch sets independent patches that need to be applied one after the other?

Re: Patch series (available at branch dev/syntax) for more argument list power. (issue 5333051)

On 2011/11/05 06:15:49, dak wrote: On 2011/11/04 23:43:04, Reinhold wrote: > On 2011/11/03 22:59:52, J_lowe wrote: > > Passes make and make check, No reg test diffs. > > Is Patch Set 3 the final version, or are the three patch sets independent > patches that need to be app

Re: Patch series (available at branch dev/syntax) for more argument list power. (issue 5333051)

On 2011/11/05 06:32:26, dak wrote: On 2011/11/05 06:15:49, dak wrote: > On 2011/11/04 23:43:04, Reinhold wrote: > > On 2011/11/03 22:59:52, J_lowe wrote: > > > Passes make and make check, No reg test diffs. > > > > Is Patch Set 3 the final version, or are the

Re: parser.yy: allow postevents in function arguments in general (issue 5339043)

Reviewers: J_lowe, Message: On 2011/11/05 11:54:16, J_lowe wrote: Patch fails to apply to current master Hardly surprising. This is why it says right at the top "This change is on top of the dev/syntax branch still in review." and why its related issue 2018 is blocked on issue 2001. I'd appr

Re: Patch series (available at branch dev/syntax) for more argument list power. (issue 5333051)

On 2011/11/05 11:56:13, J_lowe wrote: Patch fails to apply against current master Probably Rietveld did not like me deleting a patch in the middle of the series. It is really a noisome tool. I'll delete and reupload the last version. That will hopefully help. http://codereview.appspot.com/5

Re: Patch series (available at branch dev/syntax) for more argument list power. (issue 5333051)

On 2011/11/06 04:31:23, Carl wrote: One question -- as you move from parser tokens to music functions, is the capability of displayLilyMusic being kept functional for all of these functions? Since \displayLilyMusic does not see whether the music it looks at is generated by music functions or

Add \hideNote which is like \hideNotes but just \once. (issue 5342047)

In general, you should not touch the translations. We have translators for that purpose, and they keep track of the original commits corresponding to some change. The only exception is when there is a non-compatible syntax change. In that case, appropriate conversion rules should be added to py

Re: Improve HTML output of regression tests (issue 5342042)

http://codereview.appspot.com/5342042/diff/1/scripts/build/output-distance.py File scripts/build/output-distance.py (right): http://codereview.appspot.com/5342042/diff/1/scripts/build/output-distance.py#newcode1304 scripts/build/output-distance.py:1304: if len (args) % 2 == 1: On 2011/11/07 08:5

Let #{ ... #} pass its $ handling to environment cloning (issue 5340053)

Reviewers: , Message: I am replacing this review by a new one. Description: Let #{ ... #} pass its $ handling to environment cloning Permit ly:parser-clone to receive an environment lexer.ll: add $ for immediate export. Please review this at http://codereview.appspot.com/5340053/ Affected

Re: Issue 2024: Let #{ ... #} pass its $ handling to environment cloning (issue 5341049)

Reviewers: Trevor Daniels, Message: On 2011/11/08 16:37:25, Trevor Daniels wrote: I don't claim to understand much of this, but the resulting simplification seems worth the pain to me. Obviously to me as well. I want to stress that I did _no_ manual changes to Documentation and regtests, an

Re: Fixes footnote automatic numbering. (issue 4877041)

http://codereview.appspot.com/4877041/diff/25001/lily/grob.cc File lily/grob.cc (right): http://codereview.appspot.com/4877041/diff/25001/lily/grob.cc#newcode639 lily/grob.cc:639: if (!vag) How is vag supposed to have become false again after being true two lines before? http://codereview.appsp

Re: grob.cc: rewrite O(n^2) algorithm in Grob::common_refpoint algorithm to O(n) (issue 5371050)

Reviewers: joeneeman, md5i, http://codereview.appspot.com/5371050/diff/1/lily/grob.cc File lily/grob.cc (right): http://codereview.appspot.com/5371050/diff/1/lily/grob.cc#newcode536 lily/grob.cc:536: c = c->dim_cache_[a].parent_; On 2011/11/10 16:41:54, joeneeman wrote: After this loop, balanc

Re: Keep yaffut from attempting to demangle. (issue 5375051)

Reviewers: J_lowe, jan.nieuwenhuizen, Message: On 2011/11/11 06:48:28, jan.nieuwenhuizen wrote: On 2011/11/10 20:07:34, J_lowe wrote: > Passes make and no reg test diffs +// Modified to omit demangling, filter through c++filt -t instead Please, do not add changelogs to source files; we hav

Re: This removes ly:export for real and separates read/eval for Scheme expressions. (issue 5370048)

Reviewers: J_lowe, Message: On 2011/11/11 18:24:38, J_lowe wrote: This fails make (petty instantly actually) Since it removes source files, you need to remake the dependencies. Description: This removes ly:export for real and separates read/eval for Scheme expressions. Patches are: Separat

Re: grob.cc: rewrite O(n^2) algorithm in Grob::common_refpoint algorithm to O(n) (issue 5371050)

On 2011/11/12 17:06:15, Carl wrote: LGTM. Thanks, Carl http://codereview.appspot.com/5371050/diff/1/lily/grob.cc File lily/grob.cc (right): http://codereview.appspot.com/5371050/diff/1/lily/grob.cc#newcode542 lily/grob.cc:542: while (c != d) { On 2011/11/10 16:41:54, joeneeman wrote:

Re: Fixes footnote automatic numbering. (issue 4877041)

http://codereview.appspot.com/4877041/diff/26012/lily/system.cc File lily/system.cc (right): http://codereview.appspot.com/4877041/diff/26012/lily/system.cc#newcode248 lily/system.cc:248: if (s->original ()) You don't check for success of the dynamic_cast before using it. Is that a problem? ht

Re: Fixes footnote automatic numbering. (issue 4877041)

lily/grobarray.cc contains the following: SCM Grob_array::mark_smob (SCM s) { (void) s; #if 0 /* see System::derived_mark () const */ Grob_array *ga = unsmob_grob_array (s); for (vsize i = 0; i < ga->grobs_.size (); i++) scm_gc_mark (ga->grobs_[i]->self_scm ()); #endif return SCM_UN

Re: lexer.ll: Allow include file names to be specified as Scheme expression (issue 5369104)

Reviewers: J_lowe, Message: Previous patch set was done with a different base, so the diffs stopped making sense. Removed it. Just the current patch set against staging (hopefully soon master) remains. Description: lexer.ll: Allow include file names to be specified as Scheme expression Fixes

Re: Adds some info about pure properties to the CG. (issue 5364048)

http://codereview.appspot.com/5364048/diff/4001/Documentation/contributor/programming-work.itexi File Documentation/contributor/programming-work.itexi (right): http://codereview.appspot.com/5364048/diff/4001/Documentation/contributor/programming-work.itexi#newcode1832 Documentation/contributor/p

Re: Fix issue 1900 -- multiple fret-diagram-terse markups commands crash (issue 5432081)

On 2011/11/26 06:39:45, dak wrote: Does not look removed. I might have just discussed this step in the mailing list. Reviewing it right now. The functionality seems to be there just fine. What makes you suspect that this copying-of-properties-at-definition-and-documentation-time is

Re: Fix issue 1900 -- multiple fret-diagram-terse markups commands crash (issue 5432081)

I know you've pushed a different patch, which fixes Issue 1900, but doesn't fix 1997. Have you tried this patch to see if it fixes 1997? I'll do so, but frankly, if it _does_ fix 1997, I don't want it pushed. Do you have a reference for Mike's analysis? It seems we should rather fix the segf

Re: Fix issue 1900 -- multiple fret-diagram-terse markups commands crash (issue 5432081)

On 2011/11/26 15:15:25, Carl wrote: On 2011/11/26 15:07:17, dak wrote: > > I'll do so, but frankly, if it _does_ fix 1997, I don't want it pushed. Do you > have a reference for Mike's analysis? http://lists.gnu.org/archive/html/lilypond-devel/2011-11/msg00556.html

Work around compiler bug, Issue 1997: segfault in tablature-negative-fret.ly (issue 5431088)

Reviewers: , Message: This makes the regtest on my 32bit x86 Ubuntu 11.10 work with optionless autogen.sh for the first time. Description: Work around compiler bug, Issue 1997: segfault in tablature-negative-fret.ly Please review this at http://codereview.appspot.com/5431088/ Affected files:

Adds make-connected-path-stencil-with-initial-offset (issue 5434080)

I have my problems imagining why you don't just use ly:stencil-translate on the result. If you really want to have a separately specifiable origin, I would not make a separate function but rather add it as an optional argument at the end of the argument list. http://codereview.appspot.com/543408

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

Reviewers: MikeSol, Message: On 2011/12/02 19:50:03, MikeSol wrote: Hey David, This patch is way over my head, so I can't really comment on the details of the implementation, but I just wanted to voice one concern for me (and possibly other) algorithmic composers. I have a lot of algor

Re: More nuanced BarLine extra-spacing-height to allow tighter horizontal spacing (issue 5434104)

http://codereview.appspot.com/5434104/diff/1/scm/output-lib.scm File scm/output-lib.scm (right): http://codereview.appspot.com/5434104/diff/1/scm/output-lib.scm#newcode408 scm/output-lib.scm:408: (define-public (pure-from-neighbor-interface::account-for-span-bar grob) On 2011/12/05 06:00:05, Kei

Re: Remove spurious spaces from music expression display, adapt tests. (issue 5437140)

Reviewers: carl.d.sorensen_gmail.com, lemzwerg, Message: On 2011/12/06 05:39:42, lemzwerg wrote: David, I suggest to apply patches of that kind directly to the repository without setting up a Rietveld issue. Done. It is actually part of my EventChord work. I got oodles of display differenc

Re: Allows for framing comments in LilyPond backends. (issue 5450086)

On 2011/12/07 04:58:58, Colin Campbell wrote: somewhat less painful alternative might be "stencilize" in its many permutations: viz. http://dictionary.reference.com/browse/stencilize althopugh to be quite frank, one hopes a more suitable word will come to light. Nevertheless, it should go t

Doc: NR improve 1.6.3 examples (issue 5448129)

http://codereview.appspot.com/5448129/diff/1/Documentation/notation/staff.itely File Documentation/notation/staff.itely (right): http://codereview.appspot.com/5448129/diff/1/Documentation/notation/staff.itely#newcode869 Documentation/notation/staff.itely:869: } As explained previously, _this_ _w

Re: Provide Scheme sandbox. (issue 5453045)

Reviewers: Keith, Message: On 2011/12/08 00:38:07, Keith wrote: Very nice. If you make it this easy, I might learn Scheme... nah. Well, it is like converting people to Emacs. Those who have gotten burnt 20 years ago are a lost cause. You can't erase their childhood memories. And they look w

Re: Provide Scheme sandbox. (issue 5453045)

On 2011/12/08 11:32:28, Ian Hulin (gmail) wrote: Hi David, A Scheme sandbox is a good idea, in fact it is *A Very Good Idea*. However, before we focus down on this solution, might it not be a good thing to consider this: get LilyPond in its build to provide a library of all its scheme stuff

Re: 13:35:08 up 20:27, 2 users, load average: 0.07, 0.07, 0.07 (issue 5464045)

On 2011/12/19 19:38:04, Carl wrote: 1) How do we decide when we need to include (use-modules (scm markup-facility-defs)) in our input files? Is it to be included any time we define our own markup commands? I feel rather strongly that this should not be required in user LilyPond files but ra

Re: 13:35:08 up 20:27, 2 users, load average: 0.07, 0.07, 0.07 (issue 5464045)

http://codereview.appspot.com/5464045/diff/2001/Documentation/snippets/three-sided-box.ly File Documentation/snippets/three-sided-box.ly (right): http://codereview.appspot.com/5464045/diff/2001/Documentation/snippets/three-sided-box.ly#newcode20 Documentation/snippets/three-sided-box.ly:20: #(us

Re: Changes to allow compiling with -Werror using gcc 4.6.2 on i386. (issue 5489092)

On 2011/12/20 03:55:59, Carl wrote: Personally, I see no reason to use I64 instead of int. Neil Puttock changed the numerator and denominators to be I64 in 2008, with commit be65b81068e99ed855334f332c3176d8b4942a11 I don't think that research on the history of such changes should be holding

Re: Changes to allow compiling with -Werror using gcc 4.6.2 on i386. (issue 5489092)

http://codereview.appspot.com/5489092/diff/1/lily/parser.yy File lily/parser.yy (right): http://codereview.appspot.com/5489092/diff/1/lily/parser.yy#newcode36 lily/parser.yy:36: /* Define to get rid of conversion warning, int -> int16_t. This has I think the side effect of significantly increas

Doc: Usage added @knownissue for LaTeX (issue 5492075)

http://codereview.appspot.com/5492075/diff/1/Documentation/usage/lilypond-book.itely File Documentation/usage/lilypond-book.itely (right): http://codereview.appspot.com/5492075/diff/1/Documentation/usage/lilypond-book.itely#newcode915 Documentation/usage/lilypond-book.itely:915: @dots{} @code{\e

Re: T2026: Use new (scm markup-facility-defs) scheme module for markup commands. (issue 5464045)

In general, people will view a patch like this as reference for what changes are required in order to keep things working in future. So you should keep actually independent cleanups and issues and fixes separate from markup in separate patches. Personally, I tend to push that kind of small-fry c

Re: Changes to allow compiling with -Werror using gcc 4.6.2 on i386. (issue 5489092)

On 2011/12/21 02:56:37, md5i wrote: http://codereview.appspot.com/5489092/diff/1/lily/parser.yy File lily/parser.yy (right): http://codereview.appspot.com/5489092/diff/1/lily/parser.yy#newcode36 lily/parser.yy:36: /* Define to get rid of conversion warning, int -> int16_t. I'd love to see a

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

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; On 2011/12/21 15:59:52, Neil Puttock wrote: Don't you think this is a bit extravagant? At least, this

Re: T2026: Use new (scm markup-facility-defs) scheme module for markup commands. (issue 5464045)

http://codereview.appspot.com/5464045/diff/8002/input/regression/markup-cyclic-reference.ly File input/regression/markup-cyclic-reference.ly (right): http://codereview.appspot.com/5464045/diff/8002/input/regression/markup-cyclic-reference.ly#newcode2 input/regression/markup-cyclic-reference.ly:2

Re: Sends woodwind regtest op to stderr (issue 5502058)

http://codereview.appspot.com/5502058/diff/1/input/regression/woodwind-diagrams-key-lists.ly File input/regression/woodwind-diagrams-key-lists.ly (right): http://codereview.appspot.com/5502058/diff/1/input/regression/woodwind-diagrams-key-lists.ly#newcode1 input/regression/woodwind-diagrams-key-

Re: Tie LilyPond, lexer and parser together more type-safely. (issue 5501056)

On 2011/12/21 22:29:20, dak wrote: Han-Wen Nienhuys <mailto:hanw...@gmail.com> writes: > Does this require a required version bump in configure for flex/bison? Good question. Looking at /usr/share/doc/bison/changelog.gz, I see for example 2003-04-28 Tim V

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

Ok, I need some pointers here. In order to make this work compatibly at the lowest level, articulations need to behave differently depending on whether they are on note events that are children of an EventChord (which all of them are currently) or not. Ok, so now since I don't actually have a cl

Re: T2026: Use new (scm markup-facility-defs) scheme module for markup commands. (issue 5464045)

One problem that I keep having is that I fail to see any documentation of how this is supposedly interacting with the module system. What module are user-defined markup functions placed in? Previously, this would have been a module local to the compilation unit. It looks like there are multiple

Re: Update lilygit.tcl (Issue 2092) (issue 5504092)

On 2011/12/29 06:45:53, Graham Percival wrote: First thought: I'm a bit leery of adding a "push to staging", since: 1. that clutters up the interface. Sure, it's just one more button, but OTOH that's 25% more buttons. :) It would seem to me that this button need only appear when you have t

Re: Creates non-negative-integer? predicate. (issue 5501081)

On 2011/12/30 19:15:57, Graham Percival wrote: I'm sorry to throw my hat in the ring so late, but I prefer something explicit like non-negative-integer? I mean, the name tells it all. What is this function doing? It's checking whether something is a non-negative integer. If it's called

Re: Update lilygit.tcl (Issue 2092) (issue 5504092)

On 2011/12/30 20:57:02, Graham Percival wrote: Patchy will not question any ridiculous git history that arises due to any kind of weird series of commands in git. Maybe it would make sense if Patchy refused fast forwarding over a history involving a merge _from_ staging. I think that merges

Re: Update lilygit.tcl (Issue 2092) (issue 5504092)

The problem is not merges. The problem is unintentional merges. merges _from_ staging (or origin/staging) are usually a mistake. http://codereview.appspot.com/5504092/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailm

Re: lexer.ll: Warn about non-UTF-8 characters (issue 5505090)

Reviewers: lemzwerg, Keith, carl.d.sorensen_gmail.com, Message: On 2012/01/01 02:01:11, Keith wrote: Works nicely. Showing the input location will probably be very helpful. We probably want to remove the similar message from lily/misc.cc, because both message together are very noisy. D

Re: lexer.ll: Warn about non-UTF-8 characters (issue 5505090)

http://codereview.appspot.com/5505090/diff/1/lily/lexer.ll File lily/lexer.ll (right): http://codereview.appspot.com/5505090/diff/1/lily/lexer.ll#newcode134 lily/lexer.ll:134: A[a-zA-Z\200-\377] On 2012/01/01 02:01:11, Keith wrote: non-ASCII characters are used internally as-read, t

Re: lexer.ll: Warn about non-UTF-8 characters (issue 5505090)

On 2012/01/01 10:05:42, Keith wrote: On Sun, 01 Jan 2012 01:40:36 -0800, wrote: > Our lexer has been written with the decision of using non-compressed > tables and without backing up. Off topic, but maybe interesting to you: I don't think that decision was ever imple

Re: lexer.ll: Warn about non-UTF-8 characters (issue 5505090)

On 2012/01/01 10:16:47, Keith wrote: On Sun, 01 Jan 2012 01:56:04 -0800, wrote: > http://codereview.appspot.com/5505090/diff/1/lily/lexer.ll#newcode1083 > lily/lexer.ll:1083: LexerWarning (_ ("non-UTF-8 characters").c_str ()); > On 2012/01/01 02:01:11, Keith wrote: >

Re: Doc: NR added @knownissue for beam properties (issue 5504100)

On 2012/01/01 14:56:25, Carl wrote: > Can we show this using a single voice with non-chorded notes and by using > something more 'obvious' than \hideNotes? Override the beam color to red? Also \once\hideNotes might be a bit more snappy. http://codereview.appspot.com/5504100/

Re: lexer.ll: Warn about non-UTF-8 characters (issue 5505090)

On 2012/01/01 22:06:52, Keith wrote: On 2012/01/01 10:12:27, dak wrote: Sorry, I wasn't making much sense. As a reader I want to *recognize* what the but switch/case is doing rather than trying to figure it out. Maybe : // Test if these bytes are a UTF-8 encoding of a Unicode char

Re: T2026: Use new (scm markup-facility-defs) scheme module for markup commands. (issue 5464045)

http://codereview.appspot.com/5464045/diff/10002/ly/music-functions-init.ly File ly/music-functions-init.ly (right): http://codereview.appspot.com/5464045/diff/10002/ly/music-functions-init.ly#newcode31 ly/music-functions-init.ly:31: %% need (scm markup-facility-defs)for markup? Wasn't this supp

Re: T2026: Use new (scm markup-facility-defs) scheme module for markup commands. (issue 5464045)

21:19:24, Ian Hulin (gmail) wrote: On 2012/01/02 14:04:39, dak wrote: > Wasn't this supposed to get removed? This was not supposed to be needed in > user-level docs, right? No. This isn't a user-level document, it's part of the run-time initialization we do for each user

Re: Implements DOM-id property for grobs. (issue 5504106)

Is there a reason you have ignored Patrick's comments? The issue is missing a description, so is any part of the code. It needs at least a regtest demonstrating the use of this feature, otherwise nobody will be able to ever put this code to actual use (or write user documentation for it) if you

Re: Let \footnote do the job of \footnote, \footnoteGrob, \autoFootnote and \autoFootnoteGrob (issue 5527058)

Reviewers: J_lowe, carl.d.sorensen_gmail.com, lemzwerg, MikeSol, Message: On 2012/01/10 06:29:57, lemzwerg wrote: http://codereview.appspot.com/5527058/diff/1/python/convertrules.py File python/convertrules.py (right): http://codereview.appspot.com/5527058/diff/1/python/convertrules.py#newcod

Re: Let \footnote do the job of \footnote, \footnoteGrob, \autoFootnote and \autoFootnoteGrob (issue 5527058)

On 2012/01/09 20:42:30, J_lowe wrote: Does this do anything to the \auto-footnote command as well? No. http://codereview.appspot.com/5527058/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-d

Re: Let \footnote do the job of \footnote, \footnoteGrob, \autoFootnote and \autoFootnoteGrob (issue 5527058)

On 2012/01/10 07:08:29, MikeSol wrote: LGTM. Good work! The only think I'd ask is that you change the markup syntax before pushing the patch. I think that, if the distinction between footnote and auto-footnote is going to be eliminated, it needs to be categorical. Optional arguments ar

Re: Let \footnote do the job of \footnote, \footnoteGrob, \autoFootnote and \autoFootnoteGrob (issue 5527058)

On 2012/01/10 12:59:21, Reinhold wrote: LGTM. From a lazy user's POV, I don't like that I now have to use \default for auto-numbering (which is th typical case)... It is the same as with \mark (we don't have \autoMark either). One might consider moving the footnote mark argument to last

Re: Let \footnote do the job of \footnote, \footnoteGrob, \autoFootnote and \autoFootnoteGrob (issue 5527058)

On 2012/01/10 13:18:03, dak wrote: On 2012/01/10 12:59:21, Reinhold wrote: > LGTM. > > From a lazy user's POV, I don't like that I now have to use \default for > auto-numbering (which is th typical case)... It is the same as with \mark (we don't have \autoMark eit

Re: Let \footnote do the job of \footnote, \footnoteGrob, \autoFootnote and \autoFootnoteGrob (issue 5527058)

On 2012/01/11 05:11:39, janek wrote: There are some duplications in the docs now. (LBTM?) "The notation manual has not been revised yet." Since I am currently doing the convert-ly rules for juggling the argument order and this will, obviously, also affect the manual both with respect to autoco

Re: explain how to add git-cl to PATH (issue 5503093)

http://codereview.appspot.com/5503093/diff/4003/Documentation/contributor/source-code.itexi File Documentation/contributor/source-code.itexi (right): http://codereview.appspot.com/5503093/diff/4003/Documentation/contributor/source-code.itexi#newcode926 Documentation/contributor/source-code.itexi

Re: Let \footnote do the job of \footnote, \footnoteGrob, \autoFootnote and \autoFootnoteGrob (issue 5527058)

On 2012/01/11 11:45:19, J_lowe wrote: I've created http://code.google.com/p/lilypond/issues/detail?id=2213 so I'll work on the NR as I did all the \footnote Doc in the first place. I am assuming you still have to include your documentation edits in the patch so that the docs compile? Nope

Re: Issue 2100: Explanation of branches for CG (issue 5539062)

On 2012/01/15 12:18:57, Carl wrote: On 2012/01/15 08:05:35, Graham Percival wrote: > http://codereview.appspot.com/5539062/diff/1/Documentation/contributor/source-code.itexi > File Documentation/contributor/source-code.itexi (right): > > http://codereview.appspot.com/5539062/diff/1/Documenta

Re: Issue 2100: Explanation of branches for CG (issue 5539062)

On 2012/01/15 12:43:03, Carl wrote: On 2012/01/15 12:32:21, Graham Percival wrote: > On Sun, Jan 15, 2012 at 12:18:57PM +, mailto:carl.d.soren...@gmail.com wrote: > > > > I think you misunderstand what git rebase does. git rebase > > origin/staging does *not* bring origin/staging into dev/

Re: Issue 2100: Explanation of branches for CG (issue 5539062)

On 2012/01/15 13:37:13, Carl wrote: After doing some testing, it appears that the following should be able to get my dev/cg applied to origin/staging, while preserving my dev/cg: git rebase origin/staging dev/cg~0 git push origin HEAD:staging David, is this correct? As far as my painfu

Re: Issue 2100: Explanation of branches for CG (issue 5539062)

On 2012/01/15 23:15:01, Carl wrote: On 2012/01/15 14:54:22, dak wrote: > git rebase origin dev/cg My thoughts (almost) exactly. My set of commands would be git checkout dev/cg git rebase origin/master which does exactly the same thing, only with different words. I think it reac

Re: Issue 2100: Explanation of branches for CG (issue 5539062)

On 2012/01/16 16:16:40, Carl wrote: Having just spent half an hour fixing up my lilygit.tcl branch, I have personally validated the benefit of the approach now reflected in this patch. I will *never* again rebase a working branch to origin/staging. In fact, I will never again have a local b

Re: Issue 2100: Explanation of branches for CG (issue 5539062)

On 2012/01/17 21:31:01, janek wrote: http://codereview.appspot.com/5539062/diff/3004/Documentation/contributor/source-code.itexi File Documentation/contributor/source-code.itexi (right): http://codereview.appspot.com/5539062/diff/3004/Documentation/contributor/source-code.itexi#newcode297 Do

Re: Broadcast articulations not in EventChord (issue 5528111)

Reviewers: MikeSol, Message: On 2012/01/17 17:42:38, MikeSol wrote: I would advise not handling it this way - the function to_event is supposed to take music and return an event. In this patch, it is taking music, returning an event, and maybe broadcasting music and/or sending it to a conte

Implements rhythmic-music-iterator (issue 5554048)

http://codereview.appspot.com/5554048/diff/2001/lily/rhythmic-music-iterator.cc File lily/rhythmic-music-iterator.cc (right): http://codereview.appspot.com/5554048/diff/2001/lily/rhythmic-music-iterator.cc#newcode41 lily/rhythmic-music-iterator.cc:41: report_event (get_music ()); I suppose that

Re: Implements rhythmic-music-iterator (issue 5554048)

http://codereview.appspot.com/5554048/diff/5001/lily/rhythmic-music-iterator.cc File lily/rhythmic-music-iterator.cc (right): http://codereview.appspot.com/5554048/diff/5001/lily/rhythmic-music-iterator.cc#newcode42 lily/rhythmic-music-iterator.cc:42: get_music ()->set_property ("articulations",

Re: Implements rhythmic-music-iterator (issue 5554048)

On 2012/01/18 14:36:57, MikeSol wrote: On 2012/01/18 14:27:53, dak wrote: > Are iterators allowed to change their input? I'm not sure if they're allowed by design, but I know they're allowed to proliferate input that wasn't there before. I don't know what yo

Re: Implements rhythmic-music-iterator (issue 5554048)

On 2012/01/18 14:53:28, dak wrote: Then rhythmic_iterator could just override it. Better yet, EventChord could just override it, and we would not need the rhythmic iterator at all. Well, it is not really "better yet" since the current case is the simple and straightforward one, so

Re: Implements rhythmic-music-iterator (issue 5554048)

On 2012/01/18 15:28:42, mike_apollinemike.com wrote: I don't think there's a risk that one engraver in a context will get a different version of an event than another engraver in a context. This would require the NoteEvent to be reported more than once. It would merely require using the s

Re: Implements rhythmic-music-iterator (issue 5554048)

On 2012/01/18 15:55:35, dak wrote: On 2012/01/18 15:28:42, http://mike_apollinemike.com wrote: > I don't think there's a risk that one engraver in a context will get a different > version of an event than another engraver in a context. This would require the > NoteE

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

Ok, this latest iteration is quite close to what I want to end up committing. All the work of picking apart articulations and events have gone into the rhythmic event iterator. What it does is to broadcast all articulations that have a listener, and keep the rest as articulations. One side effe

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

http://codereview.appspot.com/5440084/diff/7001/lily/parser.yy File lily/parser.yy (right): http://codereview.appspot.com/5440084/diff/7001/lily/parser.yy#newcode432 lily/parser.yy:432: %type list_music On 2012/01/20 13:55:56, md5i wrote: I must *strongly* recommend that the name of either mus

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

On 2012/01/20 14:08:18, MikeSol wrote: http://codereview.appspot.com/5440084/diff/11001/lily/rhythmic-music-iterator.cc File lily/rhythmic-music-iterator.cc (right): http://codereview.appspot.com/5440084/diff/11001/lily/rhythmic-music-iterator.cc#newcode62 lily/rhythmic-music-iterator.cc:62:

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

On 2012/01/20 14:55:38, dak wrote: On 2012/01/20 14:08:18, MikeSol wrote: > Otherwise, it's better to use the C++ function. In this case: > ly_is_listened_event_class > (in translator.cc) Will do. Very funny. After putting the (required) prototype into translator.hh, it

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

On 2012/01/20 17:33:44, dak wrote: mailto:n.putt...@gmail.com writes: > 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 =

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

On 2012/01/20 17:59:40, Neil Puttock wrote: I can't think of anything better than adding another type such as `command-event' to things like TempoChangeEvent and MarkEvent and filtering that out too. Emacs stands for "Editor Macros". ;; Keyboard Macro Editor. Press C-c C-c to finish; press C

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

http://codereview.appspot.com/5440084/diff/11001/scm/modal-transforms.scm File scm/modal-transforms.scm (right): http://codereview.appspot.com/5440084/diff/11001/scm/modal-transforms.scm#newcode129 scm/modal-transforms.scm:129: (define (make-scale music) On 2012/01/20 23:02:21, Carl wrote: I'm

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

On 2012/01/24 22:47:31, Neil Puttock wrote: Great work David. I like how \harmonic works properly for single notes now. :) As opposed to an earlier version of the patch or to LilyPond's previous behavior? All of your code comments are quite spot on and touch some sore spots that I decided wou

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

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 @var{obj} a proper (non-rhythmic) event object?") On 2012/01/24 22:47:32, Neil Put

Reimplement ChordRepetition facility. (issue 5595043)

Reviewers: , http://codereview.appspot.com/5595043/diff/4020/input/regression/tablature-chord-repetition.ly File input/regression/tablature-chord-repetition.ly (right): http://codereview.appspot.com/5595043/diff/4020/input/regression/tablature-chord-repetition.ly#newcode5 input/regression/tabla

Re: Directs lilypond option help to stderr (issue 5625052)

On 2012/02/04 12:45:23, PhilEHolmes wrote: Please review minor change. If we have a Scheme function for printing, the output should not be hardwired to a non-Scheme port, whether that be stderr or stdout. I think I mentioned as much. I'll push a reimplementation to staging in the next half ho

Re: Directs output from identifier-foll*.ly to stderr (issue 5624050)

tifier-foll*.ly to stderr (issue 5624050) > Reviewers: dak, Graham Percival, > > Message: > Please review. > > Description: > identifier-following-chordmode.ly sends a load of output to stdout - > i.e. the terminal. This sends it to stderr - i.e. th

Re: Directs output from identifier-foll*.ly to stderr (issue 5624050)

ond-devel@gnu.org> >> Sent: Saturday, February 04, 2012 2:40 PM >> Subject: Directs output from identifier-foll*.ly to stderr (issue > 5624050) > > >> > Reviewers: dak, Graham Percival, >> > >> > Message: >> > Please review. >> > >

Re: Directs lilypond option help to stderr (issue 5625052)

On 2012/02/04 15:08:33, dak wrote: On 2012/02/04 12:45:23, PhilEHolmes wrote: > Please review minor change. If we have a Scheme function for printing, the output should not be hardwired to a non-Scheme port, whether that be stderr or stdout. I think I mentioned as much. I'l

Re: Directs regtest output for last 2 files to stderr (issue 5625053)

On 2012/02/04 16:39:00, PhilEHolmes wrote: Please review. I am not actually sure that we should not for the regtests divert current-output-port generally to current-error-port and be done without all those fixups. Of course, this will still need to cater for those parts of the code that choose

Re: Directs lilypond option help to stderr (issue 5625052)

On 2012/02/04 17:34:09, PhilEHolmes wrote: See new description. Please review. Well, "obviously" correct, but it will still require one pass of the staging patchy before the testing patchy will be happy with it. So depending on who runs which Patchy when, this might take some time to get the

Re: Stops woodwind diagram info appearing on terminal (issue 5636048)

http://codereview.appspot.com/5636048/diff/1/Documentation/snippets/new/woodwind-diagrams-key-lists.ly File Documentation/snippets/new/woodwind-diagrams-key-lists.ly (right): http://codereview.appspot.com/5636048/diff/1/Documentation/snippets/new/woodwind-diagrams-key-lists.ly#newcode16 Document

Re: Stops woodwind diagram info appearing on terminal (issue 5636048)

On 2012/02/06 14:07:54, mail_philholmes.net wrote: I didn't do that because I had no idea it would work. As a snippet, I think it's easier to leave as is because the average user (e.g. me) would not understand the lambda stuff. Well, the thing is that you could also write (with-output-to-

Adds stencil command suppressions to changes (issue 5633048)

http://codereview.appspot.com/5633048/diff/1/Documentation/changes.tely File Documentation/changes.tely (right): http://codereview.appspot.com/5633048/diff/1/Documentation/changes.tely#newcode67 Documentation/changes.tely:67: following stencil commands have been suppressed: "removed" or "elimina

Checks to see that stencil commands have the correct number of arguments. (issue 5649054)

http://codereview.appspot.com/5649054/diff/1/lily/stencil-expression.cc File lily/stencil-expression.cc (right): http://codereview.appspot.com/5649054/diff/1/lily/stencil-expression.cc#newcode31 lily/stencil-expression.cc:31: nargs = scm_permanent_object (scm_cons (scm_cons (ly_symbol2scm ("dumm

Re: Final redirection of texi output (issue 5650064)

http://codereview.appspot.com/5650064/diff/4001/scripts/build/run-and-check.sh File scripts/build/run-and-check.sh (right): http://codereview.appspot.com/5650064/diff/4001/scripts/build/run-and-check.sh#newcode2 scripts/build/run-and-check.sh:2: eval $1 > $2 2>&1 On 2012/02/13 10:53:44, Graham P

<    5   6   7   8   9   10   11   12   13   14   >