commit -- [was Re: patch: options dialog & preferences]

Jeff Hostetler (jeff@abisource.com)
Sat, 6 Nov 1999 18:44:32 -0600


i've applied the patch that was sent to us by Stephen Hack <shack@uiuc.edu>
it has a very nice options(preferences) dialog for Unix and a bunch of
cross-platform improvements to the preferences mechanism.

this only work on unix at the moment. i'm going to take a stab at the win32
version after a quick reboot -- but this may take a little while since dialogs
are sooooo much fun.... i won't be able to look at the beos version, since
i don't have access to a beos machine at the moment.

On Thu, 28 Oct 1999, Stephen Hack wrote:
> The lastest patch for the options dialog is at the following location
> http://argus.itg.uiuc.edu/~shack/abisource/Oct28/patch.gz

==========================================================
Stephen,

I did a quick scan of your patch. I have a couple of tweaks that I've
done or would like to suggest:

[] I removed the local definition of UT_ASSERT() in ut_dialogHelper. I
was sure what the intent was -- ut_assert should always be defined
if the header file is included....

[] In XAP_Prefs::removeListener(), the remove is done using the function
address as a key. this probably won't have the desired effect if multiple
instances of an object each register their own listener. (i'm not sure this
would happen, but) i think it would be better to key off the data pointer
instead of the function pointer. for the other listeners, I used listenerIDs
as a way to avoid this. i haven't looked at the various listeners yet, so
i'll leave this decision up to you. the thing to think about is what happens
when we have multiple windows open....

[] In XAP_Prefs::_sendPrefsSignal(), you should probably delete the
items in the hash, so that a subsequent startBlock()/endBlock() won't
resend the set of keys from an earlier one. (at least that's what it looks
like.)

> > [2] add strings to the string table for each of the dialog static
> > text fields (see abi/src/wp/ap/xp/ap_String_Id.h) and then
> > reference them in the various window_title and button_label calls
> > (this will let us internationalize them quickly)
> > (see one of the other dialogs for code snippets)
>
> Not started. Reason behind this is that I've been focusing on functionality
> initially. Once the dialog gets into CVS, patchs would be much simpler.

ok.

> > [4] down with the OK and Cancel buttons, I'd like to have a
> > "restore factory settings" button -- which would set the
> > current scheme back to _builtin_ and nuke any other schemes
> > in memory.
>
> The button is there and works as expected. (although, it does not nuke any
> 'other' scheme in memory)

it didn't look like scheme-switching sent notifications to the listeners.
it probably should. if you just want to blast notifications for all the keys
in the new scheme or just the ones that are different from the previously
installed one is up to you.

cheers and thanks again,
jeff



This archive was generated by hypermail 1.03b2.