Re: new xforms patch
On Wed, 11 Oct 2000, Angus Leeming wrote:
> Attached is a patch implementing Allan's suggestions about a FormInset base
> class. I've actually implemented three small new classes:
>
> FormBaseBI and FormBaseBD are base classes for Buffer Independent and Buffer
> Dependent dialogs respectively. FormInset is, in turn, derived from
> FormBaseBD.
[sigh] Didn't I tell you not to run off and implement this stuff for a few
days so we could have time to think about it. ;-)
It occured to me that we could make things clearer by just merging the two
enums in FormBase to:
enum BufferDependency {
BUFFER_UPDATE, // always update
BUFFER_CHECK, // same buffer then update else hide
INDEPENDENT
}
This removes the confusion I mentioned in FormPreferences, although it
still leaves the buffer check problem.
An alternative fix would be by making
Signal1 updateBufferDependent;
Such that true == "buffer change", and false == "same buffer". This is
simple enough and we could then eliminate the keeping of a Buffer*
anywhere. We wouldn't need the updateOrHide() stuff at all. We wouldn't
need the modified BufferDependency enum above or the ChangedBufferAction
enum. The various update() members would need to change to accept a bool
but they could/should also default to false. The testing of whether to
hide or update can then be done in the update() where it belongs. (It'd
also mean we wouldn't need most of your patch -- see why I told you not to
run off and implement the FormInset stuff: some other idea came up)
What do you think of this?
I'll take a look at your patch and see what I think of that.
Frustrating aren't I?
> I've also got my head around overloading connect() and disconnect() in
> the daughter classes. Incidentally, Allan, I think that the names of
> these methods are fine. Think of them as meaning "things to be done on
> connection" rather than simply "connect signals".
Fair enough. I'll add a longer comment to that effect to FormBase.
> All this gives no new functionality, just cleans up the code base. Hope that
> its now logical.
We probably still want a FormInset anyway. Although if we adopt my
suggestion for changing the updateBufferDependent signal much of what you
have written will need to be rewritten (or just culled). I'm inclinded to
just leave the BufferDependency stuff in FormBase rather than add another
layer in the form of FormBaseB[DI].
> One new piece of functioality has been added, however. I've used some of
> Baruch's code from FormGraphics in the FormInset-derived classes. If a dialog
> is open and recieves a new "Show" or "Create" signal from another inset, then
> the dialog is updated with the new inset's data. No need to first close the
> dialog to one inset before opening it for another.
Yay!
Allan. (ARRae)
Re: new xforms patch
On Thu, 12 Oct 2000, Allan Rae wrote: > On Wed, 11 Oct 2000, Angus Leeming wrote: > > > Attached is a patch implementing Allan's suggestions about a FormInset base > > class. I've actually implemented three small new classes: > > > > FormBaseBI and FormBaseBD are base classes for Buffer Independent and Buffer > > Dependent dialogs respectively. FormInset is, in turn, derived from > > FormBaseBD. > > [sigh] Didn't I tell you not to run off and implement this stuff for a few > days so we could have time to think about it. ;-) [...] Good news... I'll apply it to my tree. and then I'll do the stuff below: > An alternative fix would be by making > Signal1 updateBufferDependent; > > Such that true == "buffer change", and false == "same buffer". [...] > > I've also got my head around overloading connect() and disconnect() in > > the daughter classes. Incidentally, Allan, I think that the names of > > these methods are fine. Think of them as meaning "things to be done on > > connection" rather than simply "connect signals". Why is it then that you wanted to add an ihSignal_ in FormInset when it would make more sense to just make the connection in the appropriate showInset() like it used to be done? Over zealous perhaps? There are a few things that'll be easy to clean up before I commit it. (mostly related to redefining the updateBufferDepedent signal) Allan. (ARRae)
Re: new xforms patch
On Thu, 12 Oct 2000, Allan Rae wrote: > Good news... I'll apply it to my tree. > > and then I'll do the stuff below: > > > An alternative fix would be by making > > Signal1 updateBufferDependent; > > > > Such that true == "buffer change", and false == "same buffer". > > [...] > > There are a few things that'll be easy to clean up before I commit it. > (mostly related to redefining the updateBufferDepedent signal) I've run out of time and have to go out tonight so you'll just have to wait till tomorrow. Allan. (ARRae)
Re: new xforms patch
> [sigh] Didn't I tell you not to run off and implement this stuff for a few > days so we could have time to think about it. ;-) ;-) Things as they were were just t nasty! I blame you for pointing out just how nasty! > An alternative fix would be by making > Signal1 updateBufferDependent; > What do you think of this? Magnificent idea! Clean, simple and elegant. > Why is it then that you wanted to add an ihSignal_ in FormInset when it > would make more sense to just make the connection in the appropriate > showInset() like it used to be done? Over zealous perhaps? Well, it struck me that all those insets with dialogs will emit a hide signal when deleted or cut, so the right place to connect this signal to the dialog itself would be in FormInset::connect(). Unfortunately, the base Inset class does not have a hide signal (ie, no &Inset::hide), so I had to obtain &InsetXXX::hide in the various showInset() methods (stored as ihSignal_) and then connect it in FormInset::connect(). I know... kludgy! However, I think that the idea is basically correct. The right place to connect ih_ IS in Form::Inset::connect(). All that needs to be done is to make an Inset::hide signal rather than have all those InsetXXX::hide signals. My turn to ask: what do you think of this? Angus
Re: new xforms patch
On Thu, 12 Oct 2000, Angus Leeming wrote:
> > [sigh] Didn't I tell you not to run off and implement this stuff for a few
> > days so we could have time to think about it. ;-)
>
> ;-) Things as they were were just t nasty! I blame you for pointing
> out just how nasty!
Well, I did need the extra poke in the ribs your patch provided to think
of it. It does look a lot better now.
> > An alternative fix would be by making
> > Signal1 updateBufferDependent;
>
> > What do you think of this?
> Magnificent idea! Clean, simple and elegant.
Yeah well, hmmm...
I ended having to make everyone have an update(bool) rather than my
preferred idea of FormBaseBD::update(bool), FormBaseBI::update() and no
update in FormBase. This fell through because of the call to update() in
FormBase::show(). I didn't want to reimplement show() in both B[DI] so
just left it alone.
{... ihSignal_ ...]
> I know... kludgy! However, I think that the idea is basically correct. The
> right place to connect ih_ IS in Form::Inset::connect(). All that needs to be
> done is to make an Inset::hide signal rather than have all those
> InsetXXX::hide signals.
>
> My turn to ask: what do you think of this?
I didn't really look into the implications of such a change. It would
mean we'd require every inset to carry an extra member function which for
most would be empty.
As it stands in the revised code I culled ihSignal_ and do the connection
in the FormCommand::showInset(). show() gets called immediately
afterwards from that function so it's not worth losing sleep over since
other insets can do likewise. And many of those will derive from some
intermediate inset which could add the hide member. There are a few
insets that cuurently use dialogs that'll become editible/collapsible
in-buffer insets so the number of insets to worry about is small.
So I'm saying don't touch it until it becomes an obvious problem and
certainly don't touch it till after 1.1.6.
##
Overall the xforms stuff is looking very nice. I took at peek at the
gnome and kde stuff as a result of the change to update and
updateBufferDependent. There seems to be an aweful lot of code just to
get things running with gnome. There's also a few connections being made
to update that aren't used (since update is empty) is this a temporary
thing? (ie. do you plan on filling in some more code?)
There also appears to be a few places where a new update_something()
function exists that could be used as update() and an update connection
would then make sense.
Marko is there any particular reason for using NULL instead of 0 in a lot
of your code? Is this a gnome convention? Where is it defined?
BTW, I've said it before but I'll say it again anyway since it's friday:
You don't have to call your files by the same name as the xforms code.
I would have expected KDE for example to use DialogX myself -- or
whatever naming convention exists for apps using a particular toolkit.
John, I know it's a nice ideal to have qtarch files for building the
dialogs but feel free to use the dialogs from klyx. My plan two years ago
was just to finish the xforms stuff and then bundle in all the klyx
dialogs.
With KDE-2.0 not far off the need for easy maintenance of the KDE-1.1.2
stuff is somewhat lessened. Perhaps, Matthias and Karl did use a dialog
editor but left those files out of their dist. It's worth asking.
There that's probably pissed nearly all the GUII devvies off.
Have a nice Friday 13th,
Allan. (ARRae)
Re: new xforms patch
> Overall the xforms stuff is looking very nice. I took at peek at the > gnome and kde stuff as a result of the change to update and > updateBufferDependent. There seems to be an aweful lot of code just to > get things running with gnome. True! There are two reasons for it. First, I decided to code some dialogs and then compose the base classes looking into the implementation of several dialogs. This approach helped me to learn LyX and its requirements to compose base classes better. Second, all the dialogs which use "action" area have to be constructed on "fly" and change/update on the basis of user input. This makes using the dialogs easier, faster and much more fun, but coding ... well, you mentioned a size of the code :). > There's also a few connections being made to update that aren't used > (since update is empty) is this a temporary thing? (ie. do you plan on > filling in some more code?) Most (even it seems to me that all) of these empty update functions are in "action" area dialogs. My latest patch connects updateBufferDependent to hide function. The reasons for it are as follows. If I understand correctly "update" is called when user opens a new document or changes the document. If this is true then for "action" area dialogs the update signal has to close the dialog since user is not interested in this action anymore :). At least it is easier to implement than keeping action area state for each document... > There also appears to be a few places where a new update_something() > function exists that could be used as update() and an update connection > would then make sense. the quick grep for update in gnome sources showed only updateButtons in FormCitation that might look as a candidate for an update connection and which doesn't use it. However, updateButtons is used to track user input in FormCitation dialog and has nothing to do with change of the document. > Marko is there any particular reason for using NULL instead of 0 in a lot > of your code? Is this a gnome convention? Where is it defined? I am just used to call NULL a pointer that might lead to a core dump. I haven't looked into gnome conventions recently, but if you will give me a good reason to use integer 0 insted of NULL then I can change everywhere NULL to this particular integer value. > BTW, I've said it before but I'll say it again anyway since it's friday: > You don't have to call your files by the same name as the xforms code. > I would have expected KDE for example to use DialogX myself -- or > whatever naming convention exists for apps using a particular toolkit. Unfortunately, the gnome frontend is still lagging behind xforms. That's why it is easier to use the same names for the dialogs since it gives a good overview of the state of the port. Finally, it is nice to remember roots of LyX when everyone will switch to Gnomified version :). [...] > There that's probably pissed nearly all the GUII devvies off. No way! It is very nice to have someone who is looking into the code. It is a first step to make this effort in porting LyX to gnome or kde from one-man project to dynamic project full of discussions/suggestions/implementations as the rest of LyX is. Actually, this is a reason why I would not like to have cvs-write access anytime soon. This guarantees that the code was working at least in two occasions and someone has at least just scanned the patch. > Have a nice Friday 13th, Enjoy it too. Marko
Re: new xforms patch
> "Marko" == Marko Vendelin <[EMAIL PROTECTED]> writes: >> Marko is there any particular reason for using NULL instead of 0 in >> a lot of your code? Is this a gnome convention? Where is it >> defined? Marko> I am just used to call NULL a pointer that might lead to a core Marko> dump. I haven't looked into gnome conventions recently, but if Marko> you will give me a good reason to use integer 0 insted of NULL Marko> then I can change everywhere NULL to this particular integer Marko> value. I could not find a reference to it anywhere, but I am sure Lars will be able to remind us why NULL is evil :) JMarc
Re: new xforms patch
On Fri, 13 Oct 2000, Allan Rae wrote: > BTW, I've said it before but I'll say it again anyway since it's friday: > You don't have to call your files by the same name as the xforms code. > I would have expected KDE for example to use DialogX myself -- or > whatever naming convention exists for apps using a particular toolkit. > well I went with what was there ... in fact this is a fairly sensible naming convention I think for two reasons 1) even though there is no need for the frontends to be the same, needless differences make it more difficult for people to update unilaterally when I'm dead or something ... 2) a silly Friday reason - if we had DialogPrint... then the gui dervied class would be called DialogPrintDialog ;) 3) I like to thing of "Form" meaning LyX's notion of the dialogs, and Dialog, the GUIs notion > John, I know it's a nice ideal to have qtarch files for building the > dialogs but feel free to use the dialogs from klyx. My plan two years ago > was just to finish the xforms stuff and then bundle in all the klyx > dialogs. > I am referring to klyx source for every dialog, but in actuality (other than bits of the interface code that can be stolen) it is just as quick to use qtarch for the actual GUI parts of it (now that I've got used to its foibles ;) For example I have dlg files for formparagraph, which took about 40 minutes max to put together. Some of the FormParagraph code can be borrowed from klyx, but in fact most of the code will be fairly similar to the xforms/ stuff anyway... I use klyx for inspiration mostly ;) > With KDE-2.0 not far off the need for easy maintenance of the KDE-1.1.2 > stuff is somewhat lessened. Well, maybe I'm living in a dreamworld but I would like if possible somehow to maintain lyx frontends for both - there will be plenty of KDE 1 users for a long long while yet, especially in academic settings ... in actuality this probably isn't viable, but I can hope ... > Perhaps, Matthias and Karl did use a dialog > editor but left those files out of their dist. It's worth asking. > yep, I don't think they did though, they probably just flow-of-conciousness wrote it ;) john -- "I think there is a world market for maybe five computers." - Thomas Watson, IBM Chairman, 1943
Re: new xforms patch
On 13 Oct 2000, Jean-Marc Lasgouttes wrote: > > "Marko" == Marko Vendelin <[EMAIL PROTECTED]> writes: > > >> Marko is there any particular reason for using NULL instead of 0 in > >> a lot of your code? Is this a gnome convention? Where is it > >> defined? > > Marko> I am just used to call NULL a pointer that might lead to a core > Marko> dump. I haven't looked into gnome conventions recently, but if > Marko> you will give me a good reason to use integer 0 insted of NULL > Marko> then I can change everywhere NULL to this particular integer > Marko> value. > > I could not find a reference to it anywhere, but I am sure Lars will > be able to remind us why NULL is evil :) I'll supply one: NULL is a C'ism while 0 is C++. That's why I asked where NULL is defined. I suspect you are getting NULL from a C header file where it is probably defined as (void*)0 or something similar -- if you write 0 the C++ compiler will consider it to be static_cast(0) and the 0 will therefore have an _appropriate_ type. If you were being tricky and setting all your dynamic memory to 0x0a or some other marker to help detect uninitialised memory I could understand you using something other than 0 to initialise a pointer (provided the rest of your code was full of Assert()'s checking the pointers against that value). In C++ if you mean zero you write 0. IIRC, this is mentioned in both the C++ FAQ and Effective C++. Allan. (ARRae)
Re: new xforms patch
On Sat, 14 Oct 2000, Allan Rae wrote: > On 13 Oct 2000, Jean-Marc Lasgouttes wrote: > > > > "Marko" == Marko Vendelin <[EMAIL PROTECTED]> writes: > > > > >> Marko is there any particular reason for using NULL instead of 0 in > > >> a lot of your code? Is this a gnome convention? Where is it > > >> defined? > > > > Marko> I am just used to call NULL a pointer that might lead to a core > > Marko> dump. I haven't looked into gnome conventions recently, but if > > Marko> you will give me a good reason to use integer 0 insted of NULL > > Marko> then I can change everywhere NULL to this particular integer > > Marko> value. > > > > I could not find a reference to it anywhere, but I am sure Lars will > > be able to remind us why NULL is evil :) > > I'll supply one: NULL is a C'ism while 0 is C++. > > That's why I asked where NULL is defined. I suspect you are getting NULL > from a C header file where it is probably defined as (void*)0 or something > similar -- if you write 0 the C++ compiler will consider it to be > static_cast(0) and the 0 will therefore have an > _appropriate_ type. > > If you were being tricky and setting all your dynamic memory to 0x0a or > some other marker to help detect uninitialised memory I could understand > you using something other than 0 to initialise a pointer (provided the > rest of your code was full of Assert()'s checking the pointers against > that value). In C++ if you mean zero you write 0. IIRC, this is > mentioned in both the C++ FAQ and Effective C++. I see. I'll submit a patch as soon as I'll be back in my office (in a week or so). Marko
