Change syntax for woodwind-diagram markup

Change syntax for woodwind-diagram markup

Add graphical to list of grob properties
Move size, thickness, graphic to props.
Update regression files, add convert-ly rule

[Carl, 2010-08-14 14:59:57]

Here's a patch that changes the syntax for woodwind-diagram markups so that
size, thickness, and graphic become part of the properties, rather than part of
the arguments to the markup call.

It's your code, Mike, so I don't want to push it without your permission, but
it's more consistent with our other markup code.

Thanks,

Carl

[Neil Puttock, 2010-08-14 19:47:59]

http://codereview.appspot.com/1946043/diff/1/5
File scm/define-grob-properties.scm (right):

http://codereview.appspot.com/1946043/diff/1/5#newcode414
scm/define-grob-properties.scm:414: (graphical ,boolean? "Display in graphical,
rather than textual, form.")
don't need this

http://codereview.appspot.com/1946043/diff/1/6
File scm/display-woodwind-diagrams.scm (right):

http://codereview.appspot.com/1946043/diff/1/6#newcode1870
scm/display-woodwind-diagrams.scm:1870: (thickness .1)
0.1

http://codereview.appspot.com/1946043/diff/1/6#newcode1952
scm/display-woodwind-diagrams.scm:1952: (let*  ((radius (chain-assoc-get 'size
props 1.0))
Since these are bound with defaults above, you don't need to use chain-assoc-get

(radius size)

or use the properties directly instead of rebinding

http://codereview.appspot.com/1946043/diff/1/6#newcode1954
scm/display-woodwind-diagrams.scm:1954: (display-graphic (chain-assoc-get
'graphic props #t))
graphical

http://codereview.appspot.com/1946043/diff/1/6#newcode1966
scm/display-woodwind-diagrams.scm:1966: ;(user-draw-commands (cadddr
input-list))
remove

[Carl, 2010-08-15 04:10:36]

OK, I fixed all these things, but I did have a question about why the grob
property didn't need to be defined.

Thanks,

Carl

http://codereview.appspot.com/1946043/diff/1/5
File scm/define-grob-properties.scm (right):

http://codereview.appspot.com/1946043/diff/1/5#newcode414
scm/define-grob-properties.scm:414: (graphical ,boolean? "Display in graphical,
rather than textual, form.")
On 2010/08/14 19:47:59, Neil Puttock wrote:
> don't need this

I tested, and verified that I don't need it, but why?  I thought that we had
checks to issue a warning if an undefined property was used.  Is that only for
context properties?

Done, anyway

http://codereview.appspot.com/1946043/diff/1/6
File scm/display-woodwind-diagrams.scm (right):

http://codereview.appspot.com/1946043/diff/1/6#newcode1870
scm/display-woodwind-diagrams.scm:1870: (thickness .1)
On 2010/08/14 19:47:59, Neil Puttock wrote:
> 0.1

Done.

http://codereview.appspot.com/1946043/diff/1/6#newcode1952
scm/display-woodwind-diagrams.scm:1952: (let*  ((radius (chain-assoc-get 'size
props 1.0))
On 2010/08/14 19:47:59, Neil Puttock wrote:
> Since these are bound with defaults above, you don't need to use
chain-assoc-get
> 
> (radius size)

Done.  I forgot about how nifty the new interface is that David made possible.

http://codereview.appspot.com/1946043/diff/1/6#newcode1954
scm/display-woodwind-diagrams.scm:1954: (display-graphic (chain-assoc-get
'graphic props #t))
On 2010/08/14 19:47:59, Neil Puttock wrote:
> graphical

Done.

http://codereview.appspot.com/1946043/diff/1/6#newcode1966
scm/display-woodwind-diagrams.scm:1966: ;(user-draw-commands (cadddr
input-list))
On 2010/08/14 19:47:59, Neil Puttock wrote:
> remove

Done.

[mikesol_ufl.edu, 2010-08-18 03:11:02]

Hey all,
    Looks a-ok to me: I don't have time to run the regtests now (still
traveling, no battery...), but if they don't break, feel free to push!

~Mike


On 8/15/10 12:10 AM, "Carl.D.Sorensen@gmail.com" <Carl.D.Sorensen@gmail.com>
wrote:

> OK, I fixed all these things, but I did have a question about why the
> grob property didn't need to be defined.
> 
> Thanks,
> 
> Carl
> 
> 
> 
> http://codereview.appspot.com/1946043/diff/1/5
> File scm/define-grob-properties.scm (right):
> 
> http://codereview.appspot.com/1946043/diff/1/5#newcode414
> scm/define-grob-properties.scm:414: (graphical ,boolean? "Display in
> graphical, rather than textual, form.")
> On 2010/08/14 19:47:59, Neil Puttock wrote:
>> don't need this
> 
> I tested, and verified that I don't need it, but why?  I thought that we
> had checks to issue a warning if an undefined property was used.  Is
> that only for context properties?
> 
> Done, anyway
> 
> http://codereview.appspot.com/1946043/diff/1/6
> File scm/display-woodwind-diagrams.scm (right):
> 
> http://codereview.appspot.com/1946043/diff/1/6#newcode1870
> scm/display-woodwind-diagrams.scm:1870: (thickness .1)
> On 2010/08/14 19:47:59, Neil Puttock wrote:
>> 0.1
> 
> Done.
> 
> http://codereview.appspot.com/1946043/diff/1/6#newcode1952
> scm/display-woodwind-diagrams.scm:1952: (let*  ((radius (chain-assoc-get
> 'size props 1.0))
> On 2010/08/14 19:47:59, Neil Puttock wrote:
>> Since these are bound with defaults above, you don't need to use
> chain-assoc-get
> 
>> (radius size)
> 
> Done.  I forgot about how nifty the new interface is that David made
> possible.
> 
> http://codereview.appspot.com/1946043/diff/1/6#newcode1954
> scm/display-woodwind-diagrams.scm:1954: (display-graphic
> (chain-assoc-get 'graphic props #t))
> On 2010/08/14 19:47:59, Neil Puttock wrote:
>> graphical
> 
> Done.
> 
> http://codereview.appspot.com/1946043/diff/1/6#newcode1966
> scm/display-woodwind-diagrams.scm:1966: ;(user-draw-commands (cadddr
> input-list))
> On 2010/08/14 19:47:59, Neil Puttock wrote:
>> remove
> 
> Done.
> 
> http://codereview.appspot.com/1946043/show
> 
> _______________________________________________
> lilypond-devel mailing list
> lilypond-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/lilypond-devel
> 

[c_sorensen, 2010-08-18 03:13:11]

On 8/17/10 9:15 PM, "Mike Solomon" <mikesol@ufl.edu> wrote:

> Hey all,
>     Looks a-ok to me: I don't have time to run the regtests now (still
> traveling, no battery...), but if they don't break, feel free to push!

regtests are fine, so I'll go ahead and push, and then finish the
documentation.

Thanks,

Carl

[Neil Puttock, 2010-08-18 21:46:02]

On 2010/08/15 04:10:36, Carl wrote:

> I tested, and verified that I don't need it, but why?  I thought that we had
> checks to issue a warning if an undefined property was used.  Is that only for
> context properties?

It depends on whether you think users are likely to do this (where you'd also
have to `graphical' to an interface),

\override TextScript #'graphical = ##t
\override TextScript #'text = \markup \woodwind-diagram ...

rather than this:

\markup \override #'(graphical . #t) \woodwind-diagram ...

There's no type-checking for markup props, but you can sneakily use them in grob
overrides so they leak into markup, even if LilyPond protests.


Cheers,
Neil

[Carl, 2010-08-18 23:32:41]

On 2010/08/18 21:46:02, Neil Puttock wrote:
> On 2010/08/15 04:10:36, Carl wrote:
> 
> > I tested, and verified that I don't need it, but why?  I thought that we had
> > checks to issue a warning if an undefined property was used.  Is that only
for
> > context properties?
> 
> It depends on whether you think users are likely to do this (where you'd also
> have to `graphical' to an interface),
> 
> \override TextScript #'graphical = ##t
> \override TextScript #'text = \markup \woodwind-diagram ...

I don't think the override of #'text is necessary; \markup creates TextScript
grobs, IIUC.

I deliberately want to be able to override the TextScript property, because I
want to
be able to set the display properties for all of the  diagrams in a piece (or
perhaps in
a particular context), rather than having to set them diagram by diagram.

So I might want to do something like:

diagram = \markup \woodwind-diagram #'oboe #'((lh . (d ees))
                                    (cc . (five3qT1q))
                                    (rh . (gis)))

<<
  \new Staff {
    \override Staff.TextScript #'size = #1.5
    c''^\diagram
  }
  \new Staff {
    c''_\diagram
  }
>>


This was the original intent of moving size, thickness, and graphical from the
argument
list to the property list.

So I guess I will need to go back and add graphical as a property, as well as
adding it to
an interface.

Thanks,

Carl



> 
> rather than this:
> 
> \markup \override #'(graphical . #t) \woodwind-diagram ...
> 
> There's no type-checking for markup props, but you can sneakily use them in
grob
> overrides so they leak into markup, even if LilyPond protests.
> 
> 
> Cheers,
> Neil

[Carl, 2010-08-18 23:51:00]

I've uploaded a new patch set with graphical back in as a property
and assigned to the instrument-specific-markup interface.