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

Issue 4056041: Fix Issue 1035 -- Add context property for negative frets (Closed)

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

Description

Fix Issue 1035 -- Add context property for negative frets Add handleNegativeFrets, with possibilities of 'ignore, 'recalculate, 'include Reorder functions in scm/translation-functions.scm to put context in scope of calculate-frets-and-strings Add regression test.

Patch Set 1 #

Total comments: 2

Patch Set 2 : Add warning when recalculating fret number #

Patch Set 3 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+198 lines, -143 lines) Patch
A input/regression/tablature-negative-fret.ly View 1 chunk +32 lines, -0 lines 0 comments Download
M ly/engraver-init.ly View 1 2 2 chunks +3 lines, -0 lines 0 comments Download
M scm/define-context-properties.scm View 1 2 1 chunk +5 lines, -0 lines 0 comments Download
M scm/translation-functions.scm View 1 2 3 chunks +158 lines, -143 lines 0 comments Download

Messages

Total messages: 9
Carl
I've posted a patch for fixing issue 1035, by giving the user control over what ...
13 years, 3 months ago (2011-01-17 23:10:12 UTC) #1
marc
Hi Carl, LGTM - I don't think that many users will ever change handleNegativeFrets, but ...
13 years, 3 months ago (2011-01-18 19:42:41 UTC) #2
Carl
Thanks, Marc. Good suggestion. http://codereview.appspot.com/4056041/diff/1/scm/translation-functions.scm File scm/translation-functions.scm (right): http://codereview.appspot.com/4056041/diff/1/scm/translation-functions.scm#newcode394 scm/translation-functions.scm:394: ((eq? handle-negative 'recalculate) On 2011/01/18 ...
13 years, 3 months ago (2011-01-18 21:24:54 UTC) #3
Neil Puttock
Hi Carl, Is moving `determine-frets-and-strings' required for the patch to work? It makes reviewing the ...
13 years, 2 months ago (2011-01-23 17:54:23 UTC) #4
c_sorensen
On Jan 23, 2011, at 10:54 AM, "n.puttock@gmail.com" <n.puttock@gmail.com> wrote: > Hi Carl, > > ...
13 years, 2 months ago (2011-01-23 19:11:46 UTC) #5
Neil Puttock
On 2011/01/23 19:11:46, c_sorensen_byu.edu wrote: > I could make a separate patch that moves it, ...
13 years, 2 months ago (2011-01-23 22:25:45 UTC) #6
Graham Percival (old account)
LGTM, and passes regtests.
13 years, 2 months ago (2011-02-01 23:04:33 UTC) #7
Neil Puttock
LGTM. Just needs rebasing (I assume you don't want to delete tablature-dot-placement.ly)
13 years, 2 months ago (2011-02-02 23:55:59 UTC) #8
Carl
13 years, 2 months ago (2011-02-04 04:21:11 UTC) #9
On 2011/02/02 23:55:59, Neil Puttock wrote:
> LGTM.
> 
> Just needs rebasing (I assume you don't want to delete
> tablature-dot-placement.ly)

Done
Sign in to reply to this message.

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