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

Issue 6850073: markup-commands rest-by-number and rest (Closed)

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

Description

markup-commands rest-by-number and rest Introduces two new markup-commands: rest-by-number and rest similiar to the existing note-by-number and note. Two regression-tests for them are added.

Patch Set 1 #

Total comments: 1

Patch Set 2 : Rewording descriptions, adding texinfo markup #

Patch Set 3 : cleaning up some copy and paste error #

Total comments: 1

Patch Set 4 : some changes of conditions and other small alterations #

Total comments: 3

Patch Set 5 : Simplifications #

Patch Set 6 : Cleaning up a copy/paste error in description #

Total comments: 2

Patch Set 7 : Better indentation, minor changes #

Total comments: 2

Patch Set 8 : Better indentation, next try. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+388 lines, -0 lines) Patch
A input/regression/markup-rest.ly View 1 2 3 4 5 6 7 1 chunk +90 lines, -0 lines 0 comments Download
A input/regression/markup-rest-styles.ly View 1 2 3 4 5 6 7 1 chunk +41 lines, -0 lines 0 comments Download
M scm/define-markup-commands.scm View 1 2 3 4 5 6 7 1 chunk +257 lines, -0 lines 0 comments Download

Messages

Total messages: 24
thomasmorley65
Please review
11 years, 4 months ago (2012-11-18 23:54:37 UTC) #1
thomasmorley65
2012/11/19 <thomasmorley65@googlemail.com>: > Reviewers: , > > Message: > Please review > > Description: > ...
11 years, 4 months ago (2012-11-19 00:02:23 UTC) #2
lemzwerg
http://codereview.appspot.com/6850073/diff/1/scm/define-markup-commands.scm File scm/define-markup-commands.scm (right): http://codereview.appspot.com/6850073/diff/1/scm/define-markup-commands.scm#newcode3341 scm/define-markup-commands.scm:3341: breve, longa and maxima are valid input-strings This should ...
11 years, 4 months ago (2012-11-19 05:11:38 UTC) #3
thomasmorley65
Hi Werner, thanks for reviewing. New patch set uploded.
11 years, 4 months ago (2012-11-19 21:46:10 UTC) #4
thomasmorley65
On 2012/11/19 05:11:38, lemzwerg wrote: > http://codereview.appspot.com/6850073/diff/1/scm/define-markup-commands.scm > File scm/define-markup-commands.scm (right): > > http://codereview.appspot.com/6850073/diff/1/scm/define-markup-commands.scm#newcode3341 > ...
11 years, 4 months ago (2012-11-19 21:48:23 UTC) #5
benko.pal
http://codereview.appspot.com/6850073/diff/9001/scm/define-markup-commands.scm File scm/define-markup-commands.scm (right): http://codereview.appspot.com/6850073/diff/9001/scm/define-markup-commands.scm#newcode3247 scm/define-markup-commands.scm:3247: "M" there are no separate glyphs for rests and ...
11 years, 4 months ago (2012-11-21 08:05:09 UTC) #6
thomasmorley65
On 2012/11/21 08:05:09, benko.pal wrote: [...] > there are no separate glyphs for rests and ...
11 years, 4 months ago (2012-11-25 23:03:43 UTC) #7
benko.pal
On 2012/11/25 23:03:43, thomasmorley65 wrote: > On 2012/11/21 08:05:09, benko.pal wrote: > [...] > > ...
11 years, 4 months ago (2012-11-26 11:37:26 UTC) #8
dak
On 2012/11/26 11:37:26, benko.pal wrote: > I just meant that instead of checking several times ...
11 years, 4 months ago (2012-11-26 11:51:03 UTC) #9
dak
On 2012/11/26 11:51:03, dak wrote: > On 2012/11/26 11:37:26, benko.pal wrote: > > > I ...
11 years, 4 months ago (2012-11-26 12:55:42 UTC) #10
benko.pal
> So I'd suggest not treating the '-' at all (to make things not more ...
11 years, 4 months ago (2012-11-26 15:09:58 UTC) #11
thomasmorley65
On 2012/11/26 15:09:58, benko.pal wrote: > > So I'd suggest not treating the '-' at ...
11 years, 4 months ago (2012-11-26 21:11:53 UTC) #12
benko.pal
On 2012/11/26 21:11:53, thomasmorley65 wrote: > On 2012/11/26 15:09:58, benko.pal wrote: > > http://codereview.appspot.com/6850073/diff/6004/scm/define-markup-commands.scm > ...
11 years, 4 months ago (2012-11-27 20:03:06 UTC) #13
thomasmorley65
On 2012/11/27 20:03:06, benko.pal wrote: > > Did you had a look on the compiled ...
11 years, 4 months ago (2012-11-29 00:41:14 UTC) #14
benko.pal
On 2012/11/29 00:41:14, thomasmorley65 wrote: > On 2012/11/27 20:03:06, benko.pal wrote: > > > > ...
11 years, 4 months ago (2012-11-29 20:42:21 UTC) #15
thomasmorley65
On 2012/11/29 20:42:21, benko.pal wrote: > On 2012/11/29 00:41:14, thomasmorley65 wrote: > > On 2012/11/27 ...
11 years, 4 months ago (2012-11-30 01:33:08 UTC) #16
dak
thomasmorley65@googlemail.com writes: > On 2012/11/27 20:03:06, benko.pal wrote: > >> > Did you had a ...
11 years, 3 months ago (2012-12-02 15:41:01 UTC) #17
thomasmorley65
Better indentation, minor changes
11 years, 3 months ago (2012-12-03 02:03:37 UTC) #18
thomasmorley65
On 2012/11/30 01:33:08, thomasmorley65 wrote: > On 2012/11/29 20:42:21, benko.pal wrote: > https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest-styles.ly > > ...
11 years, 3 months ago (2012-12-03 20:22:09 UTC) #19
thomasmorley65
On 2012/12/02 15:41:01, dak wrote: > mailto:thomasmorley65@googlemail.com writes: > > > On 2012/11/27 20:03:06, benko.pal ...
11 years, 3 months ago (2012-12-03 20:30:34 UTC) #20
benko.pal
LGTM https://codereview.appspot.com/6850073/diff/17001/input/regression/markup-rest-styles.ly File input/regression/markup-rest-styles.ly (right): https://codereview.appspot.com/6850073/diff/17001/input/regression/markup-rest-styles.ly#newcode18 input/regression/markup-rest-styles.ly:18: (symbol->string style)) sorry for nitpicking, but please don't ...
11 years, 3 months ago (2012-12-04 19:49:00 UTC) #21
thomasmorley65
Better indentation, next try.
11 years, 3 months ago (2012-12-05 01:00:11 UTC) #22
thomasmorley65
On 2012/12/04 19:49:00, benko.pal wrote: > LGTM > > https://codereview.appspot.com/6850073/diff/17001/input/regression/markup-rest-styles.ly > File input/regression/markup-rest-styles.ly (right): > ...
11 years, 3 months ago (2012-12-05 01:15:56 UTC) #23
benko.pal
11 years, 3 months ago (2012-12-05 07:51:17 UTC) #24
LGTM, thanks!
Sign in to reply to this message.

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