RSS/Atom feed Twitter
Site is read-only, email is disabled

[PATCH] Gradient-Fu Rev. 2 [was Re: [PATCH] Start of aGradient Scripting Support]

This discussion is connected to the gimp-developer-list.gnome.org mailing list which is provided by the GIMP developers and not related to gimpusers.com.

This is a read-only list on gimpusers.com so this discussion thread is read-only, too.

2 of 4 messages available
Toggle history

Please log in to manage your subscriptions.

Pine.LNX.4.56.0312172238060... Shlomi Fish 17 Dec 21:42
  Pine.LNX.4.56.0312181657520... Shlomi Fish 18 Dec 15:59
   [PATCH] Gradient-Fu Rev. 2 [was Re: [PATCH] Start of a Gradient Scripting Support] Sven Neumann 18 Dec 17:51
    [PATCH] Gradient-Fu Rev. 2 [was Re: [PATCH] Start of aGradient Scripting Support] Shlomi Fish 18 Dec 18:58
Sven Neumann
2003-12-18 17:51:54 UTC (over 20 years ago)

[PATCH] Gradient-Fu Rev. 2 [was Re: [PATCH] Start of a Gradient Scripting Support]

Hi,

Shlomi Fish writes:

Well, here is the up-to-date patch according to the commentary from the core developers that I received on the IRC.

Please see my comments on the patch and put a revised version to Bugzilla.

+{
+ GimpGradientSegment *seg;
+ int i;
+
+ if (i < 0)
+ {
+ return NULL;
+ }

I think you want to check index here, not the uninitialized variable i. Better make this a g_return_val_if_fail() though. In general you should add such checks for all assumptions you make about function parameters.

+void gimp_gradient_get_segment_left_color (GimpGradient *gradient, + GimpGradientSegment *seg, + GimpRGB * color)

Please try to follow the coding style and align function parameters the same way all other GIMP functions do it. The same comment applies to the declarations in the header file.

+ gimp_data_dirty (GIMP_DATA(gradient));

Please insert a space before the opening bracket.

Sven

Shlomi Fish
2003-12-18 18:58:37 UTC (over 20 years ago)

[PATCH] Gradient-Fu Rev. 2 [was Re: [PATCH] Start of aGradient Scripting Support]

On Thu, 18 Dec 2003, Sven Neumann wrote:

Hi,

Shlomi Fish writes:

Well, here is the up-to-date patch according to the commentary from the core developers that I received on the IRC.

Please see my comments on the patch and put a revised version to Bugzilla.

+{
+ GimpGradientSegment *seg;
+ int i;
+
+ if (i < 0)
+ {
+ return NULL;
+ }

I think you want to check index here, not the uninitialized variable i. Better make this a g_return_val_if_fail() though. In general you should add such checks for all assumptions you make about function parameters.

You are right - it was a bug. Fixed.

+void gimp_gradient_get_segment_left_color (GimpGradient *gradient, + GimpGradientSegment *seg, + GimpRGB * color)

Please try to follow the coding style and align function parameters the same way all other GIMP functions do it. The same comment applies to the declarations in the header file.

Done.

+ gimp_data_dirty (GIMP_DATA(gradient));

Please insert a space before the opening bracket.

OK.

This fixes will be incorporated in the next version of the patch, which will also have position set/get routines.

Regards,

Shlomi Fish

---------------------------------------------------------------------- Shlomi Fish shlomif@vipe.technion.ac.il Home Page: http://t2.technion.ac.il/~shlomif/

An apple a day will keep a doctor away. Two apples a day will keep two doctors away.

Falk Fish