--- bulia byak <archiver_1@...19...> wrote:

I have applied and committed your patch after some cleanup. Comments:

• Please get yourself an editor that can do auto

indent. Now you seem to indent manually which is a pain both for you and for those who read your code :)

I'll look into it.

• Always generate diffs ignoring whitespace

differences (diff -b). That would make your patch much smaller and easier to read.

yeah, I need to find where tortoiseCVS does the settings for the right click menu generate patch option.

• It's "Pattern fill", not just "Bitmap fill"

Tis now, my original patch just did bitmaps tho :)

• No need to set "sodipodi:fill-pattern". You seem

to have copied it from "sodipodi:fill-cmyk" but that is entirely different story (and we plan to eliminate it btw).

wasnt sure what it did or if it was needed, erred on the side of caution

• Be more defensive in coding. You write e.g.

  pat = SP_PATTERN(g_object_get_data

(G_OBJECT(gtk_menu_get_active (GTK_MENU(gtk_option_menu_get_menu (GTK_OPTION_MENU(patmnu))))), "pattern"));

Not only is this ugly, but it's very easy to break. Instead, separate it into sequential assignments and do proper checks between them ("Did I get what I think I get?" "Is there an active item?" "Is this really a pattern?" etc). For example now your code will break if I add a separator to the menu (which is an item but has no pattern data).

yeah, sorry, thats ugly and lazy. Too used to writing code which no one else ever sees or edits. Will try and be more concious of this sort of thing. (comes from being self taught too, no one ever corrects you :) )

• Eventually it would be the best to treat patterns

the same as markers. That is, add previews to the menu, and add a patterns.svg document that would contain some nice predefined patterns for users. That should be easy because we already have all the code in place. However this has to wait until we sort out the speed problem with marker previews first.

this was the plan, as i put pattern support in get_stock when i wrote it. Again could do with a "table of previews" type widget tho, should look into if the GIMP or similar has something we can lift.

__________________________________ Do you Yahoo!? Friends. Fun. Try the all-new Yahoo! Messenger. http://messenger.yahoo.com/