On Tue, Mar 11, 2008 at 02:37:16PM +0100, Jesús Guerrero wrote:
> On Tue, 11 Mar 2008 14:12:48 +0100
> Dominik Vogt <[EMAIL PROTECTED]> wrote:
>
> > On Tue, Mar 11, 2008 at 07:31:40AM +0100, Jesús Guerrero wrote:
> > > Hello,
> > >
> > > I made a new patch which solves one of the longstanding issues I had
> > > with my fvwm menus. It's not that it's something important, but it
> > > was a small annoyance.
> >
> > ... and where is the patch?
>
> Well, I will attach it to this mail. But, as I said, it's not a finished
> product. That's why I didn't include it on the first post, but since you
> asked... It basically works, but I need to test it extensively to make
> sure that nothing strange happens. At the current state, it only support
> the top and bottom margins. I will implement margins around separators
> probably later today if I can.
>
> The docs and stuff is not done. I don't want to waste my time documenting
> it if it's never going to be merged and I am the only one using it. But
> I will gladly make the docs and test stuff if you think that this would
> be a good addition for fvwm.
>
> The patch works as follows:
>
> MenuStyle * VerticalMargins x y
>
> Where x is the top margin and y is the bottom one. As said, margins around
> separators are still not implemented, though I think they would be a good
> addition. The separator lines are just next the the selection area, and
> that makes them look really bad.
See comments below.
> 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.
> y_height = MI_HEIGHT(mi);
> if (MI_IS_SELECTABLE(mi))
> {
> text_y = y_offset + MDIM_ITEM_TEXT_Y_OFFSET(*dim);
> }
> diff -r -U5 fvwm/fvwm/menus.c fvwm/fvwm/menus.c
> --- fvwm/fvwm/menus.c 2008-03-10 22:26:30.000000000 +0100
> +++ fvwm/fvwm/menus.c 2008-03-10 21:23:52.000000000 +0100
> @@ -126,10 +126,12 @@
> } max;
> struct
> {
> unsigned is_popup_indicator_used : 1;
> } flags;
> + unsigned char top_margin;
> + unsigned char bottom_margin;
> } MenuSizingParameters;
>
> typedef enum
> {
> SCTX_MENU_ROOT,
> @@ -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;
> }
>
> /*
Looks good.
> diff -r -U5 fvwm/fvwm/menustyle.c fvwm/fvwm/menustyle.c
> --- fvwm/fvwm/menustyle.c 2008-03-10 22:26:34.000000000 +0100
> +++ fvwm/fvwm/menustyle.c 2008-03-10 22:14:22.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] < MIN_VERTICAL_SPACING || val[0] > MAX_VERTICAL_SPACING ||
> + val[1] < MIN_VERTICAL_SPACING || val[1] > MAX_VERTICAL_SPACING)
MAX_VERTICAL_SPACING and MIN_VERTICAL_SPACING are not defined in
the patch. Anyway, MIN_VERTICAL_SPACING should be just 0 and
MAX_VERTICAL_SPACING the same value that is used for
MAX_MENU_BORDER_WIDTH.
> + {
> + /* 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);
> }
> @@ -906,11 +928,11 @@
> while (poption[0] == '!')
> {
> on ^= 1;
> poption++;
> }
> - switch((i = menustyle_get_styleopt_index(poption)))
> + switch(i = menustyle_get_styleopt_index(poption))
Please undo this change. Gcc emits a warning if the parentheses
are omitted.
> {
> case 0: /* fvwm */
> case 1: /* mwm */
> case 2: /* win */
> if (i == 0)
> @@ -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;
Please re-indent this block with tabs.
>
> #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-10 22:26:35.000000000 +0100
> +++ fvwm/fvwm/menustyle.h 2008-03-10 18:48:01.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
> {
> + signed char top;
> + signed char bottom;
These should be unsigned char.
> + } vertical_margins;
> + struct
> + {
> int menu;
> int active;
> int greyed;
> int title;
> } cset;
Ciao
Dominik ^_^ ^_^
P.S.: Ich lese E-Post nur zweimal täglich.
--
Dominik Vogt
