Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(252)

Issue 4518045: Fix determine-frets so that it preserves note order (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years ago by Carl
Modified:
7 years, 11 months ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Fix determine-frets so that it preserves note order

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+131 lines, -99 lines) Patch
M input/regression/tablature-harmonic.ly View 1 chunk +8 lines, -2 lines 0 comments Download
M scm/translation-functions.scm View 7 chunks +123 lines, -97 lines 2 comments Download

Messages

Total messages: 7
Carl
In response to Mike's request, I've fixed the problem in determine-frets that sometimes changed the ...
8 years ago (2011-05-09 14:17:16 UTC) #1
Neil Puttock
Hi Carl, LGTM, apart from some indentation issues. One question for you though: do you ...
8 years ago (2011-05-09 18:34:44 UTC) #2
c_sorensen
On 5/9/11 12:34 PM, "n.puttock@gmail.com" <n.puttock@gmail.com> wrote: > Hi Carl, > > LGTM, apart from ...
8 years ago (2011-05-09 18:45:06 UTC) #3
Neil Puttock
On 9 May 2011 19:44, Carl Sorensen <c_sorensen@byu.edu> wrote: > There's no guarantee that the ...
8 years ago (2011-05-09 18:53:21 UTC) #4
Neil 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)))) could use ...
8 years ago (2011-05-09 18:53:48 UTC) #5
Keith
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 ...
7 years, 12 months ago (2011-05-26 03:26:32 UTC) #6
Carl
7 years, 12 months ago (2011-05-26 03:36:08 UTC) #7
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
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b