> From: "Rodolfo García Peñas (kix)" <[email protected]>
>
> The function dockMenuCreate is splitted in three functions:
>
> - create_clip_main_menu(WScreen*)
> - create_dock_main_menu(WScreen*)
> - dockMenuCreate(WScreen*, int*)
First I liked the patch but now I don't :) My biggest complaint is
that it introduces code duplication. Readability is a bit better, but
in retrospect, the if (type != WM_CLIP) { ... } else { ... } is quite
readable already. The last benefit of your patch is that it separated
the clip menu caching (scr->clip_menu) from the actual menu creation,
but that point alone doesn't warrant creating a new function IMHO. So
my recommendation would be to leave the original code unchanged.
-- Daniel
> The function create_clip_main_menu() creates the menu and the menu
> items
> when the menu is for a Clip.
>
> The function create_dock_main_menu() creates the menu and the menu
> items
> when the menu is for a Dock.
>
> The function dockMenuCreate() is now smaller, and more clear. The
> function
> select if the menu creation is for a clip or for a dock, and then
> call the
> right function. The function uses switch-case instead if-else and
> controls
> if the Clip was previously created. The menu and menuitem creation is
> outside
> then, is easier to understand it.
>
> Signed-off-by: Rodolfo García Peñas (kix) <[email protected]>
> ---
> src/dock.c | 114
> ++++++++++++++++++++++++++++++++++++++----------------------
> 1 file changed, 73 insertions(+), 41 deletions(-)
>
> diff --git a/src/dock.c b/src/dock.c
> index 21d6aee..d4d25ce 100644
> --- a/src/dock.c
> +++ b/src/dock.c
> @@ -961,59 +961,73 @@ static WMenu *makeClipOptionsMenu(WScreen *scr)
> return menu;
> }
>
> -static WMenu *dockMenuCreate(WScreen *scr, int type)
> +static WMenu *create_dock_main_menu(WScreen *scr)
> {
> WMenu *menu;
> WMenuEntry *entry;
>
> - if (type == WM_CLIP && scr->clip_menu)
> - return scr->clip_menu;
> + menu = wMenuCreate(scr, NULL, False);
> +
> + entry = wMenuAddCallback(menu, _("Keep on Top"),
> toggleLoweredCallback, NULL);
> + entry->flags.indicator = 1;
> + entry->flags.indicator_on = 1;
> + entry->flags.indicator_type = MI_CHECK;
> +
> + wMenuAddCallback(menu, _("Launch"), launchCallback, NULL);
> + wMenuAddCallback(menu, _("Unhide Here"), unhideHereCallback, NULL);
> +
> + entry = wMenuAddCallback(menu, _("Hide"), hideCallback, NULL);
> + wfree(entry->text);
> + entry->text = _("Hide");
> +
> + wMenuAddCallback(menu, _("Settings..."), settingsCallback, NULL);
> + wMenuAddCallback(menu, _("Kill"), killCallback, NULL);
> +
> + return menu;
> +}
> +
> +static WMenu *create_clip_main_menu(WScreen *scr)
> +{
> + WMenu *menu;
> + WMenuEntry *entry;
>
> menu = wMenuCreate(scr, NULL, False);
> - if (type != WM_CLIP) {
> - entry = wMenuAddCallback(menu, _("Keep on Top"),
> toggleLoweredCallback, NULL);
> - entry->flags.indicator = 1;
> - entry->flags.indicator_on = 1;
> - entry->flags.indicator_type = MI_CHECK;
> - } else {
> - entry = wMenuAddCallback(menu, _("Clip Options"), NULL, NULL);
> - scr->clip_options = makeClipOptionsMenu(scr);
> - if (scr->clip_options)
> - wMenuEntrySetCascade(menu, entry, scr->clip_options);
>
> - entry = wMenuAddCallback(menu, _("Rename Workspace"),
> renameCallback, NULL);
> - wfree(entry->text);
> - entry->text = _("Rename Workspace");
> + entry = wMenuAddCallback(menu, _("Clip Options"), NULL, NULL);
> + scr->clip_options = makeClipOptionsMenu(scr);
> + if (scr->clip_options)
> + wMenuEntrySetCascade(menu, entry, scr->clip_options);
>
> - entry = wMenuAddCallback(menu, _("Selected"), selectCallback,
> NULL);
> - entry->flags.indicator = 1;
> - entry->flags.indicator_on = 1;
> - entry->flags.indicator_type = MI_CHECK;
> + entry = wMenuAddCallback(menu, _("Rename Workspace"),
> renameCallback, NULL);
> + wfree(entry->text);
> + entry->text = _("Rename Workspace");
>
> - entry = wMenuAddCallback(menu, _("Select All Icons"),
> selectIconsCallback, NULL);
> - wfree(entry->text);
> - entry->text = _("Select All Icons");
> + entry = wMenuAddCallback(menu, _("Selected"), selectCallback,
> NULL);
> + entry->flags.indicator = 1;
> + entry->flags.indicator_on = 1;
> + entry->flags.indicator_type = MI_CHECK;
>
> - entry = wMenuAddCallback(menu, _("Keep Icon"),
> keepIconsCallback,
> NULL);
> - wfree(entry->text);
> - entry->text = _("Keep Icon");
> + entry = wMenuAddCallback(menu, _("Select All Icons"),
> selectIconsCallback, NULL);
> + wfree(entry->text);
> + entry->text = _("Select All Icons");
>
> - entry = wMenuAddCallback(menu, _("Move Icon To"), NULL, NULL);
> - wfree(entry->text);
> - entry->text = _("Move Icon To");
> - scr->clip_submenu = makeWorkspaceMenu(scr);
> - if (scr->clip_submenu)
> - wMenuEntrySetCascade(menu, entry, scr->clip_submenu);
> + entry = wMenuAddCallback(menu, _("Keep Icon"), keepIconsCallback,
> NULL);
> + wfree(entry->text);
> + entry->text = _("Keep Icon");
>
> - entry = wMenuAddCallback(menu, _("Remove Icon"),
> removeIconsCallback, NULL);
> - wfree(entry->text);
> - entry->text = _("Remove Icon");
> + entry = wMenuAddCallback(menu, _("Move Icon To"), NULL, NULL);
> + wfree(entry->text);
> + entry->text = _("Move Icon To");
> + scr->clip_submenu = makeWorkspaceMenu(scr);
> + if (scr->clip_submenu)
> + wMenuEntrySetCascade(menu, entry, scr->clip_submenu);
>
> - wMenuAddCallback(menu, _("Attract Icons"), colectIconsCallback,
> NULL);
> - }
> + entry = wMenuAddCallback(menu, _("Remove Icon"),
> removeIconsCallback, NULL);
> + wfree(entry->text);
> + entry->text = _("Remove Icon");
>
> + wMenuAddCallback(menu, _("Attract Icons"), colectIconsCallback,
> NULL);
> wMenuAddCallback(menu, _("Launch"), launchCallback, NULL);
> -
> wMenuAddCallback(menu, _("Unhide Here"), unhideHereCallback, NULL);
>
> entry = wMenuAddCallback(menu, _("Hide"), hideCallback, NULL);
> @@ -1021,11 +1035,29 @@ static WMenu *dockMenuCreate(WScreen *scr,
> int type)
> entry->text = _("Hide");
>
> wMenuAddCallback(menu, _("Settings..."), settingsCallback, NULL);
> -
> wMenuAddCallback(menu, _("Kill"), killCallback, NULL);
>
> - if (type == WM_CLIP)
> - scr->clip_menu = menu;
> + return menu;
> +}
> +
> +static WMenu *dockMenuCreate(WScreen *scr, int type)
> +{
> + WMenu *menu = NULL;
> +
> + switch (type) {
> + case WM_DOCK:
> + menu = create_dock_main_menu(scr);
> + break;
> + case WM_CLIP:
> + if (scr->clip_menu) {
> + /* If the common menu for Clips exists, use it */
> + menu = scr->clip_menu;
> + } else {
> + /* Else, create and save for other Clips */
> + menu = create_clip_main_menu(scr);
> + scr->clip_menu = menu;
> + }
> + }
>
> return menu;
> }
> --
> 1.7.10.4
>
>
> --
> To unsubscribe, send mail to
> [email protected].
>
--
To unsubscribe, send mail to [email protected].
