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

Re: Re: Help on development of a gtk frontend for the debian-installer

On Fri, Apr 01, 2005 at 11:46:59PM +0000, attilio wrote:
> today i managed to resolve the bugs that previously would not alow the 
> gtk fe nor the back/forward mechanism to work correctly (actually only 
> select, boolean and string questions work correctly).
> I've slightly modified the gtk_go() function with ("//atti" is where 
> i've added/changed something)

I second Joey's request for your changes as a diff rather than a
cut-and-paste of the new code. The current approach is really hard to
deal with.

Also, it's best to avoid putting your name against changes to code,
unless it's a TODO comment about which somebody might need to contact
you. Imagine how the current code would look if it had "joeyh", "tausq",
"mbc", "cjwatson", etc. scattered all over it. A nightmare. This is what
revision control is for. :-)

And, as a few side points:

  * While // comments are technically legal in C as of C99, many people
    still consider them bad style outside C++ ...

  * Variable names should be based on English, please.

  * Please try to avoid mixing code changes with whitespace changes.

Anyway, on to the code ...

> static int gtk_go(struct frontend *obj)
> {
>     struct frontend_data *data = (struct frontend_data *) obj->data;
>     struct question *q = obj->questions;
>     struct question *qaux; // atti+ added as an auxiliary pointer to an 
> element of the question list
>     int i,j, num_domande=0;
>     int ret;
>     GtkWidget *questionbox;
>     if (q == NULL) return DC_OK;
>     data->setters = NULL;
>     questionbox = gtk_vbox_new(FALSE, 5);
>     gtk_box_pack_start(GTK_BOX(data->target_box),
>                        questionbox, FALSE, FALSE, 5);
>     /* FIXME: no more description frame
>        if (strcmp(q->template->type, "note") != 0 )
>        gtk_label_set_text(GTK_LABEL(data->description_label),
>        q_get_extended_description(q));
>     */
>     qaux=q;//+atti num_domande will take count of the question list's 
> lenght, while qaux will be a ptr to the last question in the list: it 
> will be useful if the user presses the "back" button
>     num_domande=1;
>     while(qaux->next!=NULL)
>     	{
>     	num_domande++;
>     	qaux=qaux->next;
>     	}
>     syslog(LOG_DEBUG,"atti - gtk_go() called, question list's lenght: 
> %d ",num_domande);
> 	//if(num_domande>1) q=q->next; //atti + maybe the first question, 
> 	which is always debian-installer/mainmenu should not be shown outside the 
> main-menu?
> 	j=0;
>     while (q != 0)
>     	{
>     	j++;
>     	syslog(LOG_DEBUG,"  atti - question %d: %s",j,q->tag);
>         for (i = 0; i < DIM(question_handlers); i++)
>             if (strcmp(q->template->type, question_handlers[i].type) == 0)
>             {
>                 ret = question_handlers[i].handler(obj, q, questionbox);
>                 if (ret != DC_OK)
>                 	{
>                     return ret;
>                 	}
>             }
>         q->prev=NULL;	//atti+ without this the ASSERT() in frontend.c, 
> line 15, fails!
>         q = q->next;    	
>     	}
>     q = obj->questions;
>     gtk_window_set_title(GTK_WINDOW(data->window), obj->title);
>     gtk_widget_set_sensitive (data->button_prev, 
> obj->methods.can_go_back(obj, q));
>     gtk_widget_grab_default(data->button_next);
>     gtk_widget_show_all(data->window);
>     gtk_main();
>     if (data->button_val == DC_OK)
> 	    {
> 		call_setters(obj);
> 		q = obj->questions;
> 		while (q != NULL)	
> 			{
> 		    obj->qdb->methods.set(obj->qdb, q);
> 		    q = q->next;
> 			}
> 		q = obj->questions;
>    		}
> 	else if (data->button_val == DC_GOBACK && q->prev != NULL ) 	 
> 	    {
> 	    q = qaux;
> 		q = qaux->prev;
> 	    }
> 	q->next=NULL;
>     gtk_widget_destroy(questionbox);
>     /* FIXME
>       gtk_label_set_text(GTK_LABEL(data->description_label), "");
>     */
>     return DC_OK;
> }
> as you can see there are some modifications to the original code, they are:
> -after displaying a question (after calling the appropriate gtk_handler) 
> i set q->prev to NULL: this way the fe no longer crashes if the same 
> configuration module is called more than one time in sequence (example: 
> i call "kbd chooser" from the main menu two times in sequence).
> -I set q->next=NULL , where q is the first question of the list, before 
> returning from gtk_go(): this prevents the fe from crashing if the user 
> presses the forward button.
> -other modifications where done in the    "if (data->button_val == 
> DC_OK) {...} else if (data->button_val == DC_GOBACK && q->prev != NULL ) 
> {...}"
> to make the back mechanism work.

Well, I can see what you're trying to do, but I don't think this
approach is right. gtk_go() is just supposed to run through the list of
pending questions and set up handlers for all of them (so it displays
all pending questions on the same screen, which admittedly may not be
optimal in future), and then gtk_main() handles the back/forward
buttons. After the frontend's go() method returns, common code in
frontend.c (frontend_clear()) is supposed to deal with clearing up the
list of pending questions.

You can tell that it's unnecessary for a frontend to clear up the list
of pending questions itself by noting that newt_go() does not do so. As
far as I know, individual frontends should treat 'struct question'
objects passed to them as read-only, and I think I'm going to enforce
that by adding 'const' if I can.

It looks to me as if somehow frontend_clear() isn't getting called, or
the pending question list is ending up in a strange state in some other
way (perhaps due to memory corruption). However, unless the gtk frontend
is responsible for memory corruption, it shouldn't be up to the gtk
frontend to deal with keeping the pending question list in a sane state.

> if you wanna try the frontend just get the gtk.c frov svn and replace 
> the gtk_go() with this one; i've tested it on a rc2 debian-installer 
> (soon on a rc3 too) and i'm using a direct frame buffer graphic system, 
> against wich i compile gtk.c .
> i've also packed in a tar archive a direct framebuffer environement that 
> i use to test the gtk fe with a debian installer: the main problem with 
> it is that the only gtk2 library compiled against dfb is about 11 meg: i 
> think that a minimal version of that should be crated.

Personally, and after talking to some GTK hackers, my inclination is
increasingly to avoid directfb altogether and use a plain framebuffer,
although this does involve fixing GTK's linux-fb backend so that it
actually works.


Colin Watson                                       [cjwatson@debian.org]

Reply to: