BInputServerMethod::SetMenu doesn't work

[jalopeura]

BInputServerMethod::SetMenu doesn't actually add a submenu to the item in the input server menu. The problem seems to be in _BMethodAddOn_::SetMenu:

if (menu)

menu->Archive(&menuMsg);

should be

if (menu)

menu->Archive(&menuMsg, true);

Without that second argument, the menu items are not included.

(I have been unable to test this, since PM won't let me simply overwrite the input server with my newly compiled version; I'm going to have to build an entire system and install it to a partition.

I will attach a patch, but if nobody else gets to this, I will try to find time to build an entire installation to test this.

[jalopeura]

mmu_man and jessicah introduced me to the ways of blacklisting, and I was able to test this. My original patch was not the proper solution (that parameter defaults to true, so it actually didn't change the behavior at all).

However, I have uploaded a second patch that does work. You can test it with the t9 input method add-on.

[jessicah]

Can you please use static_cast<BMenu*> instead of a C-style cast?

[axeld]

I don't mind a C-style (we pretty much use it everywhere), but there are a few minor coding style issues:

• "msg" is not a good name, especially, when there already is a "message". A more descriptive name that gives you an idea of what it is used for would be appreciated.
• Also, we're using boolean expressions in if-clauses so it's fMenu != NULL and not just fMenu.

Also, is there any reason why BMenu::Instantiate() does not return a BMenu?

[jalopeura]

1) Sorry if any of you are following this ticket and just got a bunch of update emails; it's 5am here and I kept making stupid mistakes - like uploading the wrong file or adding a static_cast to a piece of code I had not previously changed, etc, and then not realizing my mistake until I looked at the color-coded diff the ticket page shows.

2) The chunk of code in MethodReplicant::AddMethod was actually copied almost entirely unchanged from MethodReplicant::UpdateMethodMenu, so the coding and naming issues came from there, and in _BMethodAddOn_::SetMenu I kept the same style. I have changed them in my new code, but not in their original location.

3) All versions of<whatever>::Instantiate are required to return a BArchivable. I believe this is so the code in Archivable.cpp can build a mangled function name in order to load the proper symbol.

[axeld]

Replying to jalopeura:

3) All versions of<whatever>::Instantiate are required to return a BArchivable. I believe this is so the code in Archivable.cpp can build a mangled function name in order to load the proper symbol.

The return type is not part of the function signature.

[jalopeura]

Replying to axeld:

The return type is not part of the function signature.

The Be Book says

Instantiate() must return a BArchivable*, regardless of the actual class in which it's implemented.

If it's not for mangling...ah, it's because it's a virtual method inherited from BArchivable, so the vtable wouldn't find it if it didn't have the right return type.

[jessicah]

Replying to axeld:

I don't mind a C-style (we pretty much use it everywhere), but there are a few minor coding style issues:

C-style casts are a coding style issue :p

"Use C++ casts (dynamic_cast, static_cast, const_cast, reinterpret_cast) over C casts."

Although, perhaps one should be using dynamic_cast here; reading up, I don't think can use static_cast with virtual inheritance.

[axeld]

I plead guilty, however, it would still be nice to get away with that cast completely.

[pulkomandy]

Applied in hrev50868.