Subject: FindPopup patch & fvwmrc_convert patch (take 2)

From: Julian Gilbey <[email-masked]>
Date: Thu, 4 Mar 1999 01:41:54 +0000 (GMT)

I was continuing to work on patching fvwmrc_convert, when I came
across a bug in the FindPopup() function.  This shows itself when a
menu name has spaces in it, such as "Window Ops", and is due the
double parsing of the name.  The patch below fixes this.  Also, there
is a new fvwmrc_convert patch.  For ease, this patch is against the
distributed 2.2 version, so the patches I posted earlier are repeated
here.  Again, I will list a rationale for the patches.  New patches
are indicated with a * in the following list.  The numbers (1), (2)
etc. refer to the sections of the diff file delimited by @@.  Also, I
haven't checked which ones, but some of these diffs, although not
marked as new, supercede the ones I posted earlier in the week, so
should be used instead.

Patches to fix the FindPopup() bug
----------------------------------

Essentially, the FindPopup() function does not distinguish between
cases when the argument should be fed to GetNextToken() prior to use,
and when not.  I have solved this by renaming the original FindPopup()
function as FindPopupGetTok(), and creating a new function FindPopup()
in place of the original which does exactly the same except that it
does not call GetNextToken() on the string given.  Then those
occurrences of FindPopup() which require the strings to be parsed are
replaced by FindPopupGetTok().

The patches to fvwmrc_convert partially assume that this patch has
been applied; otherwise, some created .fvwm2rc files may not work.

