Re: Change stringTunings entries from semitones to pitches (issue3842041)

2010-12-28 Thread Carl . D . Sorensen
Thanks again for the review. Here are my responses to your comments. Thanks, Carl http://codereview.appspot.com/3842041/diff/6001/Documentation/notation/fretted-strings.itely File Documentation/notation/fretted-strings.itely (right):

Change stringTunings entries from semitones to pitches (issue3842041)

2010-12-24 Thread Carl . D . Sorensen
Reviewers: , Message: Here's a patch that changes stringTunings entries from semitones to pitches. This is needed to allow a TabKey grob to be created. Also defines stringTunings for orchestral strings, and includes a function to allow the definition of stringTunings using absolute mode

Re: Doc: NR cross-staff (issue3782042)

2010-12-24 Thread Carl . D . Sorensen
OK -- changes posted. Keith, The patch wouldn't apply to the previous commit (I think it included your previous patch) so I applied it to the parent of your previous commit. Then it applied. Please check to make sure it's what you wanted. Thanks, Carl

Re: DOC: NR Dynamics context and postfix dynamics (issue3743045)

2010-12-22 Thread Carl . D . Sorensen
On 2010/12/22 21:43:37, Keith wrote: The snippets in input/regression/ are *not* part of the documentation Now I see. I updated the file in /snippets/new so it uses the predefined \cresc (just like updated the reg-test does). Perfect! I have this and the tweaks noted below ready for a

Re: DOC: NR Dynamics context and postfix dynamics (issue3743045)

2010-12-22 Thread Carl . D . Sorensen
Your thoughts generally look good to me, but I'll need to see a patch before I can approve. Thanks, Carl http://codereview.appspot.com/3743045/diff/1/Documentation/notation/expressive.itely File Documentation/notation/expressive.itely (right):

Re: DOC: NR Dynamics context and postfix dynamics (issue3743045)

2010-12-22 Thread Carl . D . Sorensen
http://codereview.appspot.com/3743045/diff/1/Documentation/notation/expressive.itely File Documentation/notation/expressive.itely (right): http://codereview.appspot.com/3743045/diff/1/Documentation/notation/expressive.itely#newcode366 Documentation/notation/expressive.itely:366: Textual

Re: DOC: NR Dynamics context and postfix dynamics (issue3743045)

2010-12-22 Thread Carl . D . Sorensen
http://codereview.appspot.com/3743045/diff/1/Documentation/notation/expressive.itely File Documentation/notation/expressive.itely (right): http://codereview.appspot.com/3743045/diff/1/Documentation/notation/expressive.itely#newcode366 Documentation/notation/expressive.itely:366: Textual

Re: Doc: NR cross-staff (issue3782042)

2010-12-22 Thread Carl . D . Sorensen
This has been up for nearly three days now. Any comments? http://codereview.appspot.com/3782042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: DOC: NR Dynamics context and postfix dynamics (issue3743045)

2010-12-22 Thread Carl . D . Sorensen
On 2010/12/23 04:11:34, Keith wrote: Thanks for the comments. Second patch set is up. The new patch corresponds to the state *before* makelsr.py, so that it is not cluttered with version bumps to all the snippets. Regarding deprecation, the old *prefix* implementation of \cresc, never

Re: DOC: NR Dynamics context and postfix dynamics (issue3743045)

2010-12-22 Thread Carl . D . Sorensen
On 2010/12/23 04:41:43, Keith wrote: I don't see a patch to convertrules.py. [...] Am I missing something? Reinhold created the rule months ago, along with the implementation of the new \cresc. (I should have said is *already* convert-ly-ed.) Deprecated doesn't mean removed, it means

Re: Doc: More NR and LM additions based on user emails (issue3705042)

2010-12-21 Thread Carl . D . Sorensen
On 2010/12/21 15:21:29, Trevor Daniels wrote: Looks fine to me, but then you've used the wording I suggested, so it would, wouldn't it :) Best wait for someone else to approve. I believe that Graham and I have both approved it. Carl http://codereview.appspot.com/3705042/

Re: [Patch] make post-event music functions direction-aware (issue3743043)

2010-12-21 Thread Carl . D . Sorensen
On 2010/12/21 20:45:42, Valentin Villenave wrote: On 2010/12/20 16:22:30, Carl wrote: actually, whilst it does pass make check, it breaks... all of my own scores! I can't understand why you don't want to push a patch that breaks all your scores... ;-) Does this latest patch break your

