Subject: Patch for possible "if-else" syntax

From: "Dmitry Yu. Bolkhovityanov" <[email-masked]>
Date: Wed, 8 Aug 2001 00:27:18 +0700

Hi!

    I've made a patch which implements a simple "if-else" syntax.

    A new "Otherwise" command is introduced (wouldn't clash with possible
future syntax enhancements, right?), which executes supplied command only if
previous command was unsuccessful.  So the syntax is:

AddToFunc "Do-this-or-that"
+ "I" Next (SomeProggie) Close
+ "I" Otherwise SomeProggie

(this implements Dominik's example of "if some window exists, kill it,
otherwise start it").

    Each command may return a True/False value, which defaults to "True".
This logic is implemented in execute_complex_function(), which sets the
"current return value" to True before calling a command.  There's no change
in command calling conventions, since "return value of this command" and
"return value of previous command" are global variables (sic!).  Changed are
only conditional commands, which set return value to False when
unsuccessful.  I also investigated if it would be possible to place these
two vars into exec_func_args_type, but those structs aren't passed between
*execute*function().

    Since return values are stored in global vars,
execute_complex_function() saves previous state and restores it on exit.
Unfortunately, it contained a number of returns, which had to be replaced
with "goto END_OF_FUNCTION" (I wish C had Ada/Perl "break from a block"
feature...).

    Of course, calling "Otherwise" outside a complex function is senseless.

    Man page modification isn't included.

       ___________________________________________________________________
       Dmitry Yu. Bolkhovityanov  |  Novosibirsk, RUSSIA
       phone (383-2)-39-49-56     |  The Budker Institute of Nuclear Physics
                                  |  Lab. 5-13

This message contains a file prepared for transmission using the
MIME BASE64 transfer encoding scheme. If you are using Pegasus
Mail or another MIME-compliant system, you should be able to extract
it from within your mailer. If you cannot, please ask your system
administrator for help.

   ---- File information -----------
     File:  OTHERW.DIF
     Date:  7 Aug 2001, 23:04
     Size:  7906 bytes.
     Type:  Binary

---

From: Mikhael Goikhman <[email-masked]>
Date: Wed, 8 Aug 2001 04:45:34 +0000

>     Of course, calling "Otherwise" outside a complex function is senseless.

Grr... :)

Before reading the mail I did my own patch that happens to be almost
equivalent to yours, except for minor things. Say, I used interface
functions instead of global variables (there is only one static variable).
Also in my intention, functions may return more than just "success" or
"failure". We may have in the future such "failure" codes like: "timeout",
"window mismatch", "timeout", "incorrect usage", and the Otherwise command
may have an optional parameter to distinguish between different failures.

In the future all CMD_* functions may return this value, that is "success"
in most of cases. Currently it is not needed to minimize changes.

I called the function Else, because I hoped to have If in the future (for
non-window conditions). But I have no objections to call it Otherwise.

In any way, this solution is more clean and extendible than the Dominik's
one and it does not change any existing syntax. So, if there are no
objections I will apply the merged patch implementing this new command.

And BTW, it has sense (not yet) to call this command outside of functions.

  Read myfile1 quiet
  Otherwise Read myfile2 quiet
  Otherwise Echo Error: No myfile1 or myfile2 found

Also, it should be noted that the return value is of the entire command,
not of the first or last sub-command, so the following always prints "No"
regardless of absence or presence of terminal windows:

  Next ("XTerm") None ("XTerm") Echo Yes
  Otherwise Echo No

The entire first command fails because either Next or None fails.

Regards,
Mikhael.
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Dominik Vogt <[email-masked]>
Date: Wed, 8 Aug 2001 14:53:55 +0200

On Wed, Aug 08, 2001 at 12:27:18AM +0700, Dmitry Yu. Bolkhovityanov wrote:
Content-Description: Mail message body
>     Man page modification isn't included.

Is it really necessary to pass function return codes through
global variables?  If we do this now we will regret it some day.

Bye

Dominik ^_^  ^_^

-- 
Dominik Vogt, email: [email-masked]
LifeBits Aktiengesellschaft, Albrechtstr. 9, D-72072 Tuebingen
fon: ++49 (0) 7071/7965-0, fax: ++49 (0) 7071/7965-20
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Dominik Vogt <[email-masked]>
Date: Wed, 8 Aug 2001 14:57:41 +0200

On Wed, Aug 08, 2001 at 04:45:34AM +0000, Mikhael Goikhman wrote:
> one and it does not change any existing syntax.

Hm.  I'd really prefer a solution that is *not* extensible.  I
thought there was a common consensus that we don't want to
implement a scripting language in fvwm.

> So, if there are no

That's the best example I could think of why global variables are
*not* the way to go.  Whenever a command is executed outside a
function, there is a chance that another command gets executed
first and screws up the return code.

Bye

Dominik ^_^  ^_^

---

From: "Dmitry Yu. Bolkhovityanov" <[email-masked]>
Date: Wed, 8 Aug 2001 20:14:01 +0700

> global variables?  If we do this now we will regret it some day.

	That's just a logical consequence of current CMD_fff calling
