Re: PATCH: Improved tablature support

[Carl Sorensen]

On 7/16/09 9:56 AM, "Carl Sorensen" <address@hidden> wrote:

> Marc Hohl has completed a patch for improved tablature support.  It is
> available for review at:
> 
> http://codereview.appspot.com/95059
> 
> Please review and comment.
> 
> Thanks,

Marc,

Here are some comments.

1) All regression tests should be \version "2.13.4" -- I'll fix that.

2) Your regression test for the modernTab clef:
  Doc header should say "four- to seven- stringed instruments", which means
instruments having four to seven strings.  "four to seven stringed
instruments" means four to seven instruments having strings.  Isn't english
fun?
  You should demonstrate in the regtest that it works for four strings and
seven strings, and that it works with various staff sizes.  You may want two
different regtests -- one that shows variations in string numbers and one
that shows variation in sizes.

3) Is it necessary (or desirable) to have both \deadNote and
\deadNoteOn..\deadNoteOff?  Unless a whole piece is to be written in
DeadNotes, I can't imagine that it's better to write

\deadNoteOn e e deadNoteOff

than

\deadNotes{e e}.

If there's no value to the On..Off pairs, then it's probably best to
eliminate them.  But if there is value, we can keep them.  And I'm not the
judge of value.  I'm just asking the question.


4) Formatting nit on scm/tablature.scm

line 125-128: I think it would look better to put all the arguments to
ly:stencil-combine-at-edge on line 124.

line 129-131:  I would like it better to have the last three arguments to
the first ly:stencil-combine-at-edge call on line 125, nested to the same
depth as the inner (ly:stencil-combine-at-edge call.

Other than that, things look good.

Oh, BTW, Thomas Morgan is currently working on a new Parenthesize markup
command that will be better than the current parentheses; it will draw the
parentheses as slurs.  This may be something you want to use in the future.

> 
> Carl
> 
> 
> 
>