[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#419947: [newt] Characters in titlebar are sometimes unbalanced



On Sun, May 06, 2007 at 06:58:26PM +0200, Frans Pop wrote:
> On Thursday 19 April 2007 00:44, Davide Viti wrote:
> > The attached patch makes sure characters surrounding the title are
> > always even (see also #416543).
> 
> This is really only a very cosmetic issue.

indeed
 
> I have some reservations about this patch.
> - Is it really correct to _subtract_ a position to balance the border?
>   I'd expect you would _add_ a position to make sure that you never have
>   to few positions.

I did some more testing and your assumption is true with boolean questions
(with multiselect there are no problems at all):

  ┌───┤ [!!] [10......] ├───┐
  │                         │
  │ Template di tipo        │
  │ stringa                 │
  │                         │
  │                         │
  │ ________________________│
  │                         │
  │        <Continue>       │
  │                         │
  └─────────────────────────┘

as you can see, in some situations (depending on the length of the title and 
on the priority of the question), the textboxis too close to the border. OTOH,
adding one character rather than subtractin it, will lead to another "unbalanced"
situation which needs to be fixed somewhere else:

  ┌────┤ [!!] [10......] ├────┐
  │                           │
  │ Template di tipo          │
  │ stringa                   │
  │                           │
  │                           │
  │ ________________________  │
  │                           │
  │        <Continue>         │
  │                           │
  └───────────────────────────┘





> - Should not first a "cleaner" solution be implemented for the fact that
>   the width of the sigils varies? I think subtracting a position currently
>   only "works" because most times the sigil is not 2 positions while we do
>   now reserve 2 positions for it. This function would be the correct place
>   to implement that.

there's quite a lot of code duplication in newt.c when it comes to computing 
window sizes. sigill length is not the only variable: title length must be taken
into account as well (and there are various subcases depending on the longest
of the lengths of the strings inside the window)

> - The patch makes the newtCenteredWindow calls fairly complex. It would
>   possibly be better to recalculate the width by calling a separate
>   function.

it can be achieved using a local variable which can be computed on a separate line
with something like this:

Index: modules/frontend/newt/newt.c
===================================================================
--- modules/frontend/newt/newt.c        (revision 46518)
+++ modules/frontend/newt/newt.c        (working copy)
@@ -173,6 +173,7 @@
     };
     char *buf = NULL;
     int i = -1;
+    int pad = 0;

     if (priority != NULL) {
         for (i = 0; sigils[i][0] != NULL; i++) {
@@ -184,10 +185,14 @@
                 buf = NULL;
     }
     if (buf != NULL) {
-        newtCenteredWindow(width, height, buf);
+        if ( (width + strlen(buf))%2 )
+         pad = 1 ;
+       newtCenteredWindow(width + pad, height, buf);
         free(buf);
     } else {
-        newtCenteredWindow(width, height, title);
+        if ( (width + strlen(title))%2 )
+         pad = 1 ;
+       newtCenteredWindow(width + pad, height, title);
     }
 }


regards,
Davide

Attachment: signature.asc
Description: Digital signature


Reply to: