Re: [Wireshark-dev] [Wireshark-commits] rev 39422: /trunk/gtk/ /trunk/gtk/: main_menubar.c
On Fri, Oct 14, 2011 at 3:50 PM, Guy Harris wrote: > > On Oct 14, 2011, at 8:37 AM, Tony Trinh wrote: > > > Actually, the valid #define for Lua code throughout Wireshark is > HAVE_LUA_5_1 (not HAVE_LUA). HAVE_LUA works in Windows but not other OS's > (such as OSX). > > ...which probably means "such as UN*Xes"; that sounds like a difference > between using autoconf and config.nmake. > > If we don't have a HAVE_LUA definition independent of the Lua version, that > sounds like a bug in the configure script. Does CMake define HAVE_LUA, > HAVE_LUA_5_1, both, or neither? > CMake defines HAVE_LUA_5_1 but not HAVE_LUA. On the other hand, config.nmake defines both, and it's actually the only file to define HAVE_LUA. For reference, there are 3 files that define HAVE_LUA_5_1 and 7 source files that test for it (trivial fix). > > It's really named "make_menu_actions()". I named the function based on > what it does, not based on who calls it. That function (and make_menu_xml()) > can easily be used outside the context of Lua menus, and there's nothing > about them that fundamentally binds them to Lua. That said, it doesn't > matter enough to me if their names include the word "lua" since they might > change at a later point when someone sees wider use for them. > > "Someone" meaning "the Python interface", for example? Yes. Good example. ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] rev 39422: /trunk/gtk/ /trunk/gtk/: main_menubar.c
On Oct 14, 2011, at 8:37 AM, Tony Trinh wrote: > Actually, the valid #define for Lua code throughout Wireshark is HAVE_LUA_5_1 > (not HAVE_LUA). HAVE_LUA works in Windows but not other OS's (such as OSX). ...which probably means "such as UN*Xes"; that sounds like a difference between using autoconf and config.nmake. If we don't have a HAVE_LUA definition independent of the Lua version, that sounds like a bug in the configure script. Does CMake define HAVE_LUA, HAVE_LUA_5_1, both, or neither? > It's really named "make_menu_actions()". I named the function based on what > it does, not based on who calls it. That function (and make_menu_xml()) can > easily be used outside the context of Lua menus, and there's nothing about > them that fundamentally binds them to Lua. That said, it doesn't matter > enough to me if their names include the word "lua" since they might change at > a later point when someone sees wider use for them. "Someone" meaning "the Python interface", for example? ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] rev 39422: /trunk/gtk/ /trunk/gtk/: main_menubar.c
On Fri, Oct 14, 2011 at 4:37 PM, Tony Trinh wrote: > Hi Martin/Anders, > > Actually, the valid #define for Lua code throughout Wireshark is > HAVE_LUA_5_1 (not HAVE_LUA). HAVE_LUA works in Windows but not other OS's > (such as OSX). But I do like HAVE_LUA better than a version-specific name > because I imagine these names will have to be updated upon switching to > newer versions of Lua, especially with 5.2 coming soon (unless there's some > version incompatibility). > > Given that its only called from merge_lua_menu_items(), might >> make_menu_items() be too generic a function name? > > > It's really named "make_menu_actions()". I named the function based on what > it does, not based on who calls it. That function (and make_menu_xml()) can > easily be used outside the context of Lua menus, and there's nothing about > them that fundamentally binds them to Lua. That said, it doesn't matter > enough to me if their names include the word "lua" since they might change > at a later point when someone sees wider use for them. > > -Tony > > Here is the checkin comment (r39424): Make make_menu_actions() static again, but protect with #ifdef HAVE_LUA_5_1 #endif As per discission on wireshark-dev, this function might later see wider use, so don't add 'lua' to the function name. Martin ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] rev 39422: /trunk/gtk/ /trunk/gtk/: main_menubar.c
Hi Martin/Anders, Actually, the valid #define for Lua code throughout Wireshark is HAVE_LUA_5_1 (not HAVE_LUA). HAVE_LUA works in Windows but not other OS's (such as OSX). But I do like HAVE_LUA better than a version-specific name because I imagine these names will have to be updated upon switching to newer versions of Lua, especially with 5.2 coming soon (unless there's some version incompatibility). Given that its only called from merge_lua_menu_items(), might > make_menu_items() be too generic a function name? It's really named "make_menu_actions()". I named the function based on what it does, not based on who calls it. That function (and make_menu_xml()) can easily be used outside the context of Lua menus, and there's nothing about them that fundamentally binds them to Lua. That said, it doesn't matter enough to me if their names include the word "lua" since they might change at a later point when someone sees wider use for them. -Tony ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] rev 39422: /trunk/gtk/ /trunk/gtk/: main_menubar.c
From: wireshark-dev-boun...@wireshark.org
[mailto:wireshark-dev-boun...@wireshark.org] On Behalf Of Martin Mathieson
Sent: den 14 oktober 2011 14:37
To: Developer support list for Wireshark
Subject: Re: [Wireshark-dev] [Wireshark-commits] rev 39422: /trunk/gtk/
/trunk/gtk/: main_menubar.c
On Fri, Oct 14, 2011 at 12:11 PM, Guy Harris
mailto:g...@alum.mit.edu>> wrote:
On Oct 14, 2011, at 4:03 AM,
mart...@wireshark.org<mailto:mart...@wireshark.org> wrote:
> http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=39422
>
> User: martinm
> Date: 2011/10/14 04:03 AM
>
> Log:
> make_menu_items() isn't called without LUA support, so can't be static.
Yes, it can; the only reason it couldn't be static would be if it were called
from outside gtk/main_menubar.c, which it doesn't appear to be.
Its definition might have to protected with the same #ifdef as its use(s), to
avoid "defined but not used" warnings, but that's another matter.
Given that its only called from merge_lua_menu_items(), might make_menu_items()
be too generic a function name?
I don't mind keeping the function static, but protecting the whole definition
with #ifdef HAVE_LUA_5_1
While I'm at it shall I rename make_menu_items() to make_lua_menu_items() ?
Martin
Perhaps you can use my proposal AND rename the function?
Index: main_menubar.c
===
--- main_menubar.c (revision 913)
+++ main_menubar.c (working copy)
@@ -3694,6 +3694,8 @@
* Creates an action group for the menu items in xpath, and returns it. The cal
ler should
* use g_object_unref() on the returned pointer if transferring scope.
*/
+#ifdef HAVE_LUA
+/* NOTE currently only used from Lua, remove this ifdef when used
+outside of #i
fdef LUA */
static GtkActionGroup*
make_menu_actions(const char *path, const menu_item_t *menu_item_data) {
GtkActionGroup *action_group;
@@ -3775,11 +3777,11 @@
return action_group;
}
-
+#endif
static void
merge_lua_menu_items(GList *merge_lua_menu_items_list _U_) { -#ifdef
HAVE_LUA_5_1
+#ifdef HAVE_LUA
guint merge_id;
GtkActionGroup *action_group;
menu_item_t *menu_item_data;
___
Sent via:Wireshark-dev mailing list
Archives:http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] rev 39422: /trunk/gtk/ /trunk/gtk/: main_menubar.c
On Fri, Oct 14, 2011 at 12:11 PM, Guy Harris wrote: > > On Oct 14, 2011, at 4:03 AM, mart...@wireshark.org wrote: > > > http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=39422 > > > > User: martinm > > Date: 2011/10/14 04:03 AM > > > > Log: > > make_menu_items() isn't called without LUA support, so can't be static. > > Yes, it can; the only reason it couldn't be static would be if it were > called from outside gtk/main_menubar.c, which it doesn't appear to be. > > Its definition might have to protected with the same #ifdef as its use(s), > to avoid "defined but not used" warnings, but that's another matter. > Given that its only called from merge_lua_menu_items(), might make_menu_items() be too generic a function name? I don't mind keeping the function static, but protecting the whole definition with #ifdef HAVE_LUA_5_1 While I'm at it shall I rename make_menu_items() to make_lua_menu_items() ? Martin ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] rev 39422: /trunk/gtk/ /trunk/gtk/: main_menubar.c
On Oct 14, 2011, at 4:03 AM, mart...@wireshark.org wrote: > http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=39422 > > User: martinm > Date: 2011/10/14 04:03 AM > > Log: > make_menu_items() isn't called without LUA support, so can't be static. Yes, it can; the only reason it couldn't be static would be if it were called from outside gtk/main_menubar.c, which it doesn't appear to be. Its definition might have to protected with the same #ifdef as its use(s), to avoid "defined but not used" warnings, but that's another matter. ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