DOC: NR Dynamics context and postfix dynamics (issue3743045)

2010-12-21 Thread Carl . D . Sorensen
This looks even better to me. Thanks! I have a few comments below. I'm delighted to have you manage the review and updates for this patch. When it's time to apply the patch, if you'll create a patch using git-format-patch, and email it to somebody to push (maybe Trevor Daniels, or me), then

Re: Documentation of Dynamics context and postfix dynamics changes (issue3732046)

2010-12-20 Thread Carl . D . Sorensen
On 2010/12/20 06:44:18, Keith wrote: Tell me if it is trivial to load patches from lily-git onto Reitveld. The Contributors Guide implies that the lily-git patches are intended for emailing to regular git-users. It is not too hard, but not trivial, to load patches from lily-git onto

Re: Documentation of Dynamics context and postfix dynamics changes (issue3732046)

2010-12-20 Thread Carl . D . Sorensen
http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressive.itely File Documentation/notation/expressive.itely (right): http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressive.itely#newcode313 Documentation/notation/expressive.itely:313: \bar || e2\p\ f

Re: [Patch] make post-event music functions direction-aware (issue3743043)

2010-12-20 Thread Carl . D . Sorensen
On 2010/12/20 16:01:46, Valentin Villenave wrote: On 2010/12/20 15:31:43, Carl wrote: This seems like a cool idea! Thanks! As said before, it really isn't mine :) Could it be made more simple by just letting the 0 direction be set as a music property, instead of adding only the 1 and

Re: Doc: More NR and LM additions based on user emails (issue3705042)

