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

Issue 6300092: Issue 2602: \unfoldRepeats and \repeat tremolo 7,14,15 produces weird output (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 9 months ago by dak
Modified:
11 years, 9 months ago
Reviewers:
colinpkcampbell
CC:
lilypond-devel_gnu.org
Base URL:
http://git.savannah.gnu.org/gitweb/?p=lilypond.git/trunk/
Visibility:
Public.

Description

Issue 2602: \unfoldRepeats and \repeat tremolo 7,14,15 produces weird output This just shamelessly copies the logic of make-repeat into unfold-repeats, on the assumption that we want to have the same rules apply for creating and unfolding tremoli.

Patch Set 1 #

Patch Set 2 : Also fix display, add regtest for display. #

Patch Set 3 : Fix whitespace errors and display regtest. #

Patch Set 4 : I can't claim to understand what happens with default durations in display-lily-tests.ly #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+59 lines, -59 lines) Patch
M input/regression/display-lily-tests.ly View 1 2 3 1 chunk +2 lines, -0 lines 2 comments Download
M scm/define-music-display-methods.scm View 1 2 3 2 chunks +34 lines, -39 lines 0 comments Download
M scm/music-functions.scm View 1 2 1 chunk +23 lines, -20 lines 0 comments Download

Messages

Total messages: 2
Colin Campbell
I see the values of the c'' have changed between patch sets. http://codereview.appspot.com/6300092/diff/6001/input/regression/display-lily-tests.ly File input/regression/display-lily-tests.ly ...
11 years, 9 months ago (2012-06-15 03:02:51 UTC) #1
dak
11 years, 9 months ago (2012-06-15 05:42:31 UTC) #2
http://codereview.appspot.com/6300092/diff/6001/input/regression/display-lily...
File input/regression/display-lily-tests.ly (right):

http://codereview.appspot.com/6300092/diff/6001/input/regression/display-lily...
input/regression/display-lily-tests.ly:193: \test ##[ \repeat tremolo 7 { c''32
b' } #]
On 2012/06/15 03:02:52, Colin Campbell wrote:
> Should these two lines both be c''32 or c''16?

No.  #[ #] is somewhat buggy and you can't, as a consequence, have one test use
a non-4 default duration when the next test starts with a 4.

Fixing this (in order to make the tests independent) would mean that every test
starts with a default 4.  This would affect a number of other tests.

I decided not to bother.  Try getting the tests to agree when both just use 32. 
It is not specific to those tests: you can't do this for any other sequence of
tests as well.
Sign in to reply to this message.

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