Re: Adding note color handling to musicxml2ly

[Reinhold Kainhofer]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am Dienstag, 14. Juli 2009 18:07:10 schrieb Carl Sorensen:
> On 7/13/09 1:35 AM, "Bret Aarden" <address@hidden> wrote:
> > There is no doubt a much more elegant way to do this, but the included
> > patch works for me, [...]
>
> I saw that you had provision in print_note_color for reverting color.  But
> I never saw any code elsewhere that called print_note_color with no rgb
> value to call that reversion.
>
> I would recommend that you consider replacing \override with \once
> \override, and junk the \revert.  

I was about to suggest the same. The \revert is never inserted. But then, you 
don't want an \override anyway, because the setting should only apply to that 
one note, which has the color attribute, not to all following ones. So, you 
really want \once\override.

To make sure things really work, you should create a regression test file 
input/regression/musicxml/59a-NoteColor.xml (see 
http://kainhofer.com/~lilypond/input/regression/musicxml/collated-files.html 
for the full regression test suite). That regression test should have:
- -) a colored note (at the very beginning, so you see if it works right at the 
first note, too)
- -) a following differently colored note
- -) a following colored note with the same color
- -) a following rest (should probably not be colored)
- -) a colored note
- -) a non-colored note.
- -) A colored note with some articulation and e.g. a non-default notehead 
(which uses an \override, too, so you check whether the combination of 
multiple settings for one note doesn't break)

I would suggest this order, because then you have all different types of 
sequences (two differently colored notes; rests after/before notes; uncolored 
notes). if rests can be colored, too, you should probably also add some 
combination of colored/uncolored rests and notes.

For some more comments on the patch, see below:

Am Dienstag, 14. Juli 2009 01:41:34 schrieb Bret Aarden:
> +    def print_note_color (self, object, rgb=None):
> +        if rgb:
> +            str = ("\override %s #'color = #(rgb-color %s %s %s)" %
> +                   (object, rgb[0], rgb[1], rgb[2]))

I suppose this should be \\override (i.e. escape the backslash).

> +        else:
> +            str = "\\revert %s #'color" % object
> +        self.newline()
> +        self.add_word(str)
> +        self.newline()

I'm not sure that you really want TWO newlines. Won't this insert a blank line 
between two color directives in the lilypond code?
I'm not even sure that I want a newline before/after the override at all. This 
would result in each note of a piece taking ~4 lines in the lilypond file!
At least with almost all other note-attached overrides that I implemented, I 
don't write out any newlines.

The rest looks okay.

Cheers,
Reinhold
- -- 
- ------------------------------------------------------------------
Reinhold Kainhofer, address@hidden, http://reinhold.kainhofer.com/
 * Financial & Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iD8DBQFKXLZrTqjEwhXvPN0RAlhsAJ4wWhdfKq+YdBEct6Zf28wKFl8CZwCfaLVg
nLW9gtw5DHxhlLtH9qPJjGA=
=6ESx
-----END PGP SIGNATURE-----