On Wed, 12 Mar 2008 01:38:04 +0100
Jesús Guerrero <[EMAIL PROTECTED]> wrote:
Hello, I quote my post about the separator margins that I wrote some
weeks ago. Just to bring it back to memory:
> Hello, to quote myself, from the VerticalMargins menustyle thread:
>
> "I hope to sort the separators padding stuff as well. So, if this patch is
> right, let me know your thoughts about that. But this is for another thread,
> so I will open a new one."
>
> So, I opened this new thread. My thoughts about this:
>
> If the VerticalMargins command is not the right place for this, then
> maybe a separate patch would do.
>
> If you think that it shouldn't be into the VerticalMargins stuff, maybe
> a new option would be right for that. Something like SeparatorMargins.
> This would need that we think about it a bit. I definitely want to add
> the possibility to have some padding around the separators because it
> doesn't seem right as it's now when the element above or below a separator
> is selected.
>
> If we get a SeparatorMargins option, it could be used to specify the
> margins not only above and below the separators, but also the margins
> on the left and the right. That would send SeparatorsLong and
> SeparatorsShort to the list of deprecated stuff (I never liked this
> couple to tell the truth).
>
> This would also add a greater degree of flexibility when defining
> menu separators. I don't think that separators would be a thing to
> write a thesis about, but well... We need the padding above and below,
> and adding the configurability for the separators length wouldn't add
> much complexity (I think, I haven't done any work on this yet).
>
> I'd like some feedback.
>
> Is this something that fvwm could use? I don't want to waste my time
> making patches that will only be used by me unless I really need them.
>
> If yes, is the SeparatorMargins approach correct?
>
> If so, would it be "SeparatorMargins l r t b" correct? or maybe just
> "SeparatorMargins top bottom"? In the first case, would it eventually
> turn SeparatorsLong and SeparatorsShort deprecated?
>
> I guess that if we want "l t r b" we will probably have some problems
> with ItemFormat, wouldn't we?
Since I did not receive any feedback, I took the easy way, and implemented
yet one more option for this. I'd like to know what do you think about it
and if it's a wanted feature on fvwm or not.
The patch is small enough and easy to understand. (Attached).
It adds the VerticalSeparatorMargins option (two arguments, much like
VerticalSeparators). The first one is the gap above the separators, the
second one is the gap below.
Both have a maximum value that is equal to MAX_MENU_MARGIN, no wonder, since
the function I use to parse the arguments is the same function that I used
on the VerticalMargins patch (I found really no reason to duplicate the code,
since both elements have exactly the same nature and the same function can
do the work). Just to refresh your minds, it's called
parse_vertical_margins_line()
The patch does a check around line ~1600 on menus.c which is used to determine
if the item we just procesed was a separator. In that case, the gap below is
added to y. The check also does that if the element is a title... It's
commented in the code. I don't feel that's right, maybe I should also add
another option to add a gap *after* the title underline(s).
My idea, as I explained in any other occasion, is being able to have a visible
separation between the selected menuitem and the separation lines (separators
or title underslines, it's the same). Because otherwise, it looks way too
cluttered.
I don't know if the current approach is right. Ideas are welcome.
Thanks everyone for reading.
--
Jesús Guerrero <[EMAIL PROTECTED]>
diff -U3 -r fvwm/fvwm/menus.c fvwm/fvwm/menus.c
--- fvwm/fvwm/menus.c 2008-03-18 13:17:40.000000000 +0100
+++ fvwm/fvwm/menus.c 2008-04-16 22:40:48.000000000 +0200
@@ -1644,7 +1644,8 @@
else if (MI_IS_SEPARATOR(mi))
{
/* Separator */
- MI_HEIGHT(mi) = separator_height;
+ MI_HEIGHT(mi) = separator_height +
+ MST_VERTICAL_SEPARATOR_MARGIN_TOP(msp->menu);
}
else if (MI_IS_TEAR_OFF_BAR(mi))
{
@@ -1716,6 +1717,13 @@
}
}
y += MI_HEIGHT(mi);
+ /* Adds the separator magin below the current element
+ if it's a separator, but also if it's a title element,
+ not sure if this is always desiderable though...*/
+ if (MI_IS_SEPARATOR(mi) || MI_IS_TITLE(mi))
+ {
+ y += MST_VERTICAL_SEPARATOR_MARGIN_BOTTOM(msp->menu);
+ }
/* this item would have to be the last item, or else
* we need to add a "More..." entry pointing to a new menu */
menu_height =
diff -U3 -r fvwm/fvwm/menustyle.c fvwm/fvwm/menustyle.c
--- fvwm/fvwm/menustyle.c 2008-03-17 00:01:03.000000000 +0100
+++ fvwm/fvwm/menustyle.c 2008-04-16 21:20:47.000000000 +0200
@@ -427,7 +427,7 @@
"TrianglesUseFore",
"TitleColorset", "HilightTitleBack",
"TitleFont",
- "VerticalMargins",
+ "VerticalMargins", "VerticalSeparatorMargins",
NULL
};
@@ -983,6 +983,8 @@
/* common settings */
ST_VERTICAL_MARGIN_TOP(tmpms) = 0;
ST_VERTICAL_MARGIN_BOTTOM(tmpms) = 0;
+ ST_VERTICAL_SEPARATOR_MARGIN_TOP(tmpms) = 0;
+ ST_VERTICAL_SEPARATOR_MARGIN_BOTTOM(tmpms) = 0;
ST_CSET_MENU(tmpms) = 0;
ST_HAS_MENU_CSET(tmpms) = 0;
ST_CSET_ACTIVE(tmpms) = 0;
@@ -1597,6 +1599,12 @@
&ST_VERTICAL_MARGIN_BOTTOM(tmpms),
0, 0);
break;
+ case 63: /* VerticalSeparatorMargins */
+ parse_vertical_margins_line(
+ args, &ST_VERTICAL_SEPARATOR_MARGIN_TOP(tmpms),
+ &ST_VERTICAL_SEPARATOR_MARGIN_BOTTOM(tmpms),
+ 0, 0);
+ break;
#if 0
case 99: /* PositionHints */
@@ -1775,6 +1783,9 @@
/* VerticalMargins */
ST_VERTICAL_MARGIN_TOP(destms) = ST_VERTICAL_MARGIN_TOP(origms);
ST_VERTICAL_MARGIN_BOTTOM(destms) = ST_VERTICAL_MARGIN_BOTTOM(origms);
+ /* VerticalSeparatorMargins */
+ ST_VERTICAL_SEPARATOR_MARGIN_TOP(destms) = ST_VERTICAL_SEPARATOR_MARGIN_TOP(origms);
+ ST_VERTICAL_SEPARATOR_MARGIN_BOTTOM(destms) = ST_VERTICAL_SEPARATOR_MARGIN_BOTTOM(origms);
/* SidePic */
if (ST_SIDEPIC(destms))
diff -U3 -r fvwm/fvwm/menustyle.h fvwm/fvwm/menustyle.h
--- fvwm/fvwm/menustyle.h 2008-03-17 00:01:03.000000000 +0100
+++ fvwm/fvwm/menustyle.h 2008-04-16 21:17:06.000000000 +0200
@@ -177,6 +177,10 @@
#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)
+#define ST_VERTICAL_SEPARATOR_MARGIN_TOP(s) ((s)->look.vertical_separator_margins.top)
+#define MST_VERTICAL_SEPARATOR_MARGIN_TOP(m) ((m)->s->ms->look.vertical_separator_margins.top)
+#define ST_VERTICAL_SEPARATOR_MARGIN_BOTTOM(s) ((s)->look.vertical_separator_margins.bottom)
+#define MST_VERTICAL_SEPARATOR_MARGIN_BOTTOM(m) ((m)->s->ms->look.vertical_separator_margins.bottom)
/* ---------------------------- type definitions --------------------------- */
@@ -299,6 +303,11 @@
} vertical_margins;
struct
{
+ unsigned char top;
+ unsigned char bottom;
+ } vertical_separator_margins;
+ struct
+ {
int menu;
int active;
int greyed;
