On Tue, 11 Mar 2008 16:03:37 +0100
Dominik Vogt <[EMAIL PROTECTED]> wrote:
[snip]
> See comments below.
Thanks for the extensive comments. I think it's all sorted now.
Most of the fvwm source is pretty new to me and sometimes it's hard
to find the way around. Your guidance has been helpful. If you feel
that something needs some polishing just tell me so and I will
investigate when I have the time and try to find the right way
to do the things.
>
> > diff -r -U5 fvwm/fvwm/menuitem.c fvwm/fvwm/menuitem.c
> > --- fvwm/fvwm/menuitem.c 2008-03-10 22:26:15.000000000 +0100
> > +++ fvwm/fvwm/menuitem.c 2008-03-10 21:49:15.000000000 +0100
> > @@ -388,10 +388,11 @@
> > Region region = None;
> > int i;
> > int sx1;
> > int sx2;
> > FlocaleFont* font;
> > + int top_margin = ST_VERTICAL_MARGIN_TOP(ms);
>
> I don't like initializing variables in the declaration. If you
> had compiled with -Wall -Werror you would have noticed that this
> variable is not used at all. Please remove it.
>
> > if (!mi)
> > {
> > return;
> > }
> > @@ -404,11 +405,11 @@
> > else
> > {
> > font = ST_PSTDFONT(ms);
> > }
> >
> > - y_offset = MI_Y_OFFSET(mi);
> > + y_offset = MI_Y_OFFSET(mi) + ST_VERTICAL_MARGIN_TOP(ms);
>
> This is not good. The vertical margin must be added when the
> MI_Y_OFFSET is set in the first place. There may be other places
> that rely on this value.
I moved it to menus.c. It now shold be ok for all cases.
The rest was just simple stuff that I overlooked or that needed some
cleaning.
New patch attached. If this is ok, do you agree to add a 3rd parameter
to add margins above AND below the separators?
--
Jesús Guerrero <[EMAIL PROTECTED]>
diff -r -U5 fvwm/fvwm/menus.c fvwm/fvwm/menus.c
--- fvwm/fvwm/menus.c 2008-03-11 17:09:42.000000000 +0100
+++ fvwm/fvwm/menus.c 2008-03-11 18:02:52.000000000 +0100
@@ -1601,11 +1603,11 @@
int separator_height;
separator_height = (last_item_has_relief) ?
MENU_SEPARATOR_HEIGHT + relief_thickness :
MENU_SEPARATOR_TOTAL_HEIGHT;
- MI_Y_OFFSET(mi) = y;
+ MI_Y_OFFSET(mi) = y + MST_VERTICAL_MARGIN_BOTTOM(msp->menu);
if (MI_IS_TITLE(mi))
{
MI_HEIGHT(mi) = MST_PTITLEFONT(msp->menu)->height +
MST_TITLE_GAP_ABOVE(msp->menu) +
MST_TITLE_GAP_BELOW(msp->menu);
@@ -1796,11 +1798,13 @@
if (MR_LAST_ITEM(msp->menu) != NULL &&
MI_IS_SELECTABLE(MR_LAST_ITEM(msp->menu)))
{
y += relief_thickness;
}
- MR_HEIGHT(msp->menu) = y + MST_BORDER_WIDTH(msp->menu);
+ MR_HEIGHT(msp->menu) = y + MST_BORDER_WIDTH(msp->menu)
+ + MST_VERTICAL_MARGIN_TOP(msp->menu)
+ + MST_VERTICAL_MARGIN_BOTTOM(msp->menu);
return has_continuation_menu;
}
/*
diff -r -U5 fvwm/fvwm/menustyle.c fvwm/fvwm/menustyle.c
--- fvwm/fvwm/menustyle.c 2008-03-11 17:09:44.000000000 +0100
+++ fvwm/fvwm/menustyle.c 2008-03-11 16:23:16.000000000 +0100
@@ -303,10 +303,31 @@
*below = val[1];
return;
}
+static void parse_vertical_margins_line(
+ char *args, signed char *top, signed char *bottom,
+ signed char top_default, signed char bottom_default)
+{
+ int val[2];
+
+ if (GetIntegerArguments(args, NULL, val, 2) != 2 ||
+ val[0] < 0 || val[0] > MAX_MENU_MARGIN ||
+ val[1] < 0 || val[1] > MAX_MENU_MARGIN)
+ {
+ /* illegal or missing parameters, return to default */
+ *top = top_default;
+ *bottom = bottom_default;
+ return;
+ }
+ *top = val[0];
+ *bottom = val[1];
+
+ return;
+}
+
static MenuStyle *menustyle_parse_old_style(F_CMD_ARGS)
{
char *buffer, *rest;
char *fore, *back, *stipple, *font, *style, *animated;
MenuStyle *ms = NULL;
@@ -404,10 +425,11 @@
"PopupIgnore", "PopupClose",
"MouseWheel", "ScrollOffPage",
"TrianglesUseFore",
"TitleColorset", "HilightTitleBack",
"TitleFont",
+ "VerticalMargins",
NULL
};
return GetTokenIndex(option, optlist, 0, NULL);
}
@@ -957,10 +979,12 @@
ST_DO_HILIGHT_BACK(tmpms) = 1;
ST_DO_HILIGHT_FORE(tmpms) = 1;
}
/* common settings */
+ ST_VERTICAL_MARGIN_TOP(tmpms) = 0;
+ ST_VERTICAL_MARGIN_BOTTOM(tmpms) = 0;
ST_CSET_MENU(tmpms) = 0;
ST_HAS_MENU_CSET(tmpms) = 0;
ST_CSET_ACTIVE(tmpms) = 0;
ST_HAS_ACTIVE_CSET(tmpms) = 0;
ST_CSET_GREYED(tmpms) = 0;
@@ -1565,11 +1589,16 @@
ST_PTITLEFONT(tmpms) = new_font;
ST_USING_DEFAULT_TITLEFONT(tmpms) = False;
}
has_gc_changed = True;
break;
-
+ case 62: /* VerticalMargins */
+ parse_vertical_margins_line(
+ args, &ST_VERTICAL_MARGIN_TOP(tmpms),
+ &ST_VERTICAL_MARGIN_BOTTOM(tmpms),
+ 0, 0);
+ break;
#if 0
case 99: /* PositionHints */
/* to be implemented */
break;
@@ -1741,10 +1770,13 @@
ST_HAS_TRIANGLE_RELIEF(destms) = ST_HAS_TRIANGLE_RELIEF(origms);
/* PopupDelayed */
ST_DO_POPUP_IMMEDIATELY(destms) = ST_DO_POPUP_IMMEDIATELY(origms);
/* DoubleClickTime */
ST_DOUBLE_CLICK_TIME(destms) = ST_DOUBLE_CLICK_TIME(origms);
+ /* VerticalMargins */
+ ST_VERTICAL_MARGIN_TOP(destms) = ST_VERTICAL_MARGIN_TOP(origms);
+ ST_VERTICAL_MARGIN_BOTTOM(destms) = ST_VERTICAL_MARGIN_BOTTOM(origms);
/* SidePic */
if (ST_SIDEPIC(destms))
{
PDestroyFvwmPicture(dpy, ST_SIDEPIC(destms));
diff -r -U5 fvwm/fvwm/menustyle.h fvwm/fvwm/menustyle.h
--- fvwm/fvwm/menustyle.h 2008-03-11 17:09:44.000000000 +0100
+++ fvwm/fvwm/menustyle.h 2008-03-11 16:13:20.000000000 +0100
@@ -171,10 +171,14 @@
#define MST_DOUBLE_CLICK_TIME(m) ((m)->s->ms->feel.DoubleClickTime)
#define ST_ITEM_FORMAT(s) ((s)->feel.item_format)
#define MST_ITEM_FORMAT(m) ((m)->s->ms->feel.item_format)
#define ST_SELECT_ON_RELEASE_KEY(s) ((s)->feel.select_on_release_key)
#define MST_SELECT_ON_RELEASE_KEY(m) ((m)->s->ms->feel.select_on_release_key)
+#define ST_VERTICAL_MARGIN_TOP(s) ((s)->look.vertical_margins.top)
+#define MST_VERTICAL_MARGIN_TOP(m) ((m)->s->ms->look.vertical_margins.top)
+#define ST_VERTICAL_MARGIN_BOTTOM(s) ((s)->look.vertical_margins.bottom)
+#define MST_VERTICAL_MARGIN_BOTTOM(m) ((m)->s->ms->look.vertical_margins.bottom)
/* ---------------------------- type definitions --------------------------- */
typedef enum
{
@@ -288,10 +292,15 @@
signed char separator_above;
signed char separator_below;
} vertical_spacing;
struct
{
+ unsigned char top;
+ unsigned char bottom;
+ } vertical_margins;
+ struct
+ {
int menu;
int active;
int greyed;
int title;
} cset;
diff -r -U5 fvwm/libs/defaults.h fvwm/libs/defaults.h
--- fvwm/libs/defaults.h 2008-03-11 17:09:46.000000000 +0100
+++ fvwm/libs/defaults.h 2008-03-11 16:21:12.000000000 +0100
@@ -87,10 +87,12 @@
#define DEFAULT_MENU_TITLE_TEXT_Y_OFFSET2 (DEFAULT_MENU_ITEM_TEXT_Y_OFFSET2)
/* minimum for above value */
#define MIN_VERTICAL_SPACING -100 /* pixels */
/* maximum for above value */
#define MAX_VERTICAL_SPACING 100 /* pixels */
+/* maximum for above value */
+#define MAX_MENU_MARGIN 50 /* pixels */
/* width of a tab in the item format of a menu */
#define MENU_TAB_WIDTH 8 /* spaces */
/* This is the tile width or height for V and H gradients. I guess this should
* better be a power of two. A value of 5 definitely causes XFree 3.3.3.1 to
* screw up the V_GRADIENT on an 8 bit display, but 4, 6, 7 etc. work well. */