convention.  I really investigated if globals can be avoided, but there is
*no* per-call structure to hold the return value.

	And yes, there is an obvious case when this solution would break
-- if someday fvwm will allow concurrent execution of functions (AFAIK,
currently it does so in an "interrupt" fashion, so that saving/restoring 
the state in execute_complex_function() is enough).

	_________________________________________
	  Dmitry Yu. Bolkhovityanov
	  [email-masked]
	  The Budker Institute of Nuclear Physics

---

From: Dominik Vogt <[email-masked]>
Date: Wed, 8 Aug 2001 21:29:50 +0200

> *no* per-call structure to hold the return value.

That's why I wanted avoid function return codes.  Every "return"
in each "CMD_..." function would have to be touched.

> the state in execute_complex_function() is enough).

What do you mean with "if someday"?  Fvwm already supports
asynchronous execution, e.g. when modules send commands.

Anyway, there is a fairly simple solution to save the patch:  add
another parameter to the macros in fvwm.h:

  #define F_CMD_ARGS XEvent *eventp, Window w, FvwmWindow *tmp_win, unsigned long context,char *action, int *Module, Bool *ret_rc
  #define F_PASS_ARGS eventp, w, tmp_win, context, action, Module, ret_rc
  #define F_EXEC_ARGS char *action, FvwmWindow *tmp_win, XEvent *eventp, unsigned long context, int Module, Bool *ret_rc
  #define F_PASS_EXEC_ARGS action, tmp_win, eventp, context, *Module, ret_rc

(sorry for the long lines).

The *ret_rc would be pre-initialised with "True" just like the
globals in the patch.  Only conditional commands ever set it to
False.  Now, only the few direct calls of CMD_... functions that
do not pass the arguments via one of the macros have to be
modified.  This would be good enough for me.

Bye

Dominik ^_^  ^_^

 --
Dominik Vogt, [email-masked]
Reply-To: [email-masked]
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Mikhael Goikhman <[email-masked]>
Date: Thu, 9 Aug 2001 14:29:01 +0000

> implement a scripting language in fvwm.

Are not scripting language and extensible solutions orthogonal?

Maybe we speak about different meanings of extensible solution, or you
really prefer to solve all new and old problems separately? For example,
if there is a need to do the following

  1) Add an alternative command to conditions
  2) Try several file names to Read
  3) Determine that a module was not started
  4) Determine a timeout in some commands

you suggest to alter a syntax of Next, Read, Module, ModuleSynchronous and
others, instead of identifying that all these problems are actually one
if-else problem and implementing an extensible solution, like command
statuses and "Otherwise", without a need to change any existing syntax?
With this solution, solving more similar problems is done by adding a line.

Or another example of extensible solution. We currently only need a
command status True/False, so we go with Boolean by adding it to many
places. But tomorrow we will need another Boolean, so we should add it to
the same places again. An extensible solution would suggest that we will
need more than a Boolean in the future and do it in the first place.

> first and screws up the return code.

You are right, there is a common problem with synchronizing commands from
the same stream in concurrent environment. But there are no problems if
these commands are Read from a file, because another command can't get
executed until the file is finished.

So I still think that Otherwise outside of functions is useful, although
it is almost useless interactively.

Regards,
Mikhael.

---

From: Dominik Vogt <[email-masked]>
Date: Thu, 9 Aug 2001 17:35:57 +0200

> With this solution, solving more similar problems is done by adding a line.

I'd rather rewrite the syntax 20 commands than extend the command
parser in such a way.  Currently it is *the* worst piece of fvwm
code: not extensible, hard to understand, undpredictable, buggy.

> need more than a Boolean in the future and do it in the first place.

Changing the signature of a function always causes some changes in
the code.  By the way, I *did* say that I think implementing any
kind of return codes is not a good idea.

> it is almost useless interactively.

You are right: it can only work inside complex functions and "Read"
files and fail from modules and the command line.  I guess then my
proposal is still the most sound one: easy to implement, the
patches are limited to a single file, helps with the most basic
needs some users have and does not encourage difficult to
implement enhancement requests.  In my eyes, we can only think
about scripting support in the command parser if we are prepared
to throw away the whole code in functions.c and write it from
scratch - which explicitly means that the most central part of
fvwm becomes incompatible to older versions.

Bye

Dominik ^_^  ^_^

---

From: Mikhael Goikhman <[email-masked]>
Date: Thu, 9 Aug 2001 15:10:16 +0000

> asynchronous execution, e.g. when modules send commands.

I think Dimitry meant asynchronous execution of _complex_ functions.
I myself often call "complex functions" simply functions as opposed to
"functions" that are commands.

> modified.  This would be good enough for me.

The patch became much longer, because all references to
old_execute_function and some other functions should be altered.

I attach the patch for reviewing. Changes other than in conditional.c and
functions.c are less interesting. The patch is somewhat different from
Dmitry's, but implements the same thing. Complex functions return the
status of the last inner command. Probably some things may be tuned.

