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

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

Colin Watson ha scritto:

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 ...




Sorry again for the bad coding style..


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.
I think you're right, gtk_go() should not modify the list of question as i do but, as i said before, that was just a workaround to get rid of crashes and to be able to experiment with the frontend. 
I would be happy to remove those lines of code, too..



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.
I've tried to track down any possible modification done by the frontend to the question list but i was unable to detect them, if any
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.

Cheers,
Please, could you explain better the last point? what do you mean? that a gtk frontend for the debian installer can be build without directfb? how to do it?
Reply to: