On Fri, Aug 31, 2007 at 11:32:22PM +0100, Thomas Adam wrote: > On Fri, Aug 31, 2007 at 12:36:36PM +0200, Dominik Vogt wrote: > > On Fri, Aug 31, 2007 at 11:26:03AM +0100, Thomas Adam wrote: > > > On 31/08/2007, Dominik Vogt <[EMAIL PROTECTED]> wrote: > > > > On Thu, Aug 30, 2007 at 09:32:15PM +0100, Thomas Adam wrote: > > > > 2) There is no NEWS entry for the new featur. > > > > > > I'll add one if it's something you think you'll add. > > > > I certainly will commit the patch. Never liked that FvwmEvent > > stuff myself. > > Hopefully I've implemented your suggestions properly. The only thing I did > struggle with was the damn .xml file. > > See attached file.
I'll apply the patch with a couple more changes. The most
important one is indentation. We always use tabs of width 8. If
a patch has a different style of indentation, it's a lot of work
to adapt it.
More comments inline.
> @@ -2489,6 +2489,14 @@
> {
> state_args.shade_dir = GET_TITLE_DIR(fw);
> }
> +
> + /* If we've set a style for StartShaded, ensure we override
> + * the state for it here. -- TA.
> + */
> + if (SGET_STARTS_SHADED_DIR(style) != DIR_NONE)
1) You can't use DIR_NONE here because it's equivalent to all bits
set = DIR_NW. That's what we have the do_start_shaded flag
for.
2) When we restart, we don't want to override the current shading
direction with the default one. The correct condition is
if (SDO_START_SHADED(sflags) && !state_args.do_shade)
> + {
> + state_args.shade_dir = SGET_STARTS_SHADED_DIR(style);
> + }
> big_g = (IS_MAXIMIZED(fw)) ? fw->g.max : fw->g.frame;
> new_g = big_g;
> get_shaded_geometry_with_dir(
> Index: fvwm/style.c
> ===================================================================
> RCS file: /home/cvs/fvwm/fvwm/fvwm/style.c,v
> retrieving revision 1.256
> diff -u -r1.256 style.c
> --- fvwm/style.c 3 Jun 2007 09:28:34 -0000 1.256
> +++ fvwm/style.c 31 Aug 2007 22:28:37 -0000
> @@ -585,6 +585,11 @@
> {
> SSET_LAYER(*merged_style, SGET_LAYER(*add_style));
> }
> + if (add_style->flags.do_start_shaded)
> + {
> + SSET_STARTS_SHADED_DIR(*merged_style,
> + SGET_STARTS_SHADED_DIR(*add_style));
> + }
Note that you used spaced for indentation here.
> @@ -3784,6 +3789,35 @@
> ps->flag_mask.do_start_lowered = 1;
> ps->change_mask.do_start_lowered = 1;
> }
> + else if (StrEquals(token, "StartShaded"))
dito
> + if (direction >= 0 && direction <=
> + DIR_MINOR_MASK)
I replaced DIR_MINOR_MASK with just DIR_MASK.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
signature.asc
Description: Digital signature