Regards,
Mikhael.
Index: fvwm/add_window.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/add_window.c,v
retrieving revision 1.226
diff -u -r1.226 add_window.c
--- fvwm/add_window.c	2001/06/23 18:30:15	1.226
+++ fvwm/add_window.c	2001/08/09 14:55:53
@@ -1497,7 +1497,7 @@
     if (!IsRectangleOnThisPage(&tmp_win->frame_g, Scr.CurrentDesk))
     {
       SET_STICKY(tmp_win, 0);
-      handle_stick(&Event, tmp_win->frame, tmp_win, C_FRAME, "", 0, 1);
+      handle_stick(&Event, tmp_win->frame, tmp_win, C_FRAME, "", 0, NULL, 1);
     }
   }
 
@@ -1516,7 +1516,7 @@
     Event.xbutton.y = (tmp_win->frame_g.height>>1);
     Event.xbutton.subwindow = None;
     Event.xany.window = tmp_win->w;
-    CMD_Resize(&Event , tmp_win->w, tmp_win, C_WINDOW, "", 0);
+    CMD_Resize(&Event , tmp_win->w, tmp_win, C_WINDOW, "", 0, NULL);
   }
 
   /****** window colormap ******/
Index: fvwm/builtins.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/builtins.c,v
retrieving revision 1.313
diff -u -r1.313 builtins.c
--- fvwm/builtins.c	2001/08/01 15:22:51	1.313
+++ fvwm/builtins.c	2001/08/09 14:55:57
@@ -1861,7 +1861,7 @@
   if (!*s)
     return;
   Scr.cur_decor = decor;
-  old_execute_function(s, NULL, &Event, C_ROOT, -1, 0, NULL);
+  old_execute_function(s, NULL, &Event, C_ROOT, -1, NULL, 0, NULL);
   Scr.cur_decor = NULL;
 }
 
@@ -1968,7 +1968,8 @@
 	  fw->frame_g.height -= fw->decor->title_height;
 	  fw->decor = NULL;
 	  old_execute_function(
-	    "ChangeDecor Default", fw, eventp, C_WINDOW, *Module, 0, NULL);
+	    "ChangeDecor Default", fw, eventp, C_WINDOW, *Module, NULL, 0,
+	    NULL);
 	}
       }
     }
@@ -3267,7 +3268,8 @@
       usleep(200000);
       UngrabEm(GRAB_BUSY);
     }
-    old_execute_function(stroke_action, tmp_win, eventp, context, -1, 0, NULL);
+    old_execute_function(
+      stroke_action, tmp_win, eventp, context, -1, NULL, 0, NULL);
   }
 
 }
Index: fvwm/commands.h
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/commands.h,v
retrieving revision 1.1
diff -u -r1.1 commands.h
--- fvwm/commands.h	2001/03/24 15:15:21	1.1
+++ fvwm/commands.h	2001/08/09 14:55:57
@@ -114,6 +114,7 @@
 void CMD_None(F_CMD_ARGS);
 void CMD_Nop(F_CMD_ARGS);
 void CMD_OpaqueMoveSize(F_CMD_ARGS);
+void CMD_Otherwise(F_CMD_ARGS);
 void CMD_Pick(F_CMD_ARGS);
 void CMD_PipeRead(F_CMD_ARGS);
 void CMD_PixmapPath(F_CMD_ARGS);
Index: fvwm/conditional.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/conditional.c,v
retrieving revision 1.26
diff -u -r1.26 conditional.c
--- fvwm/conditional.c	2001/07/28 12:29:30	1.26
+++ fvwm/conditional.c	2001/08/09 14:55:58
@@ -458,12 +458,15 @@
   char *restofline;
 
   found = Circulate(action, -1, &restofline);
-  if(found && restofline)
+  if (found && restofline)
   {
     old_execute_function(
-      restofline, found, eventp, C_WINDOW, *Module, 0, NULL);
+      restofline, found, eventp, C_WINDOW, *Module, NULL, 0, NULL);
   }
-
+  if (!found)
+  {
+    *status = FS_WINDOW_MISMATCH;
+  }
 }
 
 void CMD_Next(F_CMD_ARGS)
@@ -472,12 +475,15 @@
   char *restofline;
 
   found = Circulate(action, 1, &restofline);
-  if(found && restofline)
+  if (found && restofline)
   {
     old_execute_function(
-      restofline, found, eventp, C_WINDOW, *Module, 0, NULL);
+      restofline, found, eventp, C_WINDOW, *Module, NULL, 0, NULL);
   }
-
+  if (!found)
+  {
+    *status = FS_WINDOW_MISMATCH;
+  }
 }
 
 void CMD_None(F_CMD_ARGS)
@@ -486,10 +492,14 @@
   char *restofline;
 
   found = Circulate(action, 1, &restofline);
-  if(!found && restofline)
+  if (!found && restofline)
   {
     old_execute_function(
-      restofline, NULL, eventp, C_ROOT, *Module, 0, NULL);
+      restofline, NULL, eventp, C_ROOT, *Module, NULL, 0, NULL);
+  }
+  if (found)
+  {
+    *status = FS_WINDOW_MISMATCH;
   }
 }
 