2010-12-19 Thread Carl . D . Sorensen
I like it, and think it's ready for pushing. Just for purposes of clarification, let me summarize the original problem, since I didn't do a good job before. The syntax of alternative is: \alternative { ending_one ending_two ending_three } NOT \alternative { { ending_one } {

Re: Rewrite NR 3.2 Titles and headers. (issue3667041)

2010-12-19 Thread Carl . D . Sorensen
Mark, This is really useful stuff. Thanks! However, I agree with Trevor about the LM vs NR style of writing. The NR needs to be terse and show, rather than tell, the story. In an effort to be clear, we often try to both tell and show. I spent a lot of time when I first started writing NR

Re: Fix 1456 (issue3693042)

2010-12-19 Thread Carl . D . Sorensen
Thanks for the review, Patrick. I've made the changes and posted a new patch set. http://codereview.appspot.com/3693042/diff/9001/Documentation/notation/rhythms.itely File Documentation/notation/rhythms.itely (right):

Documentation of Dynamics context and postfix dynamics changes (issue3732046)

2010-12-19 Thread Carl . D . Sorensen
Reviewers: , Message: Here are Keith's suggested doc changes for the Dynamics context and postfix (de)crescendo notation. Description: Documentation of Dynamics context and postfix dynamics changes Please review this at http://codereview.appspot.com/3732046/ Affected files: M

Doc: NR cross-staff (issue3782042)

2010-12-19 Thread Carl . D . Sorensen
Reviewers: , Message: Here's Keith's patch for cross-staff documentation. LGTM. Description: Doc: NR cross-staff Some collisions are allowed by design, as controlled by the 'cross-staff' property. Please review this at http://codereview.appspot.com/3782042/ Affected files: M

Re: Documentation of Dynamics context and postfix dynamics changes (issue3732046)

2010-12-19 Thread Carl . D . Sorensen
Looks mostly good to me -- one styling nit. Thanks! Carl http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressive.itely File Documentation/notation/expressive.itely (right): http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressive.itely#newcode313

Re: Doc: More NR and LM additions based on user emails (issue3705042)

2010-12-17 Thread Carl . D . Sorensen
Thanks, James! Carl http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundamental.itely File Documentation/learning/fundamental.itely (right): http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundamental.itely#newcode2114

Re: Doc: More NR and LM additions based on user emails (issue3705042)

2010-12-17 Thread Carl . D . Sorensen
Fine to push, but one last try on the barcheck warning. Thanks, Carl http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.itely File Documentation/notation/repeats.itely (right):

Re: Fix 1456 (issue3693042)

2010-12-17 Thread Carl . D . Sorensen
Thanks for the excellent feedback. I've fixed everything now. A revised patch set is available. http://codereview.appspot.com/3693042/diff/1/Documentation/notation/rhythms.itely File Documentation/notation/rhythms.itely (left):

Fix 1456 (issue3693042)

2010-12-16 Thread Carl . D . Sorensen
Reviewers: , Message: Here's a fix for issue 1456. I've taken the context argument out of \overrideTimeSignatureSettings and \revertTimeSignatureSettings. They now apply to the Timing context. I've also added an example showing how to have different time signature settings in different

Re: harmonics and slides (issue3590041)

2010-12-11 Thread Carl . D . Sorensen
Looks pretty good to me. Looks *excellent* for a first patch. Thanks, Carl http://codereview.appspot.com/3590041/diff/1/Documentation/notation/fretted-strings.itely File Documentation/notation/fretted-strings.itely (right):

Re: Doc: Various to LM and NR from user email threads (issue3581041)

2010-12-11 Thread Carl . D . Sorensen
On 2010/12/11 16:33:48, pkx166h wrote: I hope this is the right method to have a discussion on Rietveld, I have added my own comments/responses to yours inline. Yes, this is a very good way to do it. You can also just add a comment in the patchset.

Fix 1336 (issue3594041)

2010-12-11 Thread Carl . D . Sorensen
Reviewers: , Message: Here's a patch for issue 1336 that combine's Neil's idea of not creating the paper columns with Graham's idea of erroring out when there are less than 2 columns. Thanks, Carl Description: Fix 1336 Please review this at http://codereview.appspot.com/3594041/ Affected

Re: Add /chordGlissando to music functions (issue3530042)

2010-12-11 Thread Carl . D . Sorensen
On 2010/12/11 18:52:28, Valentin Villenave wrote: I really like it. [snip ..] That being said, if we're to start accepting such tricks in the default distribution, then I can think of several great snippets that would also make good candidates for inclusion :-) Maybe we could create a

Re: Add /chordGlissando to music functions (issue3530042)

2010-12-11 Thread Carl . D . Sorensen
On 2010/12/11 19:16:13, Valentin Villenave wrote: On Sat, Dec 11, 2010 at 8:04 PM, mailto:carl.d.soren...@gmail.com wrote: If we started doing this, though, how would we distinguish between hacks that go only in the LSR and those that go in the hacks/ directory? I like this a lot! I

Re: Add /chordGlissando to music functions (issue3530042)

2010-12-11 Thread Carl . D . Sorensen
On 2010/12/11 19:42:50, Graham Percival wrote: Could it go in a separate optional-include \include guitar.ly file? I'd sign off with no qualms if it were optional. Absolutely, but it won't be guitar.ly, it'll be stringed-instruments.ly. New patch coming soon.

Re: Fix 1336 (issue3594041)

2010-12-11 Thread Carl . D . Sorensen
On 2010/12/11 20:28:18, Valentin Villenave wrote: I'm just not entirely sure about the error message: http://codereview.appspot.com/3594041/diff/1/lily/simple-spacer.cc File lily/simple-spacer.cc (right): http://codereview.appspot.com/3594041/diff/1/lily/simple-spacer.cc#newcode397

Re: Add /chordGlissando to music functions (issue3530042)

2010-12-11 Thread Carl . D . Sorensen
On 2010/12/11 20:11:45, Carl wrote: On 2010/12/11 19:42:50, Graham Percival wrote: Could it go in a separate optional-include \include guitar.ly file? I'd sign off with no qualms if it were optional. Absolutely, but it won't be guitar.ly, it'll be stringed-instruments.ly. I've

Re: Fix bookpart identifier crash with page-marker. (issue3585042)

2010-12-11 Thread Carl . D . Sorensen
LGTM http://codereview.appspot.com/3585042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Don't make paper columns if skipTypesetting = ##t. (issue3598041)

2010-12-11 Thread Carl . D . Sorensen
LGTM. http://codereview.appspot.com/3598041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Move note names to scm (issue3247041)

2010-12-09 Thread Carl . D . Sorensen
LGTM. Carl P.S. Right now I can't see anything for scm/note-names.scm without downloading the patch. I don't know how this is so messed up on rietveld, but it certainly is http://codereview.appspot.com/3247041/ ___ lilypond-devel mailing list

Re: Doc: CG: clarify lily-git.tcl and git-cl. (issue3396042)

2010-12-06 Thread Carl . D . Sorensen
LGTM. Carl http://codereview.appspot.com/3396042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Fix #1427. (issue3319041)

2010-12-06 Thread Carl . D . Sorensen
LGTM. Do you think it's ready to go now? http://codereview.appspot.com/3319041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Doc: NR 4: Minor edits. (issue3406041)

2010-12-04 Thread Carl . D . Sorensen
LGTM. Carl http://codereview.appspot.com/3406041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Add tab-tie-follow-engraver (issue2723043)

2010-12-04 Thread Carl . D . Sorensen
On 2010/12/05 00:58:13, Neil Puttock wrote: I don't know, but they're consistently shifted to the right unless there are double-digit notes present. I wanted your opinion on the spacing in the .png I sent. I adjusted the offset, but it wasn't part of this patch. I have deliberately shifted

Re: Add tab-tie-follow-engraver (issue2723043)

2010-12-04 Thread Carl . D . Sorensen
On 2010/12/05 00:58:13, Neil Puttock wrote: I don't know, but they're consistently shifted to the right unless there are double-digit notes present. I wanted your opinion on the spacing in the .png I sent. I adjusted the offset, but it wasn't part of this patch. I have deliberately shifted

Re: Add tab-tie-follow-engraver (issue2723043)

2010-12-03 Thread Carl . D . Sorensen
On 2010/11/28 15:42:47, marc wrote: Just some remarks about the harmonic detection. Thanks, Marc. I wrote a slightly different version of this (using filter). It works great! http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list

Re: Add tab-tie-follow-engraver (issue2723043)

2010-12-03 Thread Carl . D . Sorensen
Thanks for the feedback. I've responded to the comments and posted a new patch set. http://codereview.appspot.com/2723043/diff/109001/lily/tab-harmonic-engraver.cc File lily/tab-harmonic-engraver.cc (left):

Re: Add harmonic by fret and by ratio to tablatures (issue3326042)

2010-12-03 Thread Carl . D . Sorensen
This code has now been integrated in the tab-tie-follow-engraver changes, due to my mistakes in git. Please see http://codereview.appspot.com/2723043/ Thanks, Carl http://codereview.appspot.com/3326042/ ___ lilypond-devel mailing list

Fix 1248 (issue3416045)

2010-12-03 Thread Carl . D . Sorensen
Reviewers: Neil Puttock, Message: Here's a proposed patch for allowing a \bookpart variable in a \book. Description: Fix 1248 Change test from !scm_is_null to scm_is_pair Add regression test Please review this at http://codereview.appspot.com/3416045/ Affected files: A

Re: Fix #1427. (issue3319041)

2010-12-03 Thread Carl . D . Sorensen
On 2010/11/25 00:32:58, Neil Puttock wrote: OK, regtest's on hold while I work out why the following doesn't work: The regtest runs with my proposed patch for issue 1428. Thanks, Carl http://codereview.appspot.com/3319041/ ___ lilypond-devel

Fix 1252 by compressing page (issue3422041)

2010-12-02 Thread Carl . D . Sorensen
Reviewers: , Message: Here's a patch to fix issue 1252 (music overflows page) by compressing music together. It may cause collisions, but it should solve the critical overflow error. Description: Fix 1252 by compressing page Please review this at http://codereview.appspot.com/3422041/

Re: Fix 1252 by compressing page (issue3422041)

2010-12-02 Thread Carl . D . Sorensen
Thanks for the review, Neil. http://codereview.appspot.com/3422041/diff/1/lily/page-layout-problem.cc File lily/page-layout-problem.cc (right): http://codereview.appspot.com/3422041/diff/1/lily/page-layout-problem.cc#newcode313 lily/page-layout-problem.cc:313: double overflow =

Re: Add harmonic by fret and by ratio to tablatures (issue3326042)

2010-11-28 Thread Carl . D . Sorensen
I forgot to mention that this patch applies on top of http://codereview.appspot.com/2723043/ Thanks, Carl http://codereview.appspot.com/3326042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org

Re: Add tab-tie-follow-engraver (issue2723043)

2010-11-28 Thread Carl . D . Sorensen
On 2010/11/28 22:24:05, Neil Puttock wrote: http://codereview.appspot.com/2723043/diff/109001/lily/tab-harmonic-engraver.cc#newcode59 lily/tab-harmonic-engraver.cc:59: victim-set_property (style, ly_symbol2scm (harmonic)); This makes it difficult for users to tweak the appearance of the

Re: Add tab-tie-follow-engraver (issue2723043)

2010-11-27 Thread Carl . D . Sorensen
Thanks, Marc. That was a mistake I made in resolving merge conflicts. http://codereview.appspot.com/2723043/diff/103001/scm/tablature.scm File scm/tablature.scm (right): http://codereview.appspot.com/2723043/diff/103001/scm/tablature.scm#newcode291 scm/tablature.scm:291:

Add harmonic by fret and by ratio to tablatures (issue3326042)

2010-11-27 Thread Carl . D . Sorensen
Reviewers: marc, p.l.schmidt_gmx.de, Message: I've taken Marc's harmonic.ly and moved the scheme functions into scm/tablature.scm, the music functions into ly/engraver-init.ly, and the test music into input/regression/tablature-harmonic-functions.ly. I've also added docstrings for the functions

Re: Modify fret calculation algorithm (issue3320041)

2010-11-26 Thread Carl . D . Sorensen
On 2010/11/26 19:39:06, steve.yegge wrote: Hi Carl, I looked through the code; it looks great. I built it and tested it fairly thoroughly, and found no problems with it -- both features work as advertised. Thanks for the testing! I tried defaultStrings with single-string melodies,

Re: PartCombine: Keep track of the state in the Part_combine_engraver (issue3334043)

2010-11-26 Thread Carl . D . Sorensen
Looks like a good approach to improve the part combiner to me. Thanks, Carl http://codereview.appspot.com/3334043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Add tab-tie-follow-engraver (issue2723043)

2010-11-26 Thread Carl . D . Sorensen
Thanks for the review, Neil. I've responded to all your comments. I've also defined a new print function for TabNoteHeads in Scheme. It will take care of all of the necessary parentheses and harmonic brackets, based on the settings of 'display-cautionary and 'style. Thanks, Carl

Re: Modify fret calculation algorithm (issue3320041)

2010-11-26 Thread Carl . D . Sorensen
I've responded to all of Neil's comments. Thanks for the review, Neil! Carl http://codereview.appspot.com/3320041/diff/4001/input/regression/tablature-default-strings.ly File input/regression/tablature-default-strings.ly (right):

Re: Move note names to scm (issue3247041)

2010-11-25 Thread Carl . D . Sorensen
On 2010/11/25 12:24:44, Valentin Villenave wrote: On 2010/11/24 00:10:01, Neil Puttock wrote: The indentation inside language-pitch-names is artificially compressed, e.g., That allowed me to make git regard the file as renamed instead of rewritten. But you're right, I've fixed the

Re: [Patch] Add support for tempo ranges (issue3248042)

2010-11-25 Thread Carl . D . Sorensen
On 2010/11/25 11:42:40, Valentin Villenave wrote: On 2010/11/25 00:10:01, Neil Puttock wrote: That sounds OK to me. In scm/translation-functions.scm, I'd prefer to see the argument called count, not count-or-range. The property is timeUnitCount, IIUC. The count can either be a number or a

Ties: Print out a warning if a tie cannot be created (issue3310042)

2010-11-25 Thread Carl . D . Sorensen
LGTM. Carl http://codereview.appspot.com/3310042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

FiguredBass: Extenders for figures of different width should still stop at the same position (issue3280042)

2010-11-24 Thread Carl . D . Sorensen
LGTM. Carl http://codereview.appspot.com/3280042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Modify fret calculation algorithm (issue3320041)

2010-11-24 Thread Carl . D . Sorensen
Reviewers: steve yegge steve.yegge_gmail.com, patrick schmidt p.l.schmidt_gmx.de, Message: Here's a patch to the fret calculator that answers the first two of Steve's requests, I think. http://article.gmane.org/gmane.comp.gnu.lilypond.general/60177 It turns a -0 fingering into an open string

Re: Move note names to scm (issue3247041)

2010-11-23 Thread Carl . D . Sorensen
I couldn't comment on the last two files in the patch set. Here are some comments on the other files. Thanks, Carl http://codereview.appspot.com/3247041/diff/9001/ly/music-functions-init.ly File ly/music-functions-init.ly (right):

Re: Move note names to scm (issue3247041)

2010-11-23 Thread Carl . D . Sorensen
On 2010/11/23 16:25:13, Valentin Villenave wrote: On 2010/11/23 15:09:14, Carl wrote: ly/predefined-guitar-fretboards.ly:456: #(ly:parser-set-note-names parser pitchnames) Why is this call different from the call in line 20? It seems to me that it should be the same. Ideally it

Re: [Patch] Add support for tempo ranges (issue3248042)

2010-11-23 Thread Carl . D . Sorensen
On 2010/11/24 00:28:37, Neil Puttock wrote: The only thing I'm concerned about is the hybrid type; it's poorly named: `number-or-pair?' doesn't suggest a number-pair, but `number-or-number-pair?' is even worse. I think using a list might a price worth paying to avoid creating a new

Re: [Patch] Fix #1333: beamed multi-note acciaccaturas (issue3169041)

2010-11-18 Thread Carl . D . Sorensen
Valentin, I couldn't comment inline on the scm file, because it gave me a server error when I tried. Here's the comments I put in on your new macro: I'm concerned about redefining a public function so that the arguments mean totally different things. This will totally break any existing

Add ly:grob-array-list; use in live-elements-list. (issue3104044)

2010-11-17 Thread Carl . D . Sorensen
LGTM Carl http://codereview.appspot.com/3104044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Checks for git user details (issue3182041)

2010-11-17 Thread Carl . D . Sorensen
LGTM. Thanks for rationalizing the indentation. Carl http://codereview.appspot.com/3182041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Fix 1382 (issue3100041)

2010-11-14 Thread Carl . D . Sorensen
On 2010/11/14 11:22:44, Valentin Villenave wrote: Hi Carl, looks good to me! I just have minor questions wrt coding style, but these are mainly because of my own ignorance :) http://codereview.appspot.com/3100041/diff/2001/input/regression/zero_staff_space.ly File

Re: Doc: NR 4.4.1: Clean up property descriptions. (issue3089042)

2010-11-14 Thread Carl . D . Sorensen
LGTM. Carl http://codereview.appspot.com/3089042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Fix 1382 (issue3100041)

2010-11-14 Thread Carl . D . Sorensen
On 2010/11/14 14:32:48, Valentin Villenave wrote: On Sun, Nov 14, 2010 at 3:17 PM, mailto:carl.d.soren...@gmail.com wrote: I don't see how putting the names as variables helps anything. nbsp;The whole string will need to be translated for localization, rather than doing it one word at

Re: Fix 1382 (issue3100041)

2010-11-14 Thread Carl . D . Sorensen
On 2010/11/14 14:32:48, Valentin Villenave wrote: On Sun, Nov 14, 2010 at 3:17 PM, mailto:carl.d.soren...@gmail.com wrote: I don't see how putting the names as variables helps anything. nbsp;The whole string will need to be translated for localization, rather than doing it one word at

Re: Update spacing for Keith Ohara's suggested defaults (issue3099041)

2010-11-14 Thread Carl . D . Sorensen
Pushed to git. Thanks, Carl http://codereview.appspot.com/3099041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Changes to Default spacing settings (issue3061041)

2010-11-13 Thread Carl . D . Sorensen
I've posted a patch with the new spacing issues to reitveld. http://codereview.appspot.com/3099041 Keith, could you please check it to make sure it's right? Thanks, Carl http://codereview.appspot.com/3061041/ ___ lilypond-devel mailing list

Update spacing for Keith Ohara's suggested defaults (issue3099041)

2010-11-13 Thread Carl . D . Sorensen
Reviewers: , Message: Here's the new default spacing patch with the new names. Keith, can you check it out? THanks, Carl Description: Update spacing for Keith Ohara's suggested defaults Please review this at http://codereview.appspot.com/3099041/ Affected files: M ly/engraver-init.ly

Re: Update spacing for Keith Ohara's suggested defaults (issue3099041)

2010-11-13 Thread Carl . D . Sorensen
Thanks, Keith. Done. Carl http://codereview.appspot.com/3099041/diff/1/ly/engraver-init.ly File ly/engraver-init.ly (right): http://codereview.appspot.com/3099041/diff/1/ly/engraver-init.ly#newcode418 ly/engraver-init.ly:418: \override VerticalAxisGroup #'nonstaff-unrelatedstaff-spacing

Re: Update spacing for Keith Ohara's suggested defaults (issue3099041)

2010-11-13 Thread Carl . D . Sorensen
On 2010/11/13 22:38:36, Graham Percival wrote: Looks good to me, thanks Carl! I'll make .39 once this is applied, but let's wait 24 hours before pushing to give people in all time zones a chance to comment. OK, I'll wait till tomorrow night. Carl http://codereview.appspot.com/3099041/

Re: Update spacing for Keith Ohara's suggested defaults (issue3099041)

2010-11-13 Thread Carl . D . Sorensen
Thanks, Patrick. Carl http://codereview.appspot.com/3099041/diff/6001/ly/paper-defaults-init.ly File ly/paper-defaults-init.ly (right): http://codereview.appspot.com/3099041/diff/6001/ly/paper-defaults-init.ly#newcode60 ly/paper-defaults-init.ly:60: system-system-spacing = #'((space . 12)

Fix 1382 (issue3100041)

2010-11-13 Thread Carl . D . Sorensen
Reviewers: carl.d.sorensen_gmail.com, Message: Here's a fix for Issue 1382. It sets the staff position to zero if staff space is zero, which is a consistent outcome -- all the staff lines are in the same position so zero works. Please review. Thanks, Carl Description: Fix 1382 Please

Re: vert. spacing: Rename properties (lily, scm). (issue3031041)

2010-11-12 Thread Carl . D . Sorensen
On 2010/11/12 09:06:06, Trevor Daniels wrote: http://codereview.appspot.com/3031041/diff/40001/lily/axis-group-interface.cc#newcode780 lily/axis-group-interface.cc:780: The following keys may be set in the spacing alists:\n I'm not sure. The current description is in the interface. If

Re: vert. spacing: Rename properties (lily, scm). (issue3031041)

2010-11-12 Thread Carl . D . Sorensen
On 2010/11/12 22:48:57, Mark Polesky wrote: staff-staff-spacing When applied to a staff-group's StaffGrouper grob, this spacing alist controls the distance between consecutive staves within the staff-group. When applied to a staff's VerticalAxisGroup grob, it controls the distance between

Re: vert. spacing: Rename properties (lily, scm). (issue3031041)

2010-11-12 Thread Carl . D . Sorensen
On 2010/11/12 17:19:17, Mark Polesky wrote: On 2010/11/12 02:46:18, Carl wrote: One tiny modification I propose is to put the alist-key descriptions into default-staff-staff- instead of staff-staff- because otherwise the prop descriptions on the IR staff-grouper-interface page would be referring

Re: vert. spacing: Rename properties (lily, scm). (issue3031041)

2010-11-12 Thread Carl . D . Sorensen
On 2010/11/13 00:50:51, Mark Polesky wrote: So, could it possibly be? Is this okay to push? I like it a lot! Just one more thing -- James has captured new default values from Keith Ohara's work. Perhaps these new default values should be added as part of this patch.

Re: vert. spacing: Rename properties (lily, scm). (issue3031041)

2010-11-12 Thread Carl . D . Sorensen
LGTM. What happened to the description of reference points for various objects? Has that disappeared, or did I just miss it? Will it be coming back somewhere? Thanks, Carl http://codereview.appspot.com/3031041/diff/64001/Documentation/learning/fundamental.itely File

Re: Add tab-tie-follow-engraver (issue2723043)

2010-11-12 Thread Carl . D . Sorensen
Thanks for the help on the null pointer. I was thinking that it was some *other* kind of variable than a Grob, and that I was casting it wrong. Got that all taken care of -- no scheme calls at all. On 2010/11/12 17:40:42, Neil Puttock wrote: It's back to square one though, isn't it? The

Re: vert. spacing: Rename properties (lily, scm). (issue3031041)

2010-11-11 Thread Carl . D . Sorensen
Thanks again for your work on this, Mark. I think the spacing-alist description belongs in one of the grob properties; it's the value of a property, not a property of an interface. I much prefer this to the previous patch. I had an idea about the name of the StaffGrouper staff-staff-spacing

Re: Add tab-tie-follow-engraver (issue2723043)

2010-11-11 Thread Carl . D . Sorensen
On 2010/11/07 09:21:45, marc wrote: http://codereview.appspot.com/2723043/diff/57001/scm/define-grob-properties.scm#newcode1021 scm/define-grob-properties.scm:1021: (display-cautionary ,boolean? Display in cautionary form.) Hey, good choice - I like the name of the new property, it's much

Re: Add tab-tie-follow-engraver (issue2723043)

2010-11-11 Thread Carl . D . Sorensen
On 2010/11/07 09:24:31, marc wrote: Am 07.11.2010 08:47, schrieb carl.d.soren...@gmail.com: In this patch, the glissando, slur, tie, and repeat-tie callbacks have been modified. The slur callback doesn't even check for the tie status, since there's no change. But it has to take the value

Re: Add tab-tie-follow-engraver (issue2723043)

2010-11-11 Thread Carl . D . Sorensen
Here's a new version of the patch that tries to eliminate any scheme calls, and demonstrates my problem. I need to do a check on the left bound to see if it's a grob, because if I take the check out I get a Bus error. I don't know how to do such a check from C++ -- I think it's done by casting,

Re: vert. spacing: Rename properties (lily, scm). (issue3031041)

2010-11-10 Thread Carl . D . Sorensen
On 2010/11/11 03:53:44, Mark Polesky wrote: Here's a new patch-set for renaming the vertical spacing grob properties. But... I'm kind of going in circles trying to decide where to put some things: 1) individual alist-key descriptions 2) individual grob-property descriptions 3) non-staff

