Re: evil hack in buffer.c, and need for internal docs onwidgetcode

Andy> Its in my inbug box :) I'm still trying to fix the crash Andy> that occurs when you select tabs. Oh, that. At line 2744 of src/glyphs-x.c: XtSetArg (al [0], XtNtopWidget, child); child can be NULL. I don't know how it gets that way. :-) I do know that the string being passed to XtNameToWidget is the appropriate buffer name, so presumably something is wrong with the widget being passed to XtNameToWidget. When that NULL later gets passed to an Xt function, blam! As far as I can remember, the crash always occurs on an attempt to select the buffer that was originally in the leftmost position on the tabs (manager) widget. It seems likely that that label (ie, individual child tab) widget never had a next. Maybe you should use a circular list there? My initial reaction to the "callback" in buffer.c was like Ben's, but now I see what you are up to. I'm not sure it's the right way to signal the widget that its list has changed, but I do see your point that something has to do it. On the other hand, I think that using Lisp ojects in the widget's internal structure is a dubious design decision. For example, in the loop at line 2744 of src/glyphs-x.c, the fact that you are chasing two lists which are assumed to be isomorphic is concealed by the use of the LIST_LOOP macro. I'm not even sure that they should be isomorphic. (Eg, we could surely avoid this crash by using a circular list of children, which would not be isomorphic to the monotonic list of buffers. This might result in a different kind of bug, of course.) Another thing is that for a tabs widget, I don't see why any of its children ever need to be destroyed before the tabs manager itself is destroyed. Memory leak? Yes, but it's bounded. The tab widget takes up screen real estate, so (absent a mad list-creating function whose value is assigned as the label list for the tabs widget) the user won't let it grow too much. (Of course we should worry about extreme behavior; I'm talking about the ordinary case.) So I see the interface to the widget as struct tabs { LispObject current_list_of_strings; /* read only */ LispObject requested_list_of_strings; /* effectively write only */ LispObject current_top_widget; /* read only */ LispObject requested_top_widget; /* effectively write only */ LispObject current_display_parameter_list; /* faces etc, read only */ LispObject requested_display_parameter_list; /* effectively write only */ } The implementation would be a private TabsWidget object. (In practice the requested_* objects would be parts of a separate instantiator, as Ben is requesting, rather than a permanent part of the structure.) The point of this design is that you _never_ have to read data from the lw- or Xt-level TabsWidget. Your comments sprinkled throughout the code indicate that that is problematic. And it's easy enough to generate SIGSEGVs just reading Xt objects :-(. The interface would be to setf the RLOS or get the CLOS. When you setf the RLOS, the widget's redisplay callback notices this, and walks both lists. When corresponding objects differ, set tabLabelWidget.string(i) = CLOS(i) = RLOS(i) and a dirty flag on tabLabelWidget(i). In case len(RLOS) > len(CLOS), new TabLabelWidgets are allocated and initialized. Optionally, if len(RLOS) < len(CLOS), you could free some TabLabelWidgets, but unless there are a lot to be freed it's probably better to just unmanage them. Note that this could actually be efficiently separated into two loops, since you only need to compare addresses of tabLabelWidget.string(i) and CLOS(i). Similar considerations would apply to the top tab and the display parameters, except that those would be fixed structures. This should make it possible to set all the labels, display parameters, and the topWidget with one call to XtSetValues(). I don't think this is so different from what you're doing now, except that discussing it this way shows that all this stuff could be handled in about three places in the code (tabs instantiation, tabs redisplay, and the implementation of TabsSetValues() in lwlib/xlwtabs.[ch]). But at the moment things seem to be spread out all over. (At least, looking at TabsSetValues(), x_tab_control_instantiate(), and x_tab_control_redisplay() gives the impression that something must be handled elsewhere.) And the role of things like the hook in buffer.c get confused; it looks like it's tab-specific when actually it's intended as a general facility that wasn't needed until buffer tabs came along. -- University of Tsukuba Tennodai 1-1-1 Tsukuba 305-8573 JAPAN Institute of Policy and Planning Sciences Tel/fax: +81 (298) 53-5091 _________________ _________________ _________________ _________________ What are those straight lines for? "XEmacs rules."