Re: [PATCH]: -- Revised tablature implementation

[Carl D. Sorensen]

On 6/10/09 8:07 AM, "Carl D. Sorensen" <address@hidden> wrote:

> Please review Mark Hohl's patch for improved tablature.
> 
> It's available at
> 
> <http://codereview.appspot.com/67174>
> 

Marc,

I have a few comments on the patch:

1) I think you should create a new file: scm/tablature.scm.

This file would contain

x-tab-format

tab-note-head::whiteout-if-style-set

clef::print-modern-tab-if-set

glissando::calc-tab-extra-dy

parenthesize-fret-nr

parentheses-item::calc-tabstaff-parenthesis-stencils

tie::handle-tab-tie

repeat-tie::parenthesize-tab

It could also contain all of the tunings definitions moved from
scm/output-lib.scm.  I've always thought that ouitput-lib.scm was a funny
place for tunings, but I didn't have a better place to put them.  I think we
do now.

2) I think that some of the functions listed above should be renamed, so
they start with tablature:: or tablature-item::.  This kind of name would
make it more obvious that you should look in tablature.scm for those
functions.  So instead of clef::print-modern-tab-if-set, it could be
something like  tablature::print-desired-tab-clef.  I realize that this
differs from the standard grob-name:: naming convention; but I think it may
be the right thing to do here.

As an alternative to the renaming proposed above, the functions should be
placed in appropriate .scm files related to the the grobs, rather than
related to the tablature, it seems to me.

But I'm not an expert on this, which is why I waited to post to the -devel
list for this comment, rather than giving it to you directly.

3) parenthesize-fret-nr should probably be renamed; "nr" is an abbreviation.
parenthesize-fret-number or parenthesize-fret seem to be better names to me.
Actually, I think I most prefer parenthesize-tab-note-head

4) I think you should put empty lines between all of your functions (e.g.
before deadNotesOff, before the comments for palmMUte, before palmMuteOff,
etc.


Thanks,

Carl