Re: vert. spacing: Rename properties (lily, scm). (issue3031041)

2010-11-10 Thread Carl . D . Sorensen
Many of my general comments were in my reply to your message. There are some specifics in the files here. Thanks, Carl http://codereview.appspot.com/3031041/diff/30001/lily/axis-group-interface.cc File lily/axis-group-interface.cc (right):

Re: Add tab-tie-follow-engraver (issue2723043)

2010-11-07 Thread Carl . D . Sorensen
This patch has revised the callbacks in scm/tablature.scm. It appears to work properly. The tab-tie-follow-engraver is still using SCM calls to analyze the grobs. I will get that worked out soon, I hope (although I seem to have gone backward, not forward, today). In this patch, the glissando,

Re: Add tab-tie-follow-engraver (issue2723043)

2010-11-06 Thread Carl . D . Sorensen
On 2010/11/04 08:31:30, marc wrote: No, I don't think we should it do more complicated that necessary. Perhaps the name 'tie-follow is misleading, but the engraver (before you left out the slur and glissando bits) did the right job - marking exactly the tab-note-heads that have to be

Doc: CG -- Add statement on lilypond-hackers list (issue2933041)

2010-11-05 Thread Carl . D . Sorensen
Reviewers: , Message: I've posted a patch that adds information about lilypond-hackers to the Contributors' Guide. Please review. Thanks, Carl Description: Doc: CG -- Add statement on lilypond-hackers list Please review this at http://codereview.appspot.com/2933041/ Affected files: M