@@ -499,11 +509,15 @@
   char *restofline;
 
   found = Circulate(action, 0, &restofline);
-  if(found && restofline)
+  if (found && restofline)
   {
     old_execute_function(
-      restofline, found, eventp, C_WINDOW, *Module, 0, NULL);
+      restofline, found, eventp, C_WINDOW, *Module, NULL, 0, NULL);
   }
+  if (!found)
+  {
+    *status = FS_WINDOW_MISMATCH;
+  }
 }
 
 void CMD_All(F_CMD_ARGS)
@@ -542,13 +556,16 @@
   for (i = 0; i < num; i++)
   {
     old_execute_function(
-      restofline, g[i], eventp, C_WINDOW, *Module, 0, NULL);
+      restofline, g[i], eventp, C_WINDOW, *Module, NULL, 0, NULL);
   }
 
   free(g);
   FreeConditionMask(&mask);
 
-  return;
+  if (num == 0)
+  {
+    *status = FS_WINDOW_MISMATCH;
+  }
 }
 
 static void GetDirectionReference(FvwmWindow *w, rectangle *r)
@@ -713,8 +730,7 @@
   if (best_window)
   {
     old_execute_function(
-      restofline, best_window, eventp, C_WINDOW, *Module, 0,
-      NULL);
+      restofline, best_window, eventp, C_WINDOW, *Module, NULL, 0, NULL);
   }
 
   FreeConditionMask(&mask);
@@ -740,7 +756,7 @@
   if (MatchesConditionMask(tmp_win, &mask) && restofline)
   {
     old_execute_function(
-      restofline, tmp_win, eventp, C_WINDOW, *Module, 0, NULL);
+      restofline, tmp_win, eventp, C_WINDOW, *Module, NULL, 0, NULL);
   }
 }
 
@@ -754,6 +770,7 @@
   Bool use_screenroot = False;
   WindowConditionMask mask;
   char *flags, *restofline;
+  Bool was_executed = False;
 
   /* Get window ID */
   action = GetNextToken(action, &token);
@@ -777,7 +794,10 @@
       screen = 0;
     win = XRootWindow(dpy, screen);
     if (win == None)
+    {
+      *status = FS_WINDOW_MISMATCH;
       return;
+    }
   }
   else if (token)
   {
@@ -819,8 +839,11 @@
     {
       /* do it if no conditions or the conditions match */
       if (action && (!use_condition || MatchesConditionMask(t, &mask)))
+      {
         old_execute_function(
-	  action, t, eventp, C_WINDOW, *Module, 0, NULL);
+	  action, t, eventp, C_WINDOW, *Module, NULL, 0, NULL);
+        was_executed = True;
+      }
       break;
     }
   }
@@ -838,11 +861,17 @@
     efa.module = *Module;
     efa.flags.is_window_unmanaged = 1;
     execute_function(&efa);
+    was_executed = True;
   }
 
   /* Tidy up */
   if (use_condition)
   {
     FreeConditionMask(&mask);
+  }
+
+  if (!was_executed)
+  {
+    *status = FS_WINDOW_MISMATCH;
   }
 }
Index: fvwm/events.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/events.c,v
retrieving revision 1.302
diff -u -r1.302 events.c
--- fvwm/events.c	2001/07/28 22:34:01	1.302
+++ fvwm/events.c	2001/08/09 14:56:01
@@ -298,7 +298,8 @@
   if (action != NULL)
   {
     ButtonWindow = Tmp_win;
-    old_execute_function(action, Tmp_win, &Event, Context, -1, 0, NULL);
+    old_execute_function(
+      action, Tmp_win, &Event, Context, -1, NULL, 0, NULL);
     ButtonWindow = NULL;
     return;
   }
@@ -687,14 +688,15 @@
 	(Tmp_win->wmhints->flags & XUrgencyHint))
     {
       old_execute_function(
-	"Function UrgencyFunc", Tmp_win, &Event, C_WINDOW, -1, 0, NULL);
+	"Function UrgencyFunc", Tmp_win, &Event, C_WINDOW, -1, NULL, 0, NULL);
     }
 
     if ((old_wmhints_flags & XUrgencyHint) &&
 	!(Tmp_win->wmhints->flags & XUrgencyHint))
     {
       old_execute_function(
-	"Function UrgencyDoneFunc", Tmp_win, &Event, C_WINDOW, -1, 0, NULL);
+	"Function UrgencyDoneFunc", Tmp_win, &Event, C_WINDOW, -1, NULL, 0,
+        NULL);
     }
     break;
   case XA_WM_NORMAL_HINTS:
@@ -854,7 +856,8 @@
                    &(button.xmotion.y_root),
                    &JunkX, &JunkY, &JunkMask);
     button.type = 0;
-    old_execute_function("Iconify", Tmp_win, &button, C_FRAME, -1, 0, NULL);
+    old_execute_function(
+      "Iconify", Tmp_win, &button, C_FRAME, -1, NULL, 0, NULL);
     return;
   }
 
