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

Issue 1908041: feta-accordion.mf, convertrules.py, musicxml2ly.py: sanitize accordion symbol names (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 11 months ago by dak
Modified:
8 years, 2 months ago
Reviewers:
carl.d.sorensen, Neil Puttock, Graham Percival (old account)
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

feta-accordion.mf, convertrules.py, musicxml2ly.py: sanitize accordion symbol names

Patch Set 1 #

Patch Set 2 : Fix one convert script typo #

Total comments: 4

Patch Set 3 : convert strings without context, fold into current version convert rule #

Patch Set 4 : Fix oversight in convertrules.py from last commit #

Total comments: 3

Patch Set 5 : Address recent comments and check for workability. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+34 lines, -20 lines) Patch
M Documentation/snippets/accordion-discant-symbols.ly View 1 chunk +2 lines, -2 lines 0 comments Download
M mf/feta-accordion.mf View 8 chunks +8 lines, -8 lines 0 comments Download
M python/convertrules.py View 1 2 3 4 1 chunk +15 lines, -1 line 0 comments Download
M scripts/musicxml2ly.py View 2 chunks +9 lines, -9 lines 0 comments Download

Messages

Total messages: 17
Carl
Looks good, with the exception of the version being wrong for the convert rule. Carl ...
8 years, 11 months ago (2010-07-28 17:11:20 UTC) #1
dak
On 2010/07/28 17:11:20, Carl wrote: > Looks good, with the exception of the version being ...
8 years, 11 months ago (2010-07-29 06:02:24 UTC) #2
dak
http://codereview.appspot.com/1908041/diff/2001/3002 File mf/feta-accordion.mf (right): http://codereview.appspot.com/1908041/diff/2001/3002#newcode117 mf/feta-accordion.mf:117: fet_beginchar ("accordion register freebass", "freebass") On 2010/07/28 17:11:20, Carl ...
8 years, 11 months ago (2010-07-29 06:02:55 UTC) #3
c_sorensen
On 7/29/10 12:02 AM, "dak@gnu.org" <dak@gnu.org> wrote: > Reviewers: carl.d.sorensen_gmail.com, > > Message: > On ...
8 years, 11 months ago (2010-07-29 13:01:47 UTC) #4
Graham Percival (old account)
On 2010/07/29 13:01:47, c_sorensen_byu.edu wrote: > Right now you have put these conversions into a ...
8 years, 11 months ago (2010-07-29 17:43:37 UTC) #5
Neil Puttock
http://codereview.appspot.com/1908041/diff/2001/3003 File python/convertrules.py (right): http://codereview.appspot.com/1908041/diff/2001/3003#newcode3044 python/convertrules.py:3044: str = re.sub (r'(\\musicglyph\s*#"accordion\.)([a-zA-Z]+)"', Could be more generic, since ...
8 years, 11 months ago (2010-07-29 19:29:39 UTC) #6
dak
On 2010/07/29 19:29:39, Neil Puttock wrote: > http://codereview.appspot.com/1908041/diff/2001/3003 > File python/convertrules.py (right): > > http://codereview.appspot.com/1908041/diff/2001/3003#newcode3044 ...
8 years, 11 months ago (2010-07-29 20:05:30 UTC) #7
Neil Puttock
On 2010/07/29 20:05:30, dak wrote: > I don't think that convert-ly is supposed to deal ...
8 years, 11 months ago (2010-07-29 20:14:57 UTC) #8
dak
On 2010/07/29 20:14:57, Neil Puttock wrote: > On 2010/07/29 20:05:30, dak wrote: > > > ...
8 years, 11 months ago (2010-08-01 12:40:12 UTC) #9
dak
On 2010/07/29 17:43:37, Graham Percival wrote: > On 2010/07/29 13:01:47, c_sorensen_byu.edu wrote: > > Right ...
8 years, 11 months ago (2010-08-01 12:41:41 UTC) #10
dak
http://codereview.appspot.com/1908041/diff/2001/3003 File python/convertrules.py (right): http://codereview.appspot.com/1908041/diff/2001/3003#newcode3044 python/convertrules.py:3044: str = re.sub (r'(\\musicglyph\s*#"accordion\.)([a-zA-Z]+)"', On 2010/07/29 19:29:40, Neil Puttock ...
8 years, 11 months ago (2010-08-01 12:41:56 UTC) #11
Neil Puttock
On 2010/08/01 12:41:41, dak wrote: > I changed the source according to the suggestions, but ...
8 years, 11 months ago (2010-08-01 14:25:41 UTC) #12
Neil Puttock
http://codereview.appspot.com/1908041/diff/10002/18002 File python/convertrules.py (right): http://codereview.appspot.com/1908041/diff/10002/18002#newcode3014 python/convertrules.py:3014: 'accDot': 'dot', indent http://codereview.appspot.com/1908041/diff/10002/18002#newcode3018 python/convertrules.py:3018: 'accOldEE': 'oldEE'} missing 'accDiscant' ...
8 years, 11 months ago (2010-08-01 14:27:05 UTC) #13
dak
On 2010/08/01 14:27:05, Neil Puttock wrote: > http://codereview.appspot.com/1908041/diff/10002/18002 > File python/convertrules.py (right): > > http://codereview.appspot.com/1908041/diff/10002/18002#newcode3014 ...
8 years, 10 months ago (2010-08-01 18:25:39 UTC) #14
Neil Puttock
LGTM. Are you going to do the docs in a separate patch?
8 years, 10 months ago (2010-08-01 19:32:26 UTC) #15
dak
n.puttock@gmail.com writes: > LGTM. > > Are you going to do the docs in a ...
8 years, 10 months ago (2010-08-01 20:09:40 UTC) #16
c_sorensen
8 years, 10 months ago (2010-08-02 01:06:11 UTC) #17
On 8/1/10 2:42 PM, "n.puttock@gmail.com" <n.puttock@gmail.com> wrote:

> LSR snippets get stamped with the last convert rule's version whenever a
> full update's done, so the accordion snippet's version was already at
> 2.13.29 from the autobeaming changes.

And that's why the convert rule wouldn't automatically apply, and instead
needed to be forced manually.

Thanks,

Carl

Sign in to reply to this message.

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