Re: Doc: CG -- Add statement on lilypond-hackers list (issue2933041)

2010-11-05 Thread Carl . D . Sorensen
I've made the changes suggested in the comments. Thanks for the review. As far as its own section vs. part of unsorted policies -- Graham preferred its own section, so I followed his suggestion. I think that ultimately it will be its own section in the CG, because we'll want to document

Re: Doc: CG -- Add statement on lilypond-hackers list (issue2933041)

2010-11-05 Thread Carl . D . Sorensen
Pushed. http://git.savannah.gnu.org/gitweb/?p=lilypond.git;a=commit;h=c467e8f32a49ee397160d7eeccc287e65bc88e5c http://codereview.appspot.com/2933041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org

Re: Doc: NR 4.4.1: Rewrite. (issue2642043)

2010-11-04 Thread Carl . D . Sorensen
LGTM. Thanks, Mark. Carl http://codereview.appspot.com/2642043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Add tab-tie-follow-engraver (issue2723043)

2010-11-03 Thread Carl . D . Sorensen
On 2010/11/03 20:46:27, Neil Puttock wrote: Hi Carl, What do you think about folding this code into the Tab_note_heads_engraver? We could store the created TabNoteHead grobs and add an acknowledger for Tie, then set 'tie-follow in stop_translation_timestep (). I think I'd probably go

Re: Doc: NR: Move Modifying alists from 4.1.2 to 5.3. (issue2767043)

2010-11-02 Thread Carl . D . Sorensen
Maybe the solution is to show the modification of nested-alist properties *other* than spacing alists. For example, fret-diagram-details is a nested alist. And TextSpanner has bound-details as a nested alist. So is harp-pedal-details. And Stem 'details. There are probably more, as well.

Re: Doc: NR: Move Modifying alists from 4.1.2 to 5.3. (issue2767043)

2010-11-02 Thread Carl . D . Sorensen
LGTM. One comment. THanks, Carl http://codereview.appspot.com/2767043/diff/11001/Documentation/notation/changing-defaults.itely File Documentation/notation/changing-defaults.itely (right):

<    1   2   3   4   5   6   7   8   9   10   >