@@ -1653,7 +1656,8 @@
       /* release the pointer since it can't do harm over an icon */
       XAllowEvents(dpy, AsyncPointer, CurrentTime);
     }
-    old_execute_function(action, Tmp_win, &Event, Context, -1, 0, NULL);
+    old_execute_function(
+      action, Tmp_win, &Event, Context, -1, NULL, 0, NULL);
     if (Context != C_WINDOW && Context != C_NO_CONTEXT)
     {
       WaitForButtonsUp(True);
@@ -1743,7 +1747,8 @@
    /* got a match, now process it */
    if (action != NULL && (action[0] != 0))
    {
-     old_execute_function(action, Tmp_win, &Event, Context, -1, 0, NULL);
+     old_execute_function(
+       action, Tmp_win, &Event, Context, -1, NULL, 0, NULL);
      WaitForButtonsUp(True);
    }
    else
Index: fvwm/functions.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/functions.c,v
retrieving revision 1.185
diff -u -r1.185 functions.c
--- fvwm/functions.c	2001/07/21 19:29:26	1.185
+++ fvwm/functions.c	2001/08/09 14:56:02
@@ -55,6 +55,8 @@
 extern FvwmWindow *Tmp_win;
 extern char const * const Fvwm_VersionInfo;
 
+static int last_function_status = 0;
+
 /* forward declarations */
 static void execute_complex_function(F_CMD_ARGS, Bool *desperate);
 
@@ -72,6 +74,15 @@
   return;
 }
 
+void CMD_Otherwise(F_CMD_ARGS)
+{
+  if (last_function_status != FS_SUCCESS)
+  {
+    old_execute_function(F_PASS_EXEC_ARGS, 0, NULL);
+  }
+  return;
+}
+
 /*
  * be sure to keep this list properly ordered for bsearch routine!
  */
@@ -188,6 +199,7 @@
   CMD_ENTRY("none", CMD_None, F_NONE, 0),
   CMD_ENTRY("nop", CMD_Nop, F_NOP, FUNC_DONT_REPEAT),
   CMD_ENTRY("opaquemovesize", CMD_OpaqueMoveSize, F_OPAQUE, 0),
+  CMD_ENTRY("otherwise", CMD_Otherwise, F_OTHERWISE, 0),
   CMD_ENTRY("pick", CMD_Pick, F_PICK, 0),
   CMD_ENTRY("piperead", CMD_PipeRead, F_READ, 0),
   CMD_ENTRY("pixmappath", CMD_PixmapPath, F_PIXMAP_PATH, 0),
@@ -547,6 +559,10 @@
       case '.':
 	string = get_current_read_dir();
 	break;
+      case '?':
+	l2 += 10;
+	i++;
+	break;
       case 'w':
       case 'd':
       case 'x':
@@ -678,6 +694,11 @@
 	string = get_current_read_dir();
 	is_string = True;
 	break;
+      case '?':
+	sprintf(&out[j], "%i", last_function_status);
+	j += strlen(&out[j]);
+	i++;
+	break;
       case 'w':
 	if(tmp_win)
 	  sprintf(&out[j],"0x%x",(unsigned int)tmp_win->w);
@@ -925,6 +946,7 @@
   Bool must_free_string = False;
   Bool must_free_function = False;
 
+  efa->status = FS_SUCCESS;
   if (!efa->action)
   {
     /* impossibly short command */
@@ -1005,6 +1027,7 @@
   }
   if (taction == NULL)
   {
+    last_function_status = efa->status;
     if (set_silent)
       Scr.flags.silent_functions = 0;
     func_depth--;
@@ -1092,7 +1115,9 @@
     char *runaction;
 
     runaction = SkipNTokens(expaction, 1);
-    bif->action(efa->eventp,w,efa->tmp_win,efa->context,runaction,&efa->module);
+    bif->action(
+      efa->eventp, w, efa->tmp_win, efa->context, runaction,
+      &efa->module, &efa->status);
   }
   else
   {
@@ -1110,19 +1135,21 @@
     }
 
     execute_complex_function(
-      efa->eventp,w,efa->tmp_win,efa->context,runaction, &efa->module,
-      &desperate);
+      efa->eventp, w, efa->tmp_win, efa->context, runaction, &efa->module,
+      &efa->status, &desperate);
     if (!bif && desperate)
     {
       if (executeModuleDesperate(
-	efa->eventp, w, efa->tmp_win, efa->context, runaction, &efa->module) ==
-	  -1 && *function != 0)
+	efa->eventp, w, efa->tmp_win, efa->context, runaction, &efa->module,
+          &efa->status) == -1 && *function != 0)
       {
+	efa->status = FS_INCORRECT_USAGE;
 	fvwm_msg(
 	  ERR, "execute_function", "No such command '%s'", function);
       }
     }
   }
+  last_function_status = efa->status;
 
   if (set_silent)
     Scr.flags.silent_functions = 0;
