On Tue, Jul 01, 2003 at 07:14:40AM +0200, Olivier Chapuis wrote:
> On Tue, Jul 01, 2003 at 12:53:34AM +0200, [EMAIL PROTECTED] wrote:
> > 2) In clone_menu_root_static():
> >
> > What is the logic of changing
> >
> > MR_COPIES(dest_mr) = 0;
> >
> > to
> >
> > MR_COPIES(dest_mr) = 1;
> >
> > ? Was it a bug?
> >
>
> When, you create a menu (with AddToMenu) MR_COPIES(mr) = 1. The menu
> code logic implies that MR_COPIES is never 0 but in DestroyMenu just
> before it is totally destroyed (and maybe re-created and in this case
> MR_COPIES is re-incremented).
>
> Yes, I think it was a bug. What happen if you enter DestroyMenu
> with MR_COPIES(dest_mr) = 0?
>
> ...
> MR_COPIES(dest_mr)--; <-- MR_COPIES is -1
> ...
> if (MR_COPIES(dest_mr) == 0) <-- leaks
> {
> free some stuff
> }
>
> Any way if you change the test
>
> MR_COPIES(dest_mr) == 0
>
> by
>
> MR_COPIES(dest_mr) <= 0
>
> it is not a big problem to set MR_COPIES to 0 for a tear off
> menu. However, it seems that the logic is to set MR_COPIES
> to 1 when you "create" a tear off menu.
Yes, it may have been a bug. However, testing for <= 0 hides
potential problems elsewhere. Instead, we should call abort() if
it ever becomes < 0.
>
> > 4) Please name all functions exported in menustyle.c
> > "menustyle_something()". (I renamed copy_menu_style() to
> > menustyle_copy()).
> >
>
> Yep, such naming scheme is good. Ha 2 questions.
> Under which condition you start the function with "__"?
> (a private version of an exported function?).
When I feel like it :-) Seriously, the __ indicates its an
internal helper function that encapsulates some work done by
higher level functions of the same module. Only static functions
can begin with __. It indicates that the function is really,
*really* internal and must never be exported.
> What is the goal of the TearMenuOff command, how can
> we use it?
It is a dummy command only defined in menus, exactly like "Title".
Clicking on an item that uses it tears off the menu. It's not
implemented yet, though. If you try to write the command, the
following should be taken into account:
a) It should work on all visible menu items:
- normal items ("+ foo tearmenuoff")
- separators ("+ bar nop tearmenuoff"?)
- titles ("+ baz title tearmenuoff"?)
b) An item that can tear off a menu must be selectable, i.e.
must be hilighted when the mouse is over it. This must be
somehow merged into the IS_SELECTABLE flag. Perhaps we need
a new flag too.
c) It would be nice if it worked in the dynamic menu actions.
This is probably hard to implement, and thus optional.
Bye
Dominik ^_^ ^_^
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [EMAIL PROTECTED]
To report problems, send mail to [EMAIL PROTECTED]
