Fix determine-frets so that it preserves note order (issue4518045)

2011-05-09 Thread Carl . D . Sorensen
Reviewers: MikeSol, Message: In response to Mike's request, I've fixed the problem in determine-frets that sometimes changed the order of the notes in the chord. This keeps the glissando between the same notes in the TabStaff as in the Staff. I've made enough changes that I'd like to get a

Re: Fix determine-frets so that it preserves note order (issue4518045)

2011-05-09 Thread n . puttock
Hi Carl, LGTM, apart from some indentation issues. One question for you though: do you think the refactoring is an improvement on the current code? You could for example achieve the same result with the following: (sort string-fret-fingering-tuples (lambda (a b) ( (car a) (car

Re: Fix determine-frets so that it preserves note order (issue4518045)

2011-05-09 Thread Carl Sorensen
On 5/9/11 12:34 PM, n.putt...@gmail.com n.putt...@gmail.com wrote: Hi Carl, LGTM, apart from some indentation issues. One question for you though: do you think the refactoring is an improvement on the current code? You could for example achieve the same result with the following:

Re: branching

2011-05-09 Thread Joseph Wakeling
On 04/29/2011 07:15 PM, Carl Sorensen wrote: This might work, but fails to meet the major criterion of the proposed branching scheme. The proposal is to make 2.14 stable. Yes, that's why my proposal was to apply every BUGFIX to 2.14 first, not every patch. :-) (Of course, by bugfix, I mean

Re: Fix determine-frets so that it preserves note order (issue4518045)

2011-05-09 Thread Neil Puttock
On 9 May 2011 19:44, Carl Sorensen c_soren...@byu.edu wrote: There's no guarantee that the notes will be in decreasing string number order. That's the general case, but it's not required. In absolute mode I can write a C major chord as c' e' g' or g' e' c' and as far as I know, we

Re: Fix determine-frets so that it preserves note order (issue4518045)

2011-05-09 Thread n . puttock
http://codereview.appspot.com/4518045/diff/1/scm/translation-functions.scm File scm/translation-functions.scm (right): http://codereview.appspot.com/4518045/diff/1/scm/translation-functions.scm#newcode280 scm/translation-functions.scm:280: (define free-strings (map 1+ (iota (length tuning

Re: Bugfix for issue 1630 (issue4490045)

2011-05-09 Thread n . puttock
http://codereview.appspot.com/4490045/diff/1/lily/completion-note-heads-engraver.cc File lily/completion-note-heads-engraver.cc (right): http://codereview.appspot.com/4490045/diff/1/lily/completion-note-heads-engraver.cc#newcode204 lily/completion-note-heads-engraver.cc:204:

Re: Bugfix for issue 1630 (issue4490045)

2011-05-09 Thread karin . hoethker
http://codereview.appspot.com/4490045/diff/1/lily/completion-note-heads-engraver.cc File lily/completion-note-heads-engraver.cc (right): http://codereview.appspot.com/4490045/diff/1/lily/completion-note-heads-engraver.cc#newcode204 lily/completion-note-heads-engraver.cc:204:

Re: Fix calculation of vertical offset when 'staff-padding is set (issue4489042)

2011-05-09 Thread Janek WarchoĊ‚
2011/5/8 Trevor Daniels t.dani...@treda.co.uk lemniskata.bernoull...@gmail.com wrote Sunday, May 08, 2011 7:17 AM Style nitpick. http://codereview.appspot.com/4489042/diff/1/lily/side-position-interface.cc#newcode65 lily/side-position-interface.cc:65: bool include_staff = I see quite a