Re: Patch: Bug 50278

[Owen Taylor <otaylor redhat com>]

Mike Kestner <mkestner enteract com> writes:

> Owen Taylor wrote:
>  
> > Why don't you send another version of the patch with the doc comment
> > and the suggested changes; Havoc agrees with the above modifications,
> > and hopefully nobody else will disagree too strongly ;-)
> > 
> > I'll give that a quick look-over and then you should be able to go
> > ahead and commit it.
> 
> Here's the patch sans documentation.  I have yet to find any examples of
> what you are referring to as "doc comment".  If you mean a blurb in
> docs/reference/gtk/tmpl/*, I can add that and send it for review.  If
> you mean something like:
> 
> /**
>  * yadayada:
>  * @foo:
>  */
> 
> inline, I can whip that up as well.  Let me know.

I meant the latter. See, say, gtknotebook.c. We are slowly phasing out
the tmpl/ files since it is easier to maintain inline docs.

(Also, if you are going to be writing doc comments and use emacs, you should grab
gnome-libs/tools/gnome-doc/gnome-doc.el)

> +  if (fabs (step) >= 1.0)
> +    digits = 0;
> +  else {
> +    digits = abs ((gint) floor (log10 (fabs (step))));
> +    if (digits > 5)
> +      digits = 5;
> +  }

What if they pass in a step of 0? I suppose log10 might return +INF so
it works, but I wouldn't trust it. Also, why the digits > 5? If they
pass in a tiny step, should they get a lot of digits?

Also, indentation should be:

 else
   {
   }

Otherwise, looks good.
                                                 Owen