Re: GtkSpinButton Patch

[Nils Barth <nbarth fas harvard edu>]

Thus spake James M. Cape:
> Donald Ryan Willhoit wrote:
> > I sent this a while ago, but it probably got lost.  Anyway it makes the
> > SpinButton control's size request seem more reasonable.  Try it out - it
> > should insure that the largest and smallest numbers the spin button can
> > have always fit in the spin button.  This would be really important if
> > someone changes the font size.
> > 
> > If this is better behaviour and reasonable for inclusion in say 1.4, I'll
> > also write the support for GtkText, GtkEntry, GtkLabel, GtkCombo and
> > whatever else I can find that is broken in this respect.
> > 
> > Ryan Willhoit
> > willhoit@andrew.cmu.edu
> 
> You might be interested to know that in the proportional width fonts I
> have looked at (Helvetica being the most usefull in this case), four is
> the widest number, so if you use that instead of a zero, it may produce
> a more acurate size at large font sizes than a zero/eight/minus combo.

Wouldn't it simply be best to take the maximum width of all numerical
characters in the font, in case of weird fonts?
It's not that hard, is it?

That is, set the width of the box to be:
max_number_of_digits*max(width(0..9))
+ (if applicable) width(decimal delimiter) + (if applicable) width("-")

That is, could the patch change to read:
(near the top)
+  char i[2];
+  i[1]=0;

Take out the lines:
-  if (spin->adjustment->lower < 0) 
-    count += 1;
-  if (spin->digits > 0) 
-    count += (spin->digits + 1);
(dealt with below)

later:
+  temp_width = 0;
+  /* temp_width gets the maximum width of a digit */
+  for (i[0]='0'; i[0]<='9'; ++(i[0]))
+   temp_width = MAX(temp_width, gdk_text_width (widget->style->font, i, 1));
+  temp_width *= count;
+  /* If the value can be negative, add space for a negative sign */
+  if (spin->adjustment->lower < 0) 
+    temp_width += gdk_text_width (widget->style->font, "-", 1);
+  /* If it has decimal digits, add space for a decimal separator */
+  if (spin->digits > 0) 
+    temp_width += gdk_text_width (widget->style->font, ".", 1);
+
+  width = ceil( temp_width );

(sorry for the inline patch fragment -- is this poor form?)

Of course, one would change 'i' to a more meaningful name, and this
otherwise has two problems:
* It assumes that the decimal separator is a . which is wrong for many
  countries; is there a way to figure out what it is? If not, we could
  just loop through common separators . , etc.
* Does GTK always render a preceding - for a negative value? That is,
  does it allow instead a (2) for -2, or a red 2 or whatever (though
  these are ugly).

Are there any further i18n questions? Do we only want to render Roman
numerals?

Side note: kernel-traffic's latest issue had a note on a possible
problem with a simple MAX a la GLib, namely that it evaluates
functions twice, which can be bad for some functions (say, if they
return different values on repeated calls). Someone suggested
replacing it with a dummy variable assignment, then
comparing/assigning with those variables. This might also be a bit
faster...
The exact code was (for MIN):
#define min(x, y) ({typeof (x) x_ = (x); \
                    typeof (y) y_ = (y); \
                    x_ < y_ ? x_ : y_;})

Message reference in which this is discussed further:
http://kernelnotes.org/lnxlists/linux-kernel/lk_0004_03/msg00537.html

If this is changed, we'd also likely want to change MIN ABS and CLAMP

-- 
  -nils
Public key: http://www.fas.harvard.edu/~nbarth/pub-key.txt

PGP signature