On Sun, Feb 02, 2003 at 01:55:37AM +0100, Sven Neumann wrote: > > > * set alpha of all colors to 1.0 -- ifscompose ignores alpha anyway, > > and default value (122???) makes the color selection dialog pretty > > confusing > > The color selection dialog shouldn't show the alpha value. If it does, > the color buttons are set up incorrectly. Make sure they are created > with a type of GIMP_COLOR_AREA_FLAT. > > ...(later) > actually the bug was in GimpColorButton itself. It used to enable > opacity control in the GTK+ color selection dialog unconditonally. > This is now fixed in CVS. So I removed the alpha=1.0 settings. > I'd appreciate if you could try to follow the GIMP coding style which > asks for a space before the opening bracket of a function call. Well, OK. I'll restrain commenting this particular coding style requirement, because I'd have to be vulgar. I'll never learn to type them when coding, so I always have to add them later. So I added them with search and replace, and there were maybe more places in the original code lacking the spaces than in my (the original often didn't even have spaces after commas, I fixed some before and the remaining now). But, well, OK. > yeti_message_dialog() is a rather unintuitive name for a function that > is supposed to create a modal dialog. > Why do you use a modal dialog here? We usually don't use modal dialogs > in gimp. You should rather make the message dialog transient for the > file selection dialog and set destroy_with_parent on it. It was just copied from some other code. I changed yeti_ to ifscompose_, fixed problems when filename contained %, removed the modality and also parent destruction. After all, it makes more sense to try another file name when one fails. > Please, don't ever use strcpy(). It's dangerous and should never be used. It's funny, the strcpy () got there from the Qbist plugin. I looked there for simple file save/load code. So Qbist should be probably fixed too. > This code looks like a reimplementation of g_file_get_contents(). You > might want to use the latter instead. Unfortunately, I wasn't aware of g_file_get_contents (). It uses it now. With all the space-after-something stuff the patch has grown over 45k, so I'm not posting it by email. It's here: http://physics.muni.cz/~yeti/download/tmp/ifscompose-1.3.11.patch There's a one more issue I'd like to raise: the design area is still too tiny. While I'd prefer DESIGN_AREA_MAX_SIZE about 400 (now it's 256), I understand it wouldn't fit onto 800x600 screen. But even increasing the size to 320 or 300 would help considerably. Thanks for the comments. Regards, Yeti
