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

Issue 67174: First release of ly/tablature.ly (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 10 months ago by Carl
Modified:
14 years, 5 months ago
Reviewers:
joeneeman
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

First release of ly/tablature.ly Additional tunings in scm/output-lib.scm (additional tunings) Additional function in scm/parser-clef.scm to add clefs at runtime

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+295 lines, -5 lines) Patch
A ly/tablature.ly View 1 chunk +278 lines, -0 lines 2 comments Download
M scm/output-lib.scm View 3 chunks +9 lines, -5 lines 0 comments Download
M scm/parser-clef.scm View 1 chunk +8 lines, -0 lines 0 comments Download

Messages

Total messages: 1
joeneeman
14 years, 10 months ago (2009-06-23 18:31:53 UTC) #1
I don't know much about the issues here, but here's a review, fwiw. Of course,
you'll need to update the docs also (and a regression test or 2 would be nice).

http://codereview.appspot.com/67174/diff/1/2
File ly/tablature.ly (right):

http://codereview.appspot.com/67174/diff/1/2#newcode205
Line 205: tabFullNotation = {
Why not create a new context, FullTabVoice, or something like that? Do you
expect people to use \tabFullNotation mid-piece?

http://codereview.appspot.com/67174/diff/1/2#newcode237
Line 237: \layout {
It seems to me that we have 3 versions of tablature now: the "default" version
in engraver-init.ly, the version that appears if the user includes
"tablature.ly" and the "full" version that the user gets by including
tablature.ly and using the tabFullNotation. Is there a reason for keeping the
old versions in engraver-init.ly? Perhaps we should scrap them altogether and
require that the user include tablature.ly in order to get tabs.
Sign in to reply to this message.

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