On Sun, Jan 25, 2009 at 11:28:51PM +0000, Thomas Adam wrote:
> 2009/1/22 Thomas Adam <[email protected]>:
> > 2009/1/22 Dominik Vogt <[email protected]>:
> >> On Thu, Jan 22, 2009 at 09:01:44AM +0000, Thomas Adam wrote:
> >>> 2009/1/22 Dominik Vogt <[email protected]>:
> >>> >  Style Foo MapFooFunc
> >>> >  AddToFunc MapFooFunc
> >>> >  + I MoveToPage 1 1
> >>> >  + I Iconify true
> >>> >
> >>> > This way, it would even be possible to get these stupid Open
> >>> > Office windows under control that shade themselves on startup.
> >>>
> >>> I am not following you here.  What's the relationship between
> >>> MapFooFunc and InitWindowCommand?
> >>
> >> Er, I wanted to type
> >>
> >>  Style Foo InitWindowCommand MapFooFunc
> >
> > Ah ha.  And suddenly all becomes clear.
> >
> > I shall come up with something soon enough for review.  Should be fun.
> 
> And so I have. I've made "MapCommand" accept anything (i.e., a valid
> FVWM builtin, or a defined function)

Thanks, I'll try out the patch soon.

> and in so doing have done the following:
> 
> * Deprecate StartsRaised in favour of:  Style foo MapCommand Raise
> * Deprecate StartsLowered in favout of:  Style foo MapCommand Lower
> * Deprecate StartIconic in favour of:  Style foo MapCommand Iconify
> * Deprecate StartNormal in favour of:  Style foo MapCommand Iconify off
> * Deprecate StartShaded in favour of:  Style foo MapCommand WindowShade dir
> 
> I've also deprecated StartsOnPage/Desk/Screen in favour of:

I'm unsure whether this is a good thing to do.  There should be a
consistent way of having many commands as styles, including all
arguments, but the MapCommand is not a general solution.  For
example, with MapCommand windows will flash on the current screen
first and be moved to the page specified with the "StartsOnPage"
substitute only after that.  So deprecating the styles would
actually impair the functionality.

> Style foo MapWindow StartsOnLocation [screen x] a b c
> 
> Where "StartsOnLocation" works exactly like StartsOnPage, but now
> accepts an optional "screen x" parameter for specifying a starting
> screen.
> 
> I am considering the attached patch to be nothing more than a
> proof-of-concept, so testing/feedback is welcome, I daresay it's a
> little rough around the edges, and I expect that.
> 
> Perhaps the only "damaging" change is in deprecating the
> aforementioned options, I really have done just that -- i.e., there
> really is no further support for them other than a standard
> deprecation message should FVWM find one of these style commands.  I
> realise this breaks any chance of backwards compatability, but it
> allows for removing code no longer needed if this MapCommand is
> accepted.
> 
> Because this is a proof-of-concept patch, I've deliberately not
> written any documentation yet until I get some feedback.
> 
> Kindly,
> 
> -- Thomas Adam

