Re: [PATCHES]: Converting header and lyrics with musicxml2ly

[Reinhold Kainhofer]

Hi Han-Wen!

Attached are two more patches, which already address some of the points you 
mention.

-) 2007-09-01_0008-MusicXML-Conversion-of-various-types-of-spanners-o.patch  
converts some more spanners from MusicXML to lilypond. In particular, octave 
shifts, trill spanners, glissando and bends.


-) 2007-09-01_0009-MusicXML-Convert-all-clefs-escape-in-header-fiel.patch    
Convert all available clefs to lilypond. Also address the issues by Han-Wen 
mentioned below. Don't be so strict about quoting in lyrics: Only wrap the 
lyrics syllables in quotes if necessary (but then also escape " in lyrics and 
header fields!).


Am Samstag, 1. September 2007 schrieben Sie:
> if you intend to work on this more, I could give you write access to the
> repository.  It would require you to sign up for savannah.gnu.org.

Yes, I'm planning to do some more work on musicxml2ly, at least until it is in 
a state that it's easier for me to scan a score, export it to MusicXML and 
run musicxml2ly than to write it from scratch in lilypond.

My username at savannah is kainhofer.

Getting write access would be nice, but I also appreciate the feedback that I 
got here for my patches! 

> I am traveling the next 2 weeks, so I am won't have much time to look/
> I'm applying your patches. Could you address my comments in follow up
> patches?

Okay, will do.

> * In general, git commit messages should have the following form
>
>   1-line summary
>
>   background - typically the reason why you are doing it, or the
>   general idea behind it.

Okay, in KDE we don't have any such requirement

> Keep the 80 column limit in mind here. 

Okay, again, with KDE svn that limit is no issue.
 

> * You have replaced almost all dict accesses with .get() ; this is only
> desired if the default case makes sense. Sometime throwing a KeyError
> actually does make sense.
>
> For example
>
> -        (glyph, pos, c0) = self.clef_dict [self.type]
> +        (glyph, pos, c0) = self.clef_dict.get (self.type)
>
> doesn't really make sense, because the assignment will fail the default,
> None.

Ah, right, I thought I had left all [..], where an exception makes sense. Will 
revert this particular change in a later patch. This code is dead,though, 
anyway. I've redone the clef conversion code in the attached patch, so now 
almost all clefs are correctly converted from Finale 2007 to lilypond (the 
only problem I still have is the "None" key, which means that no key should 
be used. Lilypond does not seems to support this :-((( ).


> If you have to check for the default value
>
>   if key in dict:
>     ..do stuff..
>
> is clearer then
>
>   foo = dict.get(key, None)
>   if foo:
>     ...

Agreed, it's clearer, but if I need to access foo later on, the second is more 
efficient (only one lookup instead of two).

> * Also fix problem that \breathe does not accept any direction modifyer
>
> modifier

got me!

>     # return the first creator tag that has type 'editor'
> +        for i in creators:
> +            if hasattr (i, 'type') and i.type == type:
> +                return i.get_text ()
> +            else:
> +                return ''
>
> I don't understand the comment, do you mean "type" iso. editor?

This happens when you write code, then restructure and outsource some parts to 
external funcitons... The comment should rather be:
        # return the first creator tag that has the particular type



> +        if ids.get_rights ():
> +            score_information['copyright'] = ids.get_rights ()
>
> you could shorten this a little,
> removing the duplicate call of ids.get_XXX()

Done.

> +        printer.dump ('%s = "%s"' % (k, score_information[k]))
>
> this probably needs some escaping. What if there is a " in the title?

Right!

> +    for k in score_information.keys ():
>
> slightly simpler:
>
>   for (k, v) in score_information.items():
>     printer.dump ('%s = "%s"' % (k, v))

Thanks!


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/

2007-09-01_0008-MusicXML-Conversion-of-various-types-of-spanners-o.patch
Description: Text Data

2007-09-01_0009-MusicXML-Convert-all-clefs-escape-in-header-fiel.patch
Description: Text Data