Patches to fvwmrc_convert
-------------------------

 (1)* Change in accreditations; our awk is in a different place ;-)
 (2)  Whitespace only lines are treated as empty.
 (3)  Remove trailing "&" sign from Exec commands.
 (4)  Slightly more verbose ButtonStyle error message.
 (5)* Oops -- a typo in the order of +'s and -'s led to button shapes
      which looked like diabolos instead of rectangles.
 (6)* Comment out any ModulePath command.
      I thought that the paths were going to use fvwm, not fvwm2.  So
      the paths should not have gsub("/fvwm", "/fvwm2"); in them.
 (7)* Handling of the case Key ...... (Exec|Module|Restart) "name" ...
      Similar for the case Mouse .... (Exec|Module|Restart) "name" ...
      Changed the Mouse code to just pass it through otherwise; why is
      there any need to make mouse bindings sticky, anyway?
 (8)  Awful typo: searching for /^[ \t]**.../ takes huge amounts of time
      and/or memory.  This MUST be fixed (and it's trivial).
      Slightly more verbose warning message.
 (9)* Preserve the names of Menus in cases where they are quoted.
      More consistent error message.
(10)  If the internal Pager command is used, the output file now gives
      hints in the output file as to how the FvwmPager module can be
      configured to look similar to the original internal pager.

diff -Nru fvwm-2.2.orig/fvwm/builtins.c fvwm-2.2/fvwm/builtins.c
--- fvwm-2.2.orig/fvwm/builtins.c	Thu Feb 11 23:41:27 1999
+++ fvwm-2.2/fvwm/builtins.c	Thu Mar  4 01:04:22 1999
@@ -437,7 +437,7 @@
 #include <sys/time.h>

-MenuRoot *FindPopup(char *action)
+MenuRoot *FindPopupGetTok(char *action)
 {
   char *tmp;
   MenuRoot *mr;
@@ -461,6 +461,24 @@
   free(tmp);
   return NULL;
 
+}
+
+MenuRoot *FindPopup(char *action)
+{
+  MenuRoot *mr;
+
+  if(action == NULL)
+    return NULL;
+
+  mr = Scr.menus.all;
+  while(mr != NULL)
+  {
+    if(mr->name != NULL)
+      if(strcasecmp(action,mr->name)== 0)
+        return mr;
+    mr = mr->next;
+  }
+  return NULL;
 }

diff -Nru fvwm-2.2.orig/fvwm/complex.c fvwm-2.2/fvwm/complex.c
--- fvwm-2.2.orig/fvwm/complex.c	Tue Jan 12 21:07:42 1999
+++ fvwm-2.2/fvwm/complex.c	Thu Mar  4 01:04:22 1999
@@ -78,7 +78,7 @@
   MenuRoot *mr;
   extern Bool desperate;
 
-  mr = FindPopup(action);
+  mr = FindPopupGetTok(action);
   if(mr == NULL)
     {
       if(!desperate)
diff -Nru fvwm-2.2.orig/fvwm/fvwm.c fvwm-2.2/fvwm/fvwm.c
--- fvwm-2.2.orig/fvwm/fvwm.c	Wed Jan 27 19:11:49 1999
+++ fvwm-2.2/fvwm/fvwm.c	Thu Mar  4 01:04:22 1999
@@ -554,13 +554,13 @@
 
   if(Restarting)
   {
-    mr = FindPopup("RestartFunction");
+    mr = FindPopupGetTok("RestartFunction");
     if(mr != NULL)
       ExecuteFunction("Function RestartFunction",NULL,&Event,C_ROOT,-1);
   }
   else
   {
-    mr = FindPopup("InitFunction");
+    mr = FindPopupGetTok("InitFunction");
     if(mr != NULL)
       ExecuteFunction("Function InitFunction",NULL,&Event,C_ROOT,-1);
   }
@@ -1449,7 +1449,7 @@
   MoveViewport(0,0,False);
 #endif
 
-  mr = FindPopup("ExitFunction");
+  mr = FindPopupGetTok("ExitFunction");
   if(mr != NULL)
     ExecuteFunction("Function ExitFunction",NULL,&Event,C_ROOT,-1);
 
diff -Nru fvwm-2.2.orig/fvwm/misc.h fvwm-2.2/fvwm/misc.h
--- fvwm-2.2.orig/fvwm/misc.h	Tue Feb 16 16:11:44 1999
+++ fvwm-2.2/fvwm/misc.h	Thu Mar  4 01:04:22 1999
@@ -361,6 +361,7 @@
 void InteractiveMove(Window *w, FvwmWindow *tmp_win, int *FinalX, int *FinalY,
 		     XEvent *eventp);
 
+MenuRoot *FindPopupGetTok(char *action);
 MenuRoot *FindPopup(char *action);
 
 void Bell(F_CMD_ARGS);
diff -Nru fvwm-2.2.orig/utils/fvwmrc_convert fvwm-2.2/utils/fvwmrc_convert
--- fvwm-2.2.orig/utils/fvwmrc_convert	Wed Oct 14 01:03:21 1998
+++ fvwm-2.2/utils/fvwmrc_convert	Wed Mar  3 23:22:06 1999
@@ -23,16 +23,17 @@
 # Originally written by Martin Kraemer <[email-masked]>
 # Corrected, extended, and modified by Grant McDorman <[email-masked]>
 # 24 May 95
-# Addtional corrections and extensions by Bill Oswald <[email-masked]>
+# Additional corrections and extensions by Bill Oswald <[email-masked]>
 # 8 Mar 96 thru 18 Jul 96
 # Minor corrections to make it easier to customize by Charles Hines 08/01/96
-#
+# Further minor corrections by Julian Gilbey <[email-masked]> based on those
+# of Austin Donnelly <[email-masked]>, 1 Mar 1999
+# Further major corrections by Julian Gilbey <[email-masked]>, 3 Mar 1999
 
 echo "fvwmrc-to-fvwm2rc"
 
 # Point this to your gawk/nawk/awk exec:
-AWK=/usr/local/bin/gawk
-#AWK=/bin/awk
+AWK=/usr/bin/awk
 
 if [ ! -x $AWK ];then
    echo "$AWK missing, cannot run"
@@ -91,7 +92,7 @@
 	next;
 }
 
-/^$/                            {   # Empty line, pass it thru
+/^[ \t]*$/                            {   # Empty line, pass it thru
 	print $0;
 	next;
 }
@@ -100,6 +101,9 @@
 /Restart/ && /fvwm/             { gsub("fvwm", "fvwm2"); } # try and leave paths alone
 /GoodStuff/                     { gsub("GoodStuff", "FvwmButtons"); }
 
+############ Chop off trailing "&" from Exec commands ##########
+/^Exec[ \t]+/ || /[ \t]+Exec[ \t]+/ { gsub ("\&[ \t]*$",""); }
+
 ################ Highlight Colors ##############
 /^[ \t]*HiBackColor[ \t]*/	{
         dflt["hibackcolor"]=hibackcolor=$2;
@@ -192,7 +196,7 @@
 /^[ \t]*ButtonStyle[ \t]*/	{
         if ($2 == ":") {  # new style already
            if (NF != $4+4)
-	       print "ERROR: ButtonStyle command incorrect" >"'$TTY'";
+	       print "ERROR: ButtonStyle command incorrect\n" NR $0 >"'$TTY'";
 	   printf ("%s %d %d", $1, $3, $4);
 	   for (i=5; i<=NF; ++i)
 	       printf (" %s", $i);
@@ -205,7 +209,7 @@
 	   x=substr($3,1,p-1)/2;
 	   y=substr($3,p+1)/2;
 	   printf ("%s %s 5 %dx%d@0 %dx%d@0 %dx%d@0 %dx%d@1 %dx%d@1\n",
-		    $1, $2, 50-x,50+y, 50+x,50+y, 50-x,50-y, 50+x,50-y,
+		    $1, $2, 50-x,50+y, 50+x,50+y, 50+x,50-y, 50-x,50-y,
 		    50-x,50+y);
 	}
 	next;
@@ -231,9 +235,9 @@
 /^[ \t]*IconFont[ \t]*/		{ print $0; next; }
 /^[ \t]*SuppressIcons[ \t]*/	{ print "Style \"*\" NoIcon"; next; }
 /^[ \t]*StickyIcons[ \t]*/	{ print "Style \"*\" StickyIcon"; next; }
-/^[ \t]*ModulePath[ \t]*/	{ gsub("/fvwm", "/fvwm2"); print $0; next; }
-/^[ \t]*PixmapPath[ \t]*/	{ gsub("/fvwm", "/fvwm2"); print $0; next; }
-/^[ \t]*IconPath[ \t]*/		{ gsub("/fvwm", "/fvwm2"); print $0; next; }
+/^[ \t]*ModulePath[ \t]*/	{ print "#! (ModulePath command commented out)\n#" $0; next; }
+/^[ \t]*PixmapPath[ \t]*/	{ print $0; next; }
+/^[ \t]*IconPath[ \t]*/		{ print $0; next; }
 # note: Icon must be followed by some white space
 /^[ \t]*Icon[ \t]+/		{ printf "Style %s Icon %s\n", $2, $3; next; }
 
@@ -275,25 +279,60 @@
 ########## Etc ##########
 /^[ \t]*Lenience[ \t]*/		{ print "Style \"*\" " $0; next; }
 /^[ \t]*Style[ \t]*/		{ print $0; next; }
+# Key keyname context mods (Exec|Module|Restart) ...
+#  used to be followed by a "name"; it is no longer
+/^[ \t]*Key[ \t]+[^ \t]+[ \t]+[^ \t]+[ \t]+[^ \t]+[ \t]+(Exec|Module|Restart)[ \t]+\"/  {
+	# not going to handle escaped quotes
+	label=$6;
+	first=7;
+	quoted=substr(label, 1, 1)=="\"" &&
+		substr(label, length(label), 1)!="\"";
+	for (i=7;i<=NF && quoted;i++) {
+		label=$i;
+		quoted=substr(label, length(label), 1)!="\"";
+		first=i + 1;
+	}
+	printf ("%s %s %s %s %s", $1, $2, $3, $4, $5);
+	for (i=first; i<=NF; ++i)
+		printf (" %s", $i);
+	printf ("\n");
+	next;
+}
+# Any other Key command
 /^[ \t]*Key[ \t]*/		{ print $0; next; }
-/^[ \t]*Mouse[ \t]*/		{
-	if (sub("[ 	]Pop[uU]p[ 	]", " Menu ")) {
-	    if (!warn["Mouse"]) {
-		print "Note: Setting mouse bindings to sticky menus">"'$TTY'";
-		warn["Mouse"] = TRUE;
-	    }
-	    if (index($6, "\"") == 1) {
-	       gsub("\"", "")
-	       if (NF > 6) {
-	          for (i = 6; i <= NF; i++)
-                    sub($i " ", $i)
-	       }
-	    } 
-	    sub("$", " Nop");
- 	}
-	print $0; next;
+
+# Similarly with Mouse commands
+/^[ \t]*Mouse[ \t]+[^ \t]+[ \t]+[^ \t]+[ \t]+[^ \t]+[ \t]+(Exec|Module|Restart)[ \t]+\"/  {
+	# not going to handle escaped quotes
+	label=$6;
+	first=7;
+	quoted=substr(label, 1, 1)=="\"" &&
+		substr(label, length(label), 1)!="\"";
+	for (i=7;i<=NF && quoted;i++) {
+		label=$i;
+		quoted=substr(label, length(label), 1)!="\"";
+		first=i + 1;
+	}
+	printf ("%s %s %s %s %s", $1, $2, $3, $4, $5);
+	for (i=first; i<=NF; ++i)
+		printf (" %s", $i);
+	printf ("\n");
+	next;
 }
 
+/^[ \t]*Mouse[ \t]*/		{ print $0; next; }
+# This is probably not necessary
+# /^[ \t]*Mouse[ \t]*/		{
+# 		 if (sub("[ \t]Pop[uU]p[ \t]", " Menu ")) {
+# 			  if (!warn["Mouse"]) {
+# 			 print "Note: Setting mouse bindings to sticky menus">"'$TTY'";
+# 			 warn["Mouse"] = TRUE;
+# 			  }
+# 			  sub("$", " Nop");
+# 		 }
+# 		 print $0; next;
+# }
+
 /^[ \t]*WindowListSkip[ \t]*/	{ printf "Style %s WindowListSkip", $2; next; }
 /^[ \t]*WindowFont[ \t]*/	{ print $0; next; }
 /^[ \t]*ClickTime[ \t]*/		{ print $0; next; }
@@ -307,12 +346,12 @@
 	next;
 }
 
-/^[ \t]**FvwmButtons[ \t]*/	{
+/^[ \t]*FvwmButtons[ \t]*/	{
 	sub("[ 	]Swallow[ 	]*[^ 	]*", "& Exec");
 	print $0;
 	if (length($0) > 199)
 	{
-	    print "Warning: line too long" >"'$TTY'";
+	    print "Warning: line" NR "too long" >"'$TTY'";
 	    print ">> " $0 >"'$TTY'";
 	}
 	if (!warn["GoodStuff"])
@@ -362,19 +401,26 @@
         infunction=FALSE;
 	inpopup=TRUE;
 
+	label=$2;
 	if (index($2, "\"") == 1) {
-	   tstr = ""
-	   for (i = 2; index(substr($i, 2), "\"") < length($i) && i <= NF; i++)
-              tstr = sprintf("%s%s", tstr, $i);
-	   prefix = "AddToMenu " substr(tstr, 2, length(tstr) - 2)
-        } else
-	   prefix="AddToMenu " $2;
+	   # not going to handle escaped quotes
+	   first=3;
+	   quoted=substr(label, 1, 1)=="\"" &&
+	      substr(label, length(label), 1)!="\"";
+	   for (i=3;i<=NF && quoted;i++) {
+	      label=label " " $i;
+	      quoted=substr(label, length(label), 1)!="\"";
+	      first=i + 1;
+	   }
+	}
+
+	prefix="AddToMenu " label;
 	next;
 }
 
 /^[ \t]*EndPopup[ \t]*/		{
         if (!inpopup)
-	   print "ERROR: EndPopup outside of popup" >"'$TTY'";
+	   print "ERROR: EndPopup outside of popup\n" NR $0 >"'$TTY'";
         inpopup=FALSE;
 	prefix="";
 	next;
@@ -400,7 +446,17 @@
 }
 
 /^[ \t]*Pager[ \t]*/		{
-        print "#! " $0 " [deleted]";
+	print "#! " $0 " [deleted]";
+	print "#! An FvwmPager config to emulate builtin pager would be as follows:"
+	print "# *FvwmPagerBack SlateGrey";
+	print "# *FvwmPagerFore Black";
+	print "# *FvwmPagerFont none";
+	print "# *FvwmPagerHilight SlateBlue4";
+	print "# *FvwmPagerGeometry <xsize> <ysize>";
+	print "# *FvwmPagerSmallFont -*-times-medium-r-*-*-*-80-*-*-*-*-*-*";
+	print "# *FvwmPagerDeskTopScale 32";
+	print "# #! Now start pager";
+	print "# Module FvwmPager 0 0";
 	if (warned[$1]==FALSE) {
            print "Warning: " $1 " omitted, internal pager is obsolete (use FvwmPager)" >"'$TTY'";
 	   warned[$1] = TRUE;
--
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: Thu, 4 Mar 1999 18:00:16 +0100

Hi Julian,

> +++ fvwm-2.2/fvwm/builtins.c	Thu Mar  4 01:04:22 1999

could you please recreate your patch in a way that it
doesn't contain the name of your fvwm directory?
Usually you should create patches from within the
fvwm directory and use

  # diff -u some_file.orig some_file

This way you don't have the version number in the patch either.

Bye

Dominik

--
Dominik Vogt, [email-masked]
Reply-To: [email-masked]

---

From: [email-masked] (Paul D. Smith)
Date: 04 Mar 1999 12:22:34 -0500

%% Dominik Vogt <[email-masked]> writes:

  dv> Hi Julian,
  >> +++ fvwm-2.2/fvwm/builtins.c	Thu Mar  4 01:04:22 1999

  dv> could you please recreate your patch in a way that it
  dv> doesn't contain the name of your fvwm directory?

Note you can still apply it using the -p option to patch.  It is simpler
if you don't have to do this, I agree.

-- 
-------------------------------------------------------------------------------
 Paul D. Smith <[email-masked]>         Network Management Development
 "Please remain calm...I may be mad, but I am a professional." --Mad Scientist
-------------------------------------------------------------------------------
   These are my opinions---Nortel Networks takes no responsibility for them.
--
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: Julian Gilbey <[email-masked]>
Date: Fri, 5 Mar 1999 02:09:46 +0000 (GMT)

>   # diff -u some_file.orig some_file

(As root?!)

> This way you don't have the version number in the patch either.

(1)  cd fvwm-2.2/fvwm
     patch -p2 < patchfile

    will ignore everything in the filenames up to and including the
    second slash.  Standard practice: diff -Nru dir.orig dir  gives a
    full patchfile from an original source tree to a modified one, and
    it is applied using patch -p1.

(2) Having the version numbers in the top directory names makes it
    extremely clear to anyone looking at the file which version it is
    patched against.  Who knows whether it would work or be correct in
    a different version?

Just my 2c on this one ;-)

   Julian

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

  Julian Gilbey, Dept of Maths, QMW, Univ. of London. [email-masked]
             Debian GNU/Linux Developer.  [email-masked]
       -*- Finger [email-masked] for my PGP public key. -*-
--
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: Sat, 6 Mar 1999 15:06:08 +0100

> >   # diff -u some_file.orig some_file

Huh? what does this mean?

> > This way you don't have the version number in the patch either.

What will happen to the original file then? A patch created with

will create some_file.orig when applied which is *very* useful for
me. I don't create a copy of my source tree before applying every
single patch.

>     it is applied using patch -p1.

OK, then we don't adhere to the standard practice. Anyway, for me
as a developer it's quite cumbersome to fiddle with version numbers.
This way I have to look at the parts of the patch to find out where
and with which options I have to apply it.

>     patched against.  Who knows whether it would work or be correct in

Well, this is correct if you want to publish them as 'unofficial
patches', but if you want them in CVS this would prevent them from
ever being committed. The CVS tree is always changing so the chance
that your patch is against the current CVS source is close to zero.

Bye

Dominik

---

From: Julian Gilbey <[email-masked]>
Date: Sun, 7 Mar 1999 03:38:29 +0000 (GMT)

> Huh? what does this mean?

It means: 'a shell prompt of "#" usually indicates that one is root; I
was surprised by this.'  8-)

> me. I don't create a copy of my source tree before applying every

[Comments which follow apply to the GNU patch command; I'm not certain
if they apply to all versions.]

patch, when run with the option -b will create the .orig files.
Without this option, it will probably only create the .orig if the
patch failed.  A multiple-patch file like the one which I created will
then create .orig files for each patched file.

> and with which options I have to apply it.

Maybe I'll just give the required command at the top of the email next
time to make people's lives easier.  No problem!

> that your patch is against the current CVS source is close to zero.

Absolutely, but the fact that they say that they are patches against
2.2 will stop people wondering why they don't work properly with the
latest CVS version ;-)

Also, I just checked up and one could pipe my original email into the
command:

  patch -b -d <root of FVWM source tree> -p1
or
  patch --backup --directory <root of FVWM source tree> --strip=1

which would have patched everything.  Cool, eh?  We love Larry Wall!!

   Julian

P.S.  One last thought: is there any intention to release patched
versions of 2.2 as bugs are found and ironed out?

---

From: Julian Gilbey <[email-masked]>
Date: Sun, 7 Mar 1999 03:38:29 +0000 (GMT)

Julian

---

From: [email-masked] (Paul D. Smith)
Date: 07 Mar 1999 00:21:33 -0500

%% Julian Gilbey <[email-masked]> writes:

  jg> P.S.  One last thought: is there any intention to release patched
  jg> versions of 2.2 as bugs are found and ironed out?

People have been making very selected bugfixes to the current 2.2.1
branch; I assume at some point we'll release that version.

---

From: Dominik Vogt <[email-masked]>
Date: Sun, 7 Mar 1999 15:21:01 +0100

> was surprised by this.'  8-)

OK, I always use '#' since the '>' prompt I use is a bit irritating
in emails.

>... 

That would be nice :)

> 2.2 will stop people wondering why they don't work properly with the

Hmm, I never assume patches are against the current CVS source. Usually
I try to apply them and if it fails I check if it's easy to fix. If not
I just ask the submitter to update the patch (very rarely).

> Also, I just checked up and one could pipe my original email into the

Yes that's a really nice feature of patch. So you can archive the patch
and its description in one file :)

Bye

Dominik