> Index: fvwm/add_window.c
> ===================================================================
> RCS file: /home/cvs/fvwm/fvwm/fvwm/add_window.c,v
> retrieving revision 1.398
> diff -u -r1.398 add_window.c
> --- fvwm/add_window.c 6 Aug 2008 17:55:28 -0000       1.398
> +++ fvwm/add_window.c 25 Jan 2009 22:39:24 -0000
> @@ -2436,7 +2436,12 @@
>       BroadcastWindowIconNames(fw, True, False);
>  
>       /****** place the window in the stack ring ******/
> -     if (!position_new_window_in_stack_ring(fw, SDO_START_LOWERED(sflags)))
> +     /* TA:  20090124:  We don't explicitly lower the window -- leave it in
> +      * the default layer.  This can be overrided by:
> +      *
> +      * MapCommand Lower
> +      */
> +     if (!position_new_window_in_stack_ring(fw, 0))
>       {
>               XWindowChanges xwc;
>               xwc.sibling = FW_W_FRAME(get_next_window_in_stack_ring(fw));
> @@ -2535,7 +2540,7 @@
>       GNOME_SetWinArea(fw);
>  
>       /****** windowshade ******/
> -     if (state_args.do_shade || SDO_START_SHADED(sflags))
> +     if (state_args.do_shade)
>       {
>               rectangle big_g;
>               rectangle new_g;
> @@ -2545,13 +2550,6 @@
>               {
>                       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 (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/events.c
> ===================================================================
> RCS file: /home/cvs/fvwm/fvwm/fvwm/events.c,v
> retrieving revision 1.561
> diff -u -r1.561 events.c
> --- fvwm/events.c     8 Aug 2008 11:44:01 -0000       1.561
> +++ fvwm/events.c     25 Jan 2009 22:39:26 -0000
> @@ -2975,6 +2975,12 @@
>       else
>       {
>               int state;
> +             
> +             /* TA:  20090125:  Make sure we're able to access the window
> +              * flags post-mapping.
> +              */
> +             window_style style;
> +             lookup_style(fw, &style);
>  
>               if (fw->wmhints && (fw->wmhints->flags & StateHint))
>               {
> @@ -3055,6 +3061,16 @@
>                                       (unsigned long)fw);
>  #endif
>                       }
> +                     /* TA:  20090125:  We *have* to handle MapCommand
> +                      * here and not in AddWindow() to allow for correct
> +                      * timings when the window is truly mapped. (c.f.
> +                      * things like Iconify.
> +                      */
> +                     if( style.flags.has_map_command_string)
> +                     {
> +                             execute_function_override_window(NULL, NULL,
> +                                     SGET_MAP_COMMAND_STRING(style), 0, fw);
> +                     }
>                       MyXUngrabServer(dpy);
>                       break;
>  
> Index: fvwm/fvwm.h
> ===================================================================
> RCS file: /home/cvs/fvwm/fvwm/fvwm/fvwm.h,v
> retrieving revision 1.259
> diff -u -r1.259 fvwm.h
> --- fvwm/fvwm.h       28 Nov 2008 23:29:27 -0000      1.259
> +++ fvwm/fvwm.h       25 Jan 2009 22:39:27 -0000
> @@ -594,6 +594,7 @@
>       unsigned has_placement_penalty : 1;
>       unsigned has_placement_percentage_penalty : 1;
>       unsigned has_placement_position_string : 1;
> +     unsigned has_map_command_string : 1;
>  } style_flags;
>  
>  typedef struct style_id_t
> @@ -679,6 +680,7 @@
>       pl_penalty_struct pl_penalty;
>       pl_percent_penalty_struct pl_percent_penalty;
>       char *pl_position_string;
> +     char *map_command_string;
>       style_flags flags;
>       style_flags flag_default;
>       style_flags flag_mask;
> Index: fvwm/placement.c
> ===================================================================
> RCS file: /home/cvs/fvwm/fvwm/fvwm/placement.c,v
> retrieving revision 1.169
> diff -u -r1.169 placement.c
> --- fvwm/placement.c  12 Aug 2008 17:19:06 -0000      1.169
> +++ fvwm/placement.c  25 Jan 2009 22:39:28 -0000
> @@ -2200,10 +2200,7 @@
>               reason.screen.screen = SGET_START_SCREEN(*pstyle);
>       }
>       __place_handle_x_resources(fw, pstyle, &reason);
> -     if (pstyle->flags.do_start_iconic)
> -     {
> -             win_opts->initial_state = IconicState;
> -     }
> +     
>       ecc.type = EXCT_NULL;
>       ecc.w.fw = fw;
>       exc = exc_create_context(&ecc, ECC_TYPE | ECC_FW);
> Index: fvwm/style.c
> ===================================================================
> RCS file: /home/cvs/fvwm/fvwm/fvwm/style.c,v
> retrieving revision 1.260
> diff -u -r1.260 style.c
> --- fvwm/style.c      19 Oct 2008 12:04:13 -0000      1.260
> +++ fvwm/style.c      25 Jan 2009 22:39:31 -0000
> @@ -706,6 +706,14 @@
>                       *merged_style,
>                       strdup(SGET_PLACEMENT_POSITION_STRING(*add_style)));
>       }
> +     if (add_style->flags.has_map_command_string)
> +     {
> +             SAFEFREE(SGET_MAP_COMMAND_STRING(*merged_style));
> +             SSET_MAP_COMMAND_STRING(
> +                     *merged_style,
> +                     strdup(SGET_MAP_COMMAND_STRING(*add_style)));
> +
> +     }
>       /* merge the style flags */
>  
>       /*** ATTENTION:
> @@ -761,6 +769,7 @@
>       SAFEFREE(SGET_MINI_ICON_NAME(*style));
>       remove_icon_boxes_from_style(style);
>       SAFEFREE(SGET_PLACEMENT_POSITION_STRING(*style));
> +     SAFEFREE(SGET_MAP_COMMAND_STRING(*style));
>  
>       return;
>  }
> @@ -3091,6 +3100,131 @@
>                       ps->change_mask.manual_placement_honors_starts_on_page
>                               = 1;
>               }
> +             else if (StrEquals(token, "MapCommand"))
> +             {
> +                     /*  (TA:  20090123:  We accept any valid fvwm command
> +                      *  here.  But as a special case, we must check for
> +                      *  the new "StartsOnLocation" command which is
> +                      *  exactly the same as StartsOnPage, but now adds in
> +                      *  a screen parameter as optional.  In doing so this
> +                      *  deprecates StartsOnDesk and StartsOnScreen.
> +                      */
> +
> +                     char *fullCommand = rest;
> +                     char *ret_rest;
> +                     token = PeekToken(rest, &rest);
> +
> +                     /* If we match "StartsOnLocation" then parse this.
> +                      * Anything else is assumed as a legitimate command.
> +                      */
> +
> +                     if (StrEquals(token, "StartsOnLocation"))
> +                     {
> +                             /* Example:
> +                              *
> +                              * Style foo StartsOnLocation screen 1 0 0 0
> +                              *
> +                              * Places "foo" on screen 1, desk 0, page 0 0.
> +                              */
> +
> +                             /* Match "screen" as an optional parameter. */
> +                             if (MatchToken(rest, "screen"))
> +                                     token = PeekToken(rest, &rest);
> +
> +                             if (token && StrEquals(token, "screen"))
> +                             {
> +                                     token = PeekToken(rest, &rest);
> +
> +                                     if (token)
> +                                     {
> +                                             tmpno[0] = 
> FScreenGetScreenArgument(token, 'c');
> +                                             ps->flags.use_start_on_screen = 
> 1;
> +                                             
> ps->flag_mask.use_start_on_screen = 1;
> +                                             
> ps->change_mask.use_start_on_screen = 1;
> +                                             SSET_START_SCREEN(*ps, 
> tmpno[0]);
> +                                     }
> +                                     else
> +                                     {
> +                                             ps->flags.use_start_on_screen = 
> 0;
> +                                             
> ps->flag_mask.use_start_on_screen = 1;
> +                                             
> ps->change_mask.use_start_on_screen = 1;
> +                                     }
> +                             }
> +
> +                             spargs = GetIntegerArguments(
> +                                             rest, &ret_rest, tmpno, 3);
> +
> +                             if (spargs == 1 || spargs == 3)
> +                             {
> +                                     /* We have a desk no., with or without 
> page. */
> +                                     /* RBW - 11/20/1998 - allow for the 
> special
> +                                      * case of -1 */
> +                                     /* Desk is now actual + 1 */
> +                                     SSET_START_DESK(
> +                                                     *ps, (tmpno[0] > -1) ?
> +                                                     tmpno[0] + 1 : 
> tmpno[0]);
> +                             }
> +                             if (spargs == 2 || spargs == 3)
> +                             {
> +                                     if (spargs == 3)
> +                                     {
> +                                             /*  RBW - 11/20/1998 - allow 
> for the
> +                                              * special case of -1  */
> +                                             SSET_START_PAGE_X(
> +                                                             *ps, (tmpno[1] 
> > -1) ?
> +                                                             tmpno[1] + 1 : 
> tmpno[1]);
> +                                             SSET_START_PAGE_Y(
> +                                                             *ps, (tmpno[2] 
> > -1) ?
> +                                                             tmpno[2] + 1 : 
> tmpno[2]);
> +                                     }
> +                                     else
> +                                     {
> +                                             SSET_START_PAGE_X(
> +                                                             *ps, (tmpno[0] 
> > -1) ?
> +                                                             tmpno[0] + 1 : 
> tmpno[0]);
> +                                             SSET_START_PAGE_Y(
> +                                                             *ps, (tmpno[1] 
> > -1) ?
> +                                                             tmpno[1] + 1 : 
> tmpno[1]);
> +                                     }
> +                             }
> +                             if (spargs < 1 || spargs > 3)
> +                             {
> +                                     fvwm_msg(ERR, 
> "style_parse_one_style_option",
> +                                                     "bad StartsOnLocation 
> args: %s",
> +                                                     rest);
> +                             }
> +                             else
> +                             {
> +                                     /* Make sure the window gets placed
> +                                      * where expected.
> +                                      */
> +                                     ps->flags.use_start_on_desk = 1;
> +                                     ps->flag_mask.use_start_on_desk = 1;
> +                                     ps->change_mask.use_start_on_desk = 1;
> +
> +                                     /* And since we've only got one
> +                                      * command, we aren't interested in
> +                                      * anything else as there's nothing to
> +                                      * run.
> +                                      */
> +                                     ps->flags.has_map_command_string = 0;
> +                                     ps->flag_mask.has_map_command_string = 
> 0;
> +                                     ps->change_mask.has_map_command_string 
> = 0;
> +                             }
> +                     }
> +                     else
> +                     {
> +                             char *s;
> +                             s = (fullCommand != NULL) ? strdup(fullCommand) 
> : NULL;
> +                             fullCommand = NULL; /* consume the entire 
> string */
> +                             SSET_MAP_COMMAND_STRING(*ps, s);
> +
> +                             ps->flags.has_map_command_string = 1;
> +                             ps->flag_mask.has_map_command_string = 1;
> +                             ps->change_mask.has_map_command_string = 1;
> +                     }
> +                     rest = NULL;
> +             }
>               else if (StrEquals(token, "Maximizable"))
>               {
>                       S_SET_IS_UNMAXIMIZABLE(SCF(*ps), !on);
> @@ -3705,15 +3839,21 @@
>               }
>               else if (StrEquals(token, "StartIconic"))
>               {
> -                     ps->flags.do_start_iconic = on;
> -                     ps->flag_mask.do_start_iconic = 1;
> -                     ps->change_mask.do_start_iconic = 1;
> +                     fvwm_msg(OLD, "style_parse_one_style_option",
> +                             "The command StartsOnScreen is obsolete. " 
> +                             "Please use the following style option:"
> +                             "\n\n"
> +                             "MapCommand Iconify");
> +                     rest = NULL;
>               }
>               else if (StrEquals(token, "StartNormal"))
>               {
> -                     ps->flags.do_start_iconic = !on;
> -                     ps->flag_mask.do_start_iconic = 1;
> -                     ps->change_mask.do_start_iconic = 1;
> +                     fvwm_msg(OLD, "style_parse_one_style_option",
> +                             "The command StartsOnScreen is obsolete. " 
> +                             "Please use the following style option:"
> +                             "\n\n"
> +                             "MapCommand Iconify off");
> +                     rest = NULL;
>               }
>               else if (StrEquals(token, "StaysOnBottom"))
>               {
> @@ -3784,23 +3924,12 @@
>               else if (StrEquals(token, "STARTSONDESK"))
>               {
>                       spargs = GetIntegerArguments(rest, NULL, tmpno, 1);
> -                     if (spargs == 1)
> -                     {
> -                             PeekToken(rest,&rest);
> -                             ps->flags.use_start_on_desk = 1;
> -                             ps->flag_mask.use_start_on_desk = 1;
> -                             ps->change_mask.use_start_on_desk = 1;
> -                             /*  RBW - 11/20/1998 - allow for the special
> -                              * case of -1  */
> -                             SSET_START_DESK(
> -                                     *ps, (tmpno[0] > -1) ?
> -                                     tmpno[0] + 1 : tmpno[0]);
> -                     }
> -                     else
> -                     {
> -                             fvwm_msg(ERR,"style_parse_one_style_option",
> -                                      "bad StartsOnDesk arg: %s", rest);
> -                     }
> +                     fvwm_msg(OLD, "style_parse_one_style_option",
> +                             "The command StartsOnScreen is obsolete. " 
> +                             "Please use the following style option:"
> +                             "\n\n"
> +                             "MapCommand StartsOnLocation %d", tmpno[0]);
> +                     rest = NULL;
>               }
>               /* StartsOnPage is like StartsOnDesk-Plus */
>               else if (StrEquals(token, "STARTSONPAGE"))
> @@ -3808,51 +3937,14 @@
>                       char *ret_rest;
>                       spargs = GetIntegerArguments(rest, &ret_rest,
>                                                    tmpno, 3);
> -                     if (spargs == 1 || spargs == 3)
> -                     {
> -                             /* We have a desk no., with or without page. */
> -                             /* RBW - 11/20/1998 - allow for the special
> -                              * case of -1 */
> -                             /* Desk is now actual + 1 */
> -                             SSET_START_DESK(
> -                                     *ps, (tmpno[0] > -1) ?
> -                                     tmpno[0] + 1 : tmpno[0]);
> -                     }
> -                     if (spargs == 2 || spargs == 3)
> -                     {
> -                             if (spargs == 3)
> -                             {
> -                                     /*  RBW - 11/20/1998 - allow for the
> -                                      * special case of -1  */
> -                                     SSET_START_PAGE_X(
> -                                             *ps, (tmpno[1] > -1) ?
> -                                             tmpno[1] + 1 : tmpno[1]);
> -                                     SSET_START_PAGE_Y(
> -                                             *ps, (tmpno[2] > -1) ?
> -                                             tmpno[2] + 1 : tmpno[2]);
> -                             }
> -                             else
> -                             {
> -                                     SSET_START_PAGE_X(
> -                                             *ps, (tmpno[0] > -1) ?
> -                                             tmpno[0] + 1 : tmpno[0]);
> -                                     SSET_START_PAGE_Y(
> -                                             *ps, (tmpno[1] > -1) ?
> -                                             tmpno[1] + 1 : tmpno[1]);
> -                             }
> -                     }
> -                     if (spargs < 1 || spargs > 3)
> -                     {
> -                             fvwm_msg(ERR, "style_parse_one_style_option",
> -                                      "bad StartsOnPage args: %s", rest);
> -                     }
> -                     else
> -                     {
> -                             ps->flags.use_start_on_desk = 1;
> -                             ps->flag_mask.use_start_on_desk = 1;
> -                             ps->change_mask.use_start_on_desk = 1;
> -                     }
> -                     rest = ret_rest;
> +                     fvwm_msg(OLD, "style_parse_one_style_option",
> +                             "The command StartsOnScreen is obsolete. " 
> +                             "Please use the following style option:"
> +                             "\n\n"
> +                             "MapCommand StartsOnLocation %d %d %d", 
> +                             tmpno[0], tmpno[1], tmpno[2]);
> +                     rest = NULL;
> +
>               }
>               else if (StrEquals(token, "STARTSONPAGEINCLUDESTRANSIENTS"))
>               {
> @@ -3868,21 +3960,13 @@
>               }
>               else if (StrEquals(token, "StartsOnScreen"))
>               {
> -                     if (rest)
> -                     {
> -                             tmpno[0] = FScreenGetScreenArgument(rest, 'c');
> -                             PeekToken(rest,&rest);
> -                             ps->flags.use_start_on_screen = 1;
> -                             ps->flag_mask.use_start_on_screen = 1;
> -                             ps->change_mask.use_start_on_screen = 1;
> -                             SSET_START_SCREEN(*ps, tmpno[0]);
> -                     }
> -                     else
> -                     {
> -                             ps->flags.use_start_on_screen = 0;
> -                             ps->flag_mask.use_start_on_screen = 1;
> -                             ps->change_mask.use_start_on_screen = 1;
> -                     }
> +                     fvwm_msg(OLD, "style_parse_one_style_option",
> +                             "The command StartsOnScreen is obsolete. " 
> +                             "Please use the following style option:"
> +                             "\n\n"
> +                             "MapCommand StartsOnLocation screen %s", 
> +                             rest);
> +                     rest = NULL;
>               }
>               else if (StrEquals(token, "STARTSANYWHERE"))
>               {
> @@ -3892,46 +3976,34 @@
>               }
>               else if (StrEquals(token, "STARTSLOWERED"))
>               {
> -                     ps->flags.do_start_lowered = on;
> -                     ps->flag_mask.do_start_lowered = 1;
> -                     ps->change_mask.do_start_lowered = 1;
> +                     fvwm_msg(OLD, "style_parse_one_style_option",
> +                             "The command StartsLowered is obsolete. " 
> +                             "Please use the following style option:"
> +                             "\n\n"
> +                             "MapCommand Lower");
> +                     rest = NULL;
>               }
>               else if (StrEquals(token, "STARTSRAISED"))
>               {
> -                     ps->flags.do_start_lowered = !on;
> -                     ps->flag_mask.do_start_lowered = 1;
> -                     ps->change_mask.do_start_lowered = 1;
> +                     fvwm_msg(OLD, "style_parse_one_style_option",
> +                             "The command StartsRaised is obsolete. " 
> +                             "Please use the following style option:"
> +                             "\n\n"
> +                             "MapCommand Raise");
> +                     rest = NULL;
>               }
>               else if (StrEquals(token, "StartShaded"))
>               {
>                       token = PeekToken(rest, &rest);
> +                     
> +                     fvwm_msg(OLD, "style_parse_one_style_option",
> +                             "The command StartShaded is obsolete. " 
> +                             "Please use the following style option:"
> +                             "\n\n"
> +                             "MapCommand WindowShade %s", 
> +                             token ? token : "");
> +                     rest = NULL;
>  
> -                     if (token)
> -                     {
> -                             direction_t direction;
> -
> -                             direction = gravity_parse_dir_argument(
> -                                     token, &token, DIR_NONE);
> -                             if (direction >= 0 && direction <= DIR_MASK)
> -                             {
> -                                     SSET_STARTS_SHADED_DIR(*ps, direction);
> -                             }
> -                             else
> -                             {
> -                                     fvwm_msg(
> -                                             ERR,
> -                                             "style_parse_one_style_option",
> -                                             "Option: %s is not valid with"
> -                                             " StartShaded", token);
> -                             }
> -                     }
> -                     else
> -                     {
> -                             SSET_STARTS_SHADED_DIR(*ps, DIR_N);
> -                     }
> -                     ps->flags.do_start_shaded = on;
> -                     ps->flag_mask.do_start_shaded = 1;
> -                     ps->change_mask.do_start_shaded = 1;
>               }
>               else if (StrEquals(token, "SaveUnder"))
>               {
> Index: fvwm/style.h
> ===================================================================
> RCS file: /home/cvs/fvwm/fvwm/fvwm/style.h,v
> retrieving revision 1.89
> diff -u -r1.89 style.h
> --- fvwm/style.h      17 Nov 2007 11:47:57 -0000      1.89
> +++ fvwm/style.h      25 Jan 2009 22:39:32 -0000
> @@ -15,10 +15,6 @@
>       ((sf)->do_decorate_transient)
>  #define SDO_SAVE_UNDER(sf) \
>       ((sf)->do_save_under)
> -#define SDO_START_LOWERED(sf) \
> -     ((sf)->do_start_lowered)
> -#define SDO_START_SHADED(sf) \
> -     ((sf)->do_start_shaded)
>  #define SHAS_BORDER_WIDTH(sf) \
>       ((sf)->has_border_width)
>  #define SHAS_COLOR_BACK(sf) \
> @@ -657,6 +653,10 @@
>       ((s).pl_position_string)
>  #define SSET_PLACEMENT_POSITION_STRING(s,x)  \
>       ((s).pl_position_string = (x))
> +#define SGET_MAP_COMMAND_STRING(s) \
> +     ((s).map_command_string)
> +#define SSET_MAP_COMMAND_STRING(s,x) \
> +     ((s).map_command_string = (x))
>  
>  /* function prototypes */
>  void lookup_style(FvwmWindow *fw, window_style *styles);



Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt

Reply via email to