Subject: command parsing problems

From: Mikhael Goikhman <[email-masked]>
Date: Mon, 21 May 2001 14:50:45 +0000

The current command parsing is done in this order:

  1) Handling the module command (if starting with '*')
  2) Getting the first word (command name)
  3) Expanding the first word and the entire command separately
  4) Handling the internal command

This leads to several problems, like:

  * The command starting with '*' is not a subject for parameter expansion,
    thus making it impossible to construct module commands or have generic
    module templates (without an overhead of forking using PipeRead echo)
  * In module commands extended variables like $[fg.cs1] are not
    expanded, sometimes making it hard to have functional module configs
  * Calling function named "echo twice" does not work, although calling
    function named "print twice" works. This just shows how bad is the
    existing parsing order.

The problems are real, for example a sample in FAQ 7.13 does not work now.
I am not sure why it worked at all (2 bugs eliminate each other?).

Currently I am learning now how hard will be to implement a more correct
order without breaking anything:

  1) Expanding the entire command
  2) Handling the module command (if starting with '*')
  2) Getting the first word (command name)
  3) Handling the internal command

This would fix all 3 problems above together with more inconsistencies,
say, a leading '-' (meaning don't expand) would work for all commands.

Of course, for backward compatibility single-letter variables in module
commands should not be expanded (this is unfortunate, but needed, since
FvwmButtons and FvwmForm expect this). However $[] variables is a new
invention and there is no reason not to expand them in module commands.

Does anyone see any problem if $0 to $9 are expanded in module commands
inside functions? I can't think of any except of really exotic examples,
in which the raw '$0' could be escaped like in any other place - '$$0'.

BTW, I think it is not late yet to reduce expanding collisions and use '%'
instead of '$' in the new FvwmButtons constructions: %fg, %right, %width.
Or a less conflicting syntax $(fg), like the one already used in FvwmForm.

This is not the best time for this reworking, but the existing command
parsing problems should be solved (at least FAQ 7.13 sample should work).
As usual I will not change anything without a thorough testing first.
Any comments are welcome.

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: Mon, 21 May 2001 18:16:14 +0200

> Any comments are welcome.

I'm rather reluctant to open this can of worms again this close to
2.4.  The problems you mention are not nice, but everything works
as it used to right now.  I already tried to fix some of this, but
was not able to do so without breaking backwards compatibility.
May this be a candidate for the ominous 3.0 release?  I'd really
like to throw away the whole parsing stuff and write it from
scratch in a clean, well designed way.  Of course that will break
a lot of config files :-/

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: Mikhael Goikhman <[email-masked]>
Date: Mon, 21 May 2001 21:55:33 +0000

> a lot of config files :-/

I know about these issues, because I write a lot of config stuff. But I am
concerned that fvwm-themes should work with fvwm-2.4.x not fvwm-3.0.x.

I see how to fix the "space in function name" problem, it just requires
some duplication - second PeekToken after expand and reevaluation of bif.
But this problem is not important, and we may just leave it as is.
However the module command processing is way too limiting.

I patched execute_function() locally and other problems mentioned by me
are fixed. Tomorrow I will patch expand() so it will not expand too much
for module commands, then test everything and send the resulting small
patch. I want to keep backward compatibility as much as possible.

Regards,
Mikhael.

---

From: Dan Espen <[email-masked]>
Date: Tue, 22 May 2001 10:45:55 -0400

> I am not sure why it worked at all (2 bugs eliminate each other?).

Which 2 bugs?

I'm pretty sure 7.13 worked at one time.  As I remember, more than
one of us looked at it.

I'd expect function args to be expanded during function interpretation
regardless of the command in the function.

-- 
Dan Espen
444 Hoes Lane  Room RRC 1C-214           E-mail: [email-masked]
Piscataway, NJ 08854                     Phone: (732) 699-5570
--
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: Tue, 22 May 2001 17:07:29 +0200

> I'm pretty sure 7.13 worked at one time.  As I remember, more than

That does not necessarily mean that the example worked at a time
when parsing was compatible with the 2.2.x versions.  The example
may have been written at a time when parsing was broken.

Bye

Dominik ^_^  ^_^

---

From: Dan Espen <[email-masked]>
Date: Tue, 22 May 2001 13:04:09 -0400

Dominik Vogt <[email-masked]> writes:
> > > The problems are real, for example a sample in FAQ 7.13 does not work now

Yes, but:

 $0 to $9
    The positional parameters given to a  complex  func-
    tion  (a  function  that  has  been defined with the
    AddToFunc command).  "$0" is replaced with the first
    parameter, "$1" with the second parameter and so on.

---

From: Mikhael Goikhman <[email-masked]>
Date: Tue, 22 May 2001 20:21:27 +0000

>     parameter, "$1" with the second parameter and so on.

True, but there is also this line (I hope it will not be valid tomorrow):

    Note that module configuration lines (i.e. lines beginning with '*')
    are not subject to parameter expansion.

Here is a small patch adding partial expansion to module config lines;
function parameters and new inventions like $[page.nx] are expended.

I tested it during the day and verified that it does not conflict with
any of our 27 modules accourding to their man pages.

With this patch FAQ 7.13 works. I plan to apply it tomorrow.
I don't want to do any other changes to command parsing before 2.4.0.

Regards,
Mikhael.
Index: functions.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/functions.c,v
retrieving revision 1.181
diff -u -r1.181 functions.c
--- functions.c	2001/05/08 23:53:22	1.181
+++ functions.c	2001/05/22 20:08:28
@@ -470,7 +470,7 @@
 }
 
 static char *expand(
-  char *input, char *arguments[], FvwmWindow *tmp_win, Bool addto)
+  char *input, char *arguments[], FvwmWindow *tmp_win, Bool addto, Bool ismod)
 {
   int l,i,l2,n,k,j,m;
   int xlen;
@@ -491,7 +491,7 @@
   i=0;
   while(i<l)
   {
-    if(input[i] == '$')
+    if (input[i] == '$' && (!ismod || !isalpha(input[i+1])))
     {
       switch (input[i+1])
       {
@@ -598,7 +598,7 @@
   j=0;
   while(i<l)
   {
-    if(input[i] == '$')
+    if (input[i] == '$' && (!ismod || !isalpha(input[i+1])))
     {
       switch (input[i+1])
       {
@@ -944,24 +944,6 @@
   fvwm_msg(INFO, "LOG", "%s", efa->action);
 #endif
 
-  /* Note: the module config command, "*" can not be handled by the
-   * regular command table because there is no required white space after
-   * the asterisk. */
-  if (efa->action[0] == '*')
-  {
-#ifdef USEDECOR
-    if (Scr.cur_decor && Scr.cur_decor != &Scr.DefaultDecor)
-    {
-      fvwm_msg(
-	WARN, "execute_function",
-	"Command can not be added to a decor; executing command now: '%s'",
-	efa->action);
-    }
-#endif
-    /* process a module config command */
-    ModuleConfig(efa->action);
-    return;
-  }
   func_depth++;
   if (efa->args)
   {
@@ -1036,8 +1018,8 @@
 
   function = PeekToken(taction, NULL);
   if (function)
-    function = expand(function, arguments, efa->tmp_win, False);
-  if (function)
+    function = expand(function, arguments, efa->tmp_win, False, False);
+  if (function && taction[0] != '*')
   {
 #if 1
     /* DV: with this piece of code it is impossible to have a complex function
@@ -1069,10 +1051,11 @@
       efa->action);
   }
 #endif
+
   if (!(efa->flags.exec & FUNC_DONT_EXPAND_COMMAND))
   {
     expaction = expand(taction, arguments, efa->tmp_win,
-		       (bif) ? !!(bif->flags & FUNC_ADD_TO) : False);
+      (bif) ? !!(bif->flags & FUNC_ADD_TO) : False, taction[0] == '*');
     if (func_depth <= 1)
       must_free_string = set_repeat_data(expaction, REPEAT_COMMAND, bif);
     else
@@ -1082,7 +1065,25 @@
   {
     expaction = taction;
   }
-  j = 0;
+
+  /* Note: the module config command, "*" can not be handled by the
+   * regular command table because there is no required white space after
+   * the asterisk. */
+  if (expaction[0] == '*')
+  {
+#ifdef USEDECOR
+    if (Scr.cur_decor && Scr.cur_decor != &Scr.DefaultDecor)
+    {
+      fvwm_msg(
+	WARN, "execute_function",
+	"Command can not be added to a decor; executing command now: '%s'",
+	expaction);
+    }
+#endif
+    /* process a module config command */
+    ModuleConfig(expaction);
+  }
+  else
   if (bif && bif->func_type != F_FUNCTION)
   {
     char *runaction;