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

Issue 3842041: Change stringTunings entries from semitones to pitches (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 6 months ago by Carl
Modified:
8 years, 5 months ago
Reviewers:
pkx166h, cpkc, marc, james.lowe
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Change stringTunings entries from semitones to pitches This lays the foundation for creating a TabKey grob that lists the pitches of each string in the tablature. Create a new file ly/string-tunings-init.ly Defines \makeStringTuning, which takes an absolute-mode chord as an argument, so stringTunings definitions are more easily human-readable. Collects all the default tunings from scm/tablatures.scm, and defines them using \makeStringTuning. Adds tunings for orchestral strings. Add table documenting predefined string tunings to appendix. Write convert-ly rule for change. Apply convert-ly to source tree. Edit updated source files to comply with standard .ly formatting guidelines. Change definition in python/musicexp.py Update Docs to match new syntax.

Patch Set 1 #

Patch Set 2 : Include changes to notation-appendices #

Patch Set 3 : Moved default string tunings to alist, auto-generated docs #

Total comments: 18

Patch Set 4 : Respond to reviewers' comments #

Patch Set 5 : Adjusted for 2.13.46, fixed alterations in convertrules #

Unified diffs Side-by-side diffs Delta from patch set Stats (+598 lines, -260 lines) Patch
M Documentation/de/notation/fretted-strings.itely View 1 2 3 4 2 chunks +5 lines, -2 lines 0 comments Download
M Documentation/es/notation/fretted-strings.itely View 1 2 3 4 2 chunks +5 lines, -2 lines 0 comments Download
M Documentation/fr/notation/fretted-strings.itely View 1 2 3 4 2 chunks +5 lines, -2 lines 0 comments Download
A Documentation/included/display-predefined-string-tunings.ly View 1 2 3 4 1 chunk +87 lines, -0 lines 0 comments Download
M Documentation/notation/fretted-strings.itely View 1 2 3 4 6 chunks +105 lines, -17 lines 0 comments Download
M Documentation/notation/notation-appendices.itely View 1 2 3 4 3 chunks +9 lines, -1 line 0 comments Download
M Documentation/snippets/defining-predefined-fretboards-for-other-instruments.ly View 1 2 3 1 chunk +0 lines, -167 lines 0 comments Download
A Documentation/snippets/new/defining-predefined-fretboards-for-other-instruments.ly View 1 2 3 4 1 chunk +167 lines, -0 lines 0 comments Download
M input/regression/tablature-letter.ly View 1 2 3 4 2 chunks +10 lines, -6 lines 0 comments Download
M input/regression/tablature-string-tunings.ly View 1 2 3 4 2 chunks +7 lines, -4 lines 0 comments Download
M ly/declarations-init.ly View 3 chunks +25 lines, -22 lines 0 comments Download
A ly/string-tunings-init.ly View 1 2 3 4 1 chunk +135 lines, -0 lines 0 comments Download
M python/convertrules.py View 1 2 3 4 1 chunk +35 lines, -1 line 0 comments Download
M python/musicexp.py View 1 chunk +2 lines, -2 lines 0 comments Download
M scm/tablature.scm View 1 chunk +0 lines, -33 lines 0 comments Download
M scm/translation-functions.scm View 1 2 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 14
Carl
Here's a patch that changes stringTunings entries from semitones to pitches. This is needed to ...
8 years, 6 months ago (2010-12-24 19:31:51 UTC) #1
Keith
I'm not a programmer, but accustomed to doing code review as a systems engineer. > ...
8 years, 6 months ago (2010-12-29 02:25:44 UTC) #2
Carl
On 2010/12/29 02:25:44, Keith wrote: > I'm not a programmer, but accustomed to doing code ...
8 years, 6 months ago (2010-12-29 03:37:12 UTC) #3
Carl
Thanks again for the review. Here are my responses to your comments. Thanks, Carl http://codereview.appspot.com/3842041/diff/6001/Documentation/notation/fretted-strings.itely ...
8 years, 6 months ago (2010-12-29 03:50:23 UTC) #4
Keith
Everything I know how to check looks good. http://codereview.appspot.com/3842041/diff/6001/Documentation/notation/fretted-strings.itely File Documentation/notation/fretted-strings.itely (right): http://codereview.appspot.com/3842041/diff/6001/Documentation/notation/fretted-strings.itely#newcode477 Documentation/notation/fretted-strings.itely:477: A ...
8 years, 6 months ago (2010-12-29 05:18:07 UTC) #5
pkx166h
Just a couple of Doc-policy things http://codereview.appspot.com/3842041/diff/6001/Documentation/notation/fretted-strings.itely File Documentation/notation/fretted-strings.itely (right): http://codereview.appspot.com/3842041/diff/6001/Documentation/notation/fretted-strings.itely#newcode484 Documentation/notation/fretted-strings.itely:484: A string pitch ...
8 years, 6 months ago (2010-12-29 09:13:24 UTC) #6
marc
Am 29.12.2010 06:18, schrieb k-ohara5a5a@oco.net: > [...] > The order for the chord entry was ...
8 years, 5 months ago (2010-12-29 18:22:56 UTC) #7
marc
Am 29.12.2010 19:22, schrieb Marc Hohl: > [...] > But for duitar, I meant "guitar", ...
8 years, 5 months ago (2010-12-29 18:25:38 UTC) #8
James.Lowe_datacore.com
________________________________ From: lilypond-devel-bounces+james.lowe=datacore.com@gnu.org [lilypond-devel-bounces+james.lowe=datacore.com@gnu.org] on behalf of Marc Hohl [marc@hohlart.de] Sent: 29 December 2010 18:25 ...
8 years, 5 months ago (2010-12-30 11:42:10 UTC) #9
cpkc_shaw.ca
On Thu, 2010-12-30 at 11:42 +0000, James Lowe wrote: > > ________________________________ > From: lilypond-devel-bounces+james.lowe=datacore.com@gnu.org ...
8 years, 5 months ago (2010-12-30 13:21:23 UTC) #10
Carl
On 2010/12/29 05:18:07, Keith wrote: > Agreed. My earlier 'arbitrary' was a mental slip. I ...
8 years, 5 months ago (2010-12-31 14:52:36 UTC) #11
Carl
On 2010/12/29 05:18:07, Keith wrote: > ly/string-tunings-init.ly:43: (make-music 'SequentialMusic 'void #t))) > > We need ...
8 years, 5 months ago (2010-12-31 14:54:31 UTC) #12
Carl
I've responded to all the commandments and put up a new patch. Thanks for all ...
8 years, 5 months ago (2011-01-01 05:04:55 UTC) #13
pkx166h
8 years, 5 months ago (2011-01-03 16:36:11 UTC) #14
For the Doc/Notation/fretted-strings.itely - that looks good.

I can't comment on the rest.
Sign in to reply to this message.

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