@@ -1149,7 +1176,7 @@
 
 void old_execute_function(
   char *action, FvwmWindow *tmp_win, XEvent *eventp, unsigned long context,
-  int Module, FUNC_FLAGS_TYPE exec_flags, char *args[])
+  int Module, int *status, FUNC_FLAGS_TYPE exec_flags, char *args[])
 {
   exec_func_args_type efa;
 
@@ -1160,9 +1187,12 @@
   efa.args = args;
   efa.context = context;
   efa.module = Module;
+  efa.status = FS_SUCCESS;  /* not really used */
   efa.flags.exec = exec_flags;
   execute_function(&efa);
 
+  if (status)
+    *status = efa.status;
   return;
 }
 
@@ -1575,6 +1605,7 @@
   {
     if(*desperate == 0)
       fvwm_msg(ERR, "ComplexFunction", "No such function %s",action);
+    *status = FS_INCORRECT_USAGE;
     return;
   }
   if (!depth)
@@ -1656,7 +1687,8 @@
       else
 	w = None;
       old_execute_function(
-	fi->action, tmp_win, eventp, context, -1, 0, arguments);
+	fi->action, tmp_win, eventp, context, -1, status, 0, arguments);
+      last_function_status = *status;
       break;
     case CF_DOUBLE_CLICK:
       HaveDoubleClick = True;
@@ -1768,7 +1800,8 @@
       else
 	w = None;
       old_execute_function(
-	fi->action, tmp_win, ev, context, -1, 0, arguments);
+	fi->action, tmp_win, ev, context, -1, status, 0, arguments);
+      last_function_status = *status;
     }
     fi = fi->next_item;
   }
Index: fvwm/functions.h
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/functions.h,v
retrieving revision 1.44
diff -u -r1.44 functions.h
--- fvwm/functions.h	2001/07/21 19:29:26	1.44
+++ fvwm/functions.h	2001/08/09 14:56:02
@@ -40,7 +40,8 @@
 {
   char *keyword;
 #ifdef __STDC__
-  void (*action)(XEvent *,Window,FvwmWindow *, unsigned long,char *, int *);
+  void (*action)(
+    XEvent *, Window, FvwmWindow *, unsigned long, char *, int *, int *);
 #else
   void (*action)();
 #endif
@@ -68,6 +69,12 @@
 /* for exec_flags parameter of ExecuteFunction */
 #define FUNC_DONT_EXPAND_COMMAND 0x01
 
+/* Function status codes. */
+#define FS_SUCCESS 0
+#define FS_FAILURE -1  /* generic failure */
+#define FS_INCORRECT_USAGE 1
+#define FS_WINDOW_MISMATCH 2
+
 void find_func_type(char *action, short *func_type, unsigned char *flags);
 FvwmFunction *FindFunction(const char *function_name);
 extern FvwmFunction *NewFvwmFunction(const char *name);
@@ -77,7 +84,7 @@
 void execute_function(exec_func_args_type *efa);
 void old_execute_function(
   char *action, FvwmWindow *tmp_win, XEvent *eventp, unsigned long context,
-  int Module, FUNC_FLAGS_TYPE exec_flags, char *args[]);
+  int Module, int *status, FUNC_FLAGS_TYPE exec_flags, char *args[]);
 void AddToFunction(FvwmFunction *func, char *action);

@@ -147,6 +154,7 @@
   F_NEXT,
   F_NONE,
   F_OPAQUE,
+  F_OTHERWISE,
   F_PICK,
   F_PIXMAP_PATH,
   F_POINTERKEY,
Index: fvwm/fvwm.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/fvwm.c,v
retrieving revision 1.270
diff -u -r1.270 fvwm.c
--- fvwm/fvwm.c	2001/08/01 15:22:52	1.270
+++ fvwm/fvwm.c	2001/08/09 14:56:05
@@ -655,7 +655,7 @@
     {
       DoingCommandLine = True;
       old_execute_function(
-	config_commands[i], NULL, &Event, C_ROOT, 1, 0, NULL);
+	config_commands[i], NULL, &Event, C_ROOT, 1, NULL, 0, NULL);
       free(config_commands[i]);
     }
     DoingCommandLine = False;
@@ -791,14 +791,14 @@
   /* migo (04-Sep-1999): execute StartFunction */
   if (FindFunction(startFuncName)) {
     char *action = "Function " startFuncName;
-    old_execute_function(action, NULL, &Event, C_ROOT, 1, 0, NULL);
+    old_execute_function(action, NULL, &Event, C_ROOT, 1, NULL, 0, NULL);
   }
 
   /* migo (03-Jul-1999): execute [Session]{Init|Restart}Function */
   initFuncName = getInitFunctionName(Restarting == True);
   if (FindFunction(initFuncName)) {
     char *action = safestrdup(CatString2("Function ", initFuncName));
-    old_execute_function(action, NULL, &Event, C_ROOT, 1, 0, NULL);
+    old_execute_function(action, NULL, &Event, C_ROOT, 1, NULL, 0, NULL);
     free(action);
   }
 
@@ -850,7 +850,7 @@
 
   while (defaults[i])
   {
-    old_execute_function(defaults[i], NULL, &Event, C_ROOT, 1, 0, NULL);
+    old_execute_function(defaults[i], NULL, &Event, C_ROOT, 1, NULL, 0, NULL);
     i++;
   }
 } /* SetRCDefaults */
