Re: Fix determine-frets so that it preserves note order (issue4518045)
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#newcode351 scm/translation-functions.scm:351: (note-pitch note) string)) (car pitch-entry) string)) `make check` was crashing on an unbound variable 'note'. Given line 345 above the fix was so obvious that I just pushed it. http://codereview.appspot.com/4518045/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fix determine-frets so that it preserves note order (issue4518045)
On 2011/05/26 03:26:32, Keith wrote: `make check` was crashing on an unbound variable 'note'. Given line 345 above the fix was so obvious that I just pushed it. Thanks for the fix. I had found that fix as well, and was getting ready to push it. But there's still another problem. Right now 'ignore has the same effect as 'recalculate, and it shouldn't. So this patch fixes compiling, but not the regtest. Thanks, Carl http://codereview.appspot.com/4518045/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fix determine-frets so that it preserves note order (issue4518045)
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 b) ;; end of determine-frets-and-strings Cheers, Neil http://codereview.appspot.com/4518045/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fix determine-frets so that it preserves note order (issue4518045)
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: (sort string-fret-fingering-tuples (lambda (a b) ( (car a) (car b) ;; end of determine-frets-and-strings 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 have no requirement that we write chords with the lowest note first. Originally, (before the harmonic became part of the tab note head) we could be order-agnostic, because each head had its own information necessary to place the glyph. But now we aren't order agnostic because of glissandos on each note, and because the harmonic needs to be associated with the proper note. Additionally, there are instruments (at least one, the ukulele) where strings do not increase in pitch, so pitch order (as chords are generally entered in relative mode) doesn't necessarily match string order (which is what the sort call you proposed creates). For these reasons, I think something like what I've done is better than just the sort code you proposed. But I'm open to having the discussion. Thanks for the feedback! Carl Cheers, Neil http://codereview.appspot.com/4518045/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fix determine-frets so that it preserves note order (issue4518045)
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 have no requirement that we write chords with the lowest note first. Ah, good point. I hadn't thought of that. :) In that case, it definitely LGTM. Cheers, Neil ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fix determine-frets so that it preserves note order (issue4518045)
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 could use srfi-1 iota here: (iota (length tuning) 1) http://codereview.appspot.com/4518045/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
