Re: [PATCH] Optionally add text to metronome marks (i.e. allow \tempo "Allegro" 4=120)

[Reinhold Kainhofer]

Am Sonntag, 22. Juni 2008 schrieb Han-Wen Nienhuys:
> On Thu, Jun 19, 2008 at 3:16 PM, Reinhold Kainhofer
>
> <address@hidden> wrote:
> >> Oops, you really got me there. If there is a text, I set the tempoText
> >> property, but I forgot that if that property is already set, I need to
> >> unset it there... Fixed with the attached patch.
> >
> > Since that last mail didn't get any responses, I'll ask again: Is the
> > patch now okay to be pushed to master?
>
> sorry for the delay, feel free to apply after taking these comments
> into consideration.

Okay, will commit it.

> > One thing that I don't like about it is that in the parser I haven't
> > found a way to match a string or markup, so I had to duplicate the cases:
> >
> > +       | TEMPO string steno_duration '=' bare_unsigned {
> > +               $$ = MAKE_SYNTAX ("tempo", @$, make_simple_markup($2),
> > $3, scm_int2num ($5));
> > +       }
> > +       | TEMPO full_markup steno_duration '=' bare_unsigned    {
> > +               $$ = MAKE_SYNTAX ("tempo", @$, $2, $3, scm_int2num ($5));
> > +       }
> >
> > Did I miss some pattern that matches a string and a full markup (and can
> > be used equivalently in the tempo and tempoText calls)?
>
> No. Please add a rule.
>
> Thinking about this a little more... I think the formatting support
> for this is definitely the worth the effort, but would it not be a
> better idea to use a standard music function/properties-sets on the
> input side?  How big is the necessity for supporting 4 different
> variants of input directly in the parser ?

For tempo marks, I think they should be handled by the parser, because 
otherwise the syntax duration=count is not possible.

> +    (set! props (append props (list
> +            (if text (make-property-set 'tempoText text)
> +                     (make-property-unset 'tempoText)))))
>
> it usually is shorter and more efficient to prepend (ie. to cons) the
> extra elements onto the front of the list

Okay, will do.

> +      )
> +    )
> +  )
> +)
>
> don't put a single paren on a line (ie. move to previous line)

For developing, I put them on a separate line, because then it's easier to see 
where I need to insert a command. I simply forgot to put them on the previous 
line before comitting...

> +          (make-concat-markup (list (make-simple-markup "(")
> +                                    note-markup
> +                                    (make-simple-markup ")")))))
>
> that surprises me, I'd think you have to switch off spaces, otherwise you
> get
>
>   (<SPACE>NOTE = xx <SPACE>)
>
> did I miss the part that sets wordspace to 0 

Yes, make-concat-markup ;-)
Of course, I had to insert " " before and after the "="...

Cheers,
Reinhold

-- 
------------------------------------------------------------------
Reinhold Kainhofer, Vienna University of Technology, Austria
email: address@hidden, http://reinhold.kainhofer.com/
 * Financial and Actuarial Mathematics, TU Wien, http://www.fam.tuwien.ac.at/
 * K Desktop Environment, http://www.kde.org, KOrganizer maintainer
 * Chorvereinigung "Jung-Wien", http://www.jung-wien.at/