@@ -1549,7 +1549,7 @@
   if (FindFunction(exitFuncName))
   {
     char *action = safestrdup(CatString2("Function ", exitFuncName));
-    old_execute_function(action, NULL, &Event, C_ROOT, 1, 0, NULL);
+    old_execute_function(action, NULL, &Event, C_ROOT, 1, NULL, 0, NULL);
     free(action);
   }
 
Index: fvwm/fvwm.h
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/fvwm.h,v
retrieving revision 1.142
diff -u -r1.142 fvwm.h
--- fvwm/fvwm.h	2001/07/28 12:29:30	1.142
+++ fvwm/fvwm.h	2001/08/09 14:56:06
@@ -67,11 +67,11 @@
 
 /* Macro for args passed to fvwm commands... */
 #define F_CMD_ARGS XEvent *eventp, Window w, FvwmWindow *tmp_win,\
-unsigned long context,char *action, int *Module
-#define F_PASS_ARGS eventp, w, tmp_win, context, action, Module
+  unsigned long context, char *action, int *Module, int *status
+#define F_PASS_ARGS eventp, w, tmp_win, context, action, Module, status
 #define F_EXEC_ARGS char *action, FvwmWindow *tmp_win, XEvent *eventp,\
-unsigned long context, int Module
-#define F_PASS_EXEC_ARGS action, tmp_win, eventp, context, *Module
+  unsigned long context, int Module, int *status
+#define F_PASS_EXEC_ARGS action, tmp_win, eventp, context, *Module, status
 #define FUNC_FLAGS_TYPE unsigned char
 typedef struct
 {
@@ -85,6 +85,7 @@
   /* the context in which the button was pressed */
   unsigned long context;
   int module;
+  int status;
   /* If tmp_win is NULL, the is_window_unmanaged flag may be set along with
    * this field to indicate the function should run with an unmanaged window.
    */
Index: fvwm/gnome.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/gnome.c,v
retrieving revision 1.37
diff -u -r1.37 gnome.c
--- fvwm/gnome.c	2001/05/20 16:25:13	1.37
+++ fvwm/gnome.c	2001/08/09 14:56:07
@@ -984,13 +984,13 @@
     {
       /* shade up */
       old_execute_function(
-	"WindowShade True", fwin, ev, C_WINDOW, -1, 0, NULL);
+	"WindowShade True", fwin, ev, C_WINDOW, -1, NULL, 0, NULL);
     }
     else
     {
       /* shade down */
       old_execute_function(
-	"WindowShade False", fwin, ev, C_WINDOW, -1, 0, NULL);
+	"WindowShade False", fwin, ev, C_WINDOW, -1, NULL, 0, NULL);
 
     }
   }
Index: fvwm/module_interface.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/module_interface.c,v
retrieving revision 1.94
diff -u -r1.94 module_interface.c
--- fvwm/module_interface.c	2001/07/28 23:59:09	1.94
+++ fvwm/module_interface.c	2001/08/09 14:56:08
@@ -640,7 +640,8 @@
   Event.xbutton.subwindow = None;
   Context = GetContext(tmp_win,&Event,&w);
   ButtonWindow = tmp_win;
-  old_execute_function(text, tmp_win, &Event, Context, module, 0, NULL);
+  old_execute_function(
+    text, tmp_win, &Event, Context, module, NULL, 0, NULL);
   ButtonWindow = NULL;
 }
 
Index: fvwm/move_resize.h
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/move_resize.h,v
retrieving revision 1.16
diff -u -r1.16 move_resize.h
--- fvwm/move_resize.h	2001/07/25 00:18:19	1.16
+++ fvwm/move_resize.h	2001/08/09 14:56:08
@@ -26,8 +26,7 @@
 Bool moveLoop(FvwmWindow *tmp_win, int XOffset, int YOffset, int Width,
 	      int Height, int *FinalX, int *FinalY,Bool do_move_opaque);
 void move_window_doit(
-  XEvent *eventp,Window w,FvwmWindow *tmp_win, unsigned long context,
-  char *action,int* Module, Bool do_animate, Bool do_move_to_page);
+  F_CMD_ARGS, Bool do_animate, Bool do_move_to_page);
 void handle_stick(F_CMD_ARGS, int toggle);
 void resize_geometry_window(void);
 
Index: fvwm/read.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/read.c,v
retrieving revision 1.47
diff -u -r1.47 read.c
--- fvwm/read.c	2001/05/24 21:26:17	1.47
+++ fvwm/read.c	2001/08/09 14:56:09
@@ -143,7 +143,8 @@
       fvwm_msg(DBG,"ReadSubFunc","Module switch %d, about to exec: '%.*s'",
 	       Module,strlen(tline)-1,tline);
 
-    old_execute_function(tline, tmp_win, eventp, context, Module, 0, NULL);
+    old_execute_function(
+      tline, tmp_win, eventp, context, Module, NULL, 0, NULL);
     tline = fgets(line, (sizeof line) - 1, f);
   }
 }
@@ -267,14 +268,19 @@
 	     *Module, action);
 
   if ( !parse_filename( "Read", action, &filename, &read_quietly ) )
+  {
+    *status = FS_INCORRECT_USAGE;
     return;
+  }
   cursor_control(True);
-  if ( !run_command_file( filename, eventp, tmp_win, context, *Module ) &&
-       !read_quietly )
+  if (!run_command_file(filename, eventp, tmp_win, context, *Module))
   {
-    fvwm_msg( ERR, "Read",
-	      "file '%s' not found in %s or "FVWM_DATADIR,
-	      filename, fvwm_userdir );
+    if (!read_quietly)
+    {
+      fvwm_msg(ERR, "Read",
+        "file '%s' not found in %s or "FVWM_DATADIR, filename, fvwm_userdir);
+    }
+    *status = FS_FAILURE;
   }
   free( filename );
   cursor_control(False);
@@ -294,7 +300,10 @@
     fvwm_msg(DBG,"PipeRead","about to attempt '%s'", action);
 
   if (!parse_filename("PipeRead", action, &command, &read_quietly))
+  {
+    *status = FS_INCORRECT_USAGE;
     return;
+  }
   cursor_control(True);
   f = popen(command, "r");
 
@@ -304,6 +313,7 @@
       fvwm_msg( ERR, "PipeRead", "command '%s' not run", command );
     free(command);
     cursor_control(False);
+    *status = FS_FAILURE;
     return;
   }
   free(command);
Index: fvwm/update.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/update.c,v
retrieving revision 1.36
diff -u -r1.36 update.c
--- fvwm/update.c	2001/04/07 12:11:15	1.36
+++ fvwm/update.c	2001/08/09 14:56:09
@@ -100,8 +100,8 @@
     if (IS_ICON_STICKY(pstyle))
     {
       /* stick and unstick the window to force the icon on the current page */
-      handle_stick(&Event, t->frame, t, C_FRAME, "", 0, 1);
-      handle_stick(&Event, t->frame, t, C_FRAME, "", 0, 0);
+      handle_stick(&Event, t->frame, t, C_FRAME, "", 0, NULL, 1);
+      handle_stick(&Event, t->frame, t, C_FRAME, "", 0, NULL, 0);
     }
     else
     {
@@ -110,7 +110,8 @@
   }
   else if (flags->do_update_stick)
   {
-    handle_stick(&Event, t->frame, t, C_FRAME, "", 0, SFIS_STICKY(*pstyle));
+    handle_stick(
+      &Event, t->frame, t, C_FRAME, "", 0, NULL, SFIS_STICKY(*pstyle));
   }
 #ifdef MINI_ICONS
   if (flags->do_update_mini_icon)
Index: fvwm/windowlist.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/windowlist.c,v
retrieving revision 1.46
diff -u -r1.46 windowlist.c
--- fvwm/windowlist.c	2001/07/23 22:38:29	1.46
+++ fvwm/windowlist.c	2001/08/09 14:56:09
@@ -544,7 +544,7 @@
   DestroyMenu(mr, False, False);
   if (mret.rc == MENU_DOUBLE_CLICKED && default_action && *default_action)
     old_execute_function(
-      default_action, tmp_win, eventp, context, *Module, 0 , NULL);
+      default_action, tmp_win, eventp, context, *Module, NULL, 0 , NULL);
   if (default_action != NULL)
     free(default_action);
   if (use_condition)

---

From: Mikhael Goikhman <[email-masked]>
Date: Thu, 9 Aug 2001 22:21:44 +0000

> code: not extensible, hard to understand, undpredictable, buggy.

I am not sure why do you think so. I see no reasons.

It is actually not hard to understand if you understood the original
smaller patch. Granted, I didn't tested it fully, only maybe 5-6 cases,
But if you don't use the new stuff (command Otherwise or last status $?)
there should not be any reduction in stability. This may wait for 2.5.0.

> fvwm becomes incompatible to older versions.

Concurrent command execution is already broken within "+" command. This is
an old problem, which probably can be solved if we store all "global" data
last_* separately for every module pipe. Will this be a solution?

Dominik, you once said that a lot of harm was done by many small patches
each implementing its own inconsistent syntax. The enhancement you suggest
is powerful, of course, you may write a whole if-else program in one line
using nested conditions. But I don't feel this is a good syntax to
introduce. I would not introduce any new syntax as radical as this without
thinking about consistency with other command syntaxes. The solution that
does not introduce any new syntax, but solves more than one problem is
more preferable to me. After 2.5.0 we may think and change syntax.

Anyway, I have no any willing to force my solution without your agreement.

Regards,
Mikhael.

---

From: Dominik Vogt <[email-masked]>
Date: Fri, 10 Aug 2001 06:26:07 +0200

> "functions" that are commands.

Well, of course he meant complex functions.  But e.g. FvwmIconMan
already supports it's own implementation of complex functions by
sending commands successively over the module pipe.

Bye

Dominik ^_^  ^_^