Subject: Re: StyleById patch voting thread

From: Olivier Chapuis <[email-masked]>
Date: Sat, 7 Jun 2003 19:20:12 +0200

On Mon, May 19, 2003 at 01:44:27PM +0200, Dominik Vogt wrote:
> Should the StyleById patch be applied before 2.6?  Please cast
> your votes here.
>

Seems that there is no conclusion here. It seems that there is two
votes for it (me and Mikhael) one vote against (Dominik) and one
unclear vote (Dan). So I ask for more votes and clarification
(Dan?). For that I send an other version of the patch (attached).
I've followed all the advice (I can follow) that I get in this
thread.  In particular, I've followed all the remarks (as I can) of
Dominik regarding the code. So, Dominik I even hope you revert your
vote (very little hope ...). Also: the cmds are named WindowStyle and
DestroyWindowStyle and act on the selected window and there is the
doc and some tests in purify.fvwm2rc.

Regards, Olivier

PS: What are the rules for the votes? 
Index: fvwm/add_window.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/add_window.c,v
retrieving revision 1.348
diff -u -r1.348 add_window.c
--- fvwm/add_window.c	27 May 2003 00:37:00 -0000	1.348
+++ fvwm/add_window.c	7 Jun 2003 17:13:26 -0000
@@ -3014,6 +3014,18 @@
 		return;
 	}
 
+	/* remove window style */
+	if (!IS_SCHEDULED_FOR_DESTROY(fw) && !DO_REUSE_DESTROYED(fw))
+	{
+		style_id_t s_id;
+
+		memset(&s_id, 0, sizeof(style_id_t));
+		SID_SET_WINDOW_ID(s_id, (XID)FW_W(fw));
+		SID_SET_HAS_WINDOW_ID(s_id, True);
+
+		style_destroy_style(s_id);
+	}
+
 	/****** remove from window list ******/
 	/* if the window is sheduled fro destroy the window has been already
 	 * removed from list */
Index: fvwm/commands.h
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/commands.h,v
retrieving revision 1.36
diff -u -r1.36 commands.h
--- fvwm/commands.h	3 Jan 2003 06:57:35 -0000	1.36
+++ fvwm/commands.h	7 Jun 2003 17:13:27 -0000
@@ -167,6 +167,7 @@
 	F_DESTROY,
 	F_DESTROY_DECOR,
 	F_DESTROY_MOD,
+	F_DESTROY_WINDOW_STYLE,
 	F_ECHO,
 	F_FLIP_FOCUS,
 	F_FOCUS,
@@ -194,6 +195,7 @@
 	F_WARP,
 	F_WINDOWID,
 	F_WINDOW_SHADE,
+	F_WINDOW_STYLE,
 
 	F_END_OF_LIST = 999,
 
@@ -260,6 +262,7 @@
 void CMD_DestroyMenuStyle(F_CMD_ARGS);
 void CMD_DestroyModuleConfig(F_CMD_ARGS);
 void CMD_DestroyStyle(F_CMD_ARGS);
+void CMD_DestroyWindowStyle(F_CMD_ARGS);
 void CMD_Direction(F_CMD_ARGS);
 void CMD_Echo(F_CMD_ARGS);
 void CMD_EdgeCommand(F_CMD_ARGS);
@@ -383,6 +386,7 @@
 void CMD_WindowList(F_CMD_ARGS);
 void CMD_WindowShade(F_CMD_ARGS);
 void CMD_WindowShadeAnimate(F_CMD_ARGS);
+void CMD_WindowStyle(F_CMD_ARGS);
 void CMD_Xinerama(F_CMD_ARGS);
 void CMD_XineramaPrimaryScreen(F_CMD_ARGS);
 void CMD_XineramaSls(F_CMD_ARGS);
Index: fvwm/functable.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/functable.c,v
retrieving revision 1.23
diff -u -r1.23 functable.c
--- fvwm/functable.c	11 Mar 2003 08:54:44 -0000	1.23
+++ fvwm/functable.c	7 Jun 2003 17:13:29 -0000
@@ -220,6 +220,10 @@
 	CMD_ENT("destroystyle", CMD_DestroyStyle, F_DESTROY_STYLE, 0, 0),
 	/* - Delete style defined using Style */
 
+	CMD_ENT("destroywindowstyle", CMD_DestroyWindowStyle,
+		F_DESTROY_WINDOW_STYLE, FUNC_NEEDS_WINDOW, CRS_SELECT),
+	/* - Delete style defined using WindowStyle */
+
 	CMD_ENT("direction", CMD_Direction, F_DIRECTION, 0, 0),
 	/* - Operate on the next window in the specified direction */
 
@@ -619,6 +623,10 @@
 	CMD_ENT("windowshadeanimate", CMD_WindowShadeAnimate, F_SHADE_ANIMATE,
 		0, 0),
 	/* - (obsolete, use Style * WindowShadeSteps) */
+
+	CMD_ENT("windowstyle", CMD_WindowStyle, F_WINDOW_STYLE,
+		FUNC_NEEDS_WINDOW, CRS_SELECT),
+	/* - Set styles on the selected window */
 
 	CMD_ENT("xinerama", CMD_Xinerama, F_XINERAMA, 0, 0),
 	/* - Control Xinerama support */
Index: fvwm/fvwm.1.in
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/fvwm.1.in,v
retrieving revision 1.92
diff -u -r1.92 fvwm.1.in
--- fvwm/fvwm.1.in	3 Jun 2003 19:22:31 -0000	1.92
+++ fvwm/fvwm.1.in	7 Jun 2003 17:14:07 -0000
@@ -5769,10 +5769,16 @@
 all styles starting with "Application".
 
 .TP
+.B "DestroyWindowStyle"
+deletes the styles set by the
+.B WindowStyle
+command on the selected window. The changes take effect immediately.
+
+.TP
 .BI "UpdateStyles"
 All pending updates of all windows' styles and looks are applied
 immediately.  E.g. if
-.BR Style " or " TitleStyle
+.BR Style ", " WindowStyle " or " TitleStyle
 commands were issued inside a fvwm function.
 
 .TP
@@ -7540,6 +7546,14 @@
 Style * NoIcon
 Style FvwmPager Icon
 .EE
+
+.TP
+.BI "WindowStyle " "options"
+sets attributes (styles) on the selected window. The
+.I options  
+are exactly the same as for the
+.B Style
+command.
 
 .SS OTHER COMMANDS CONTROLLING WINDOW STYLES
 
Index: fvwm/fvwm.h
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/fvwm.h,v
retrieving revision 1.220
diff -u -r1.220 fvwm.h
--- fvwm/fvwm.h	2 Jan 2003 12:01:18 -0000	1.220
+++ fvwm/fvwm.h	7 Jun 2003 17:14:10 -0000
@@ -541,12 +541,24 @@
 	unsigned has_placement_percentage_penalty : 1;
 } style_flags;
 
+typedef struct
+{
+	char *name;
+	XID window_id;
+	struct
+	{
+		unsigned has_name:1;
+		unsigned has_window_id:1;
+	} flags;
+} style_id_t;
+
+
 /* only style.c and add_window.c are allowed to access this struct!! */
 typedef struct window_style
 {
 	struct window_style *next;
 	struct window_style *prev;
-	char *name;
+	style_id_t id;
 #if 0
 	WindowConditionMask *condition_mask;
 #endif
Index: fvwm/style.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/style.c,v
retrieving revision 1.210
diff -u -r1.210 style.c
--- fvwm/style.c	21 May 2003 22:58:58 -0000	1.210
+++ fvwm/style.c	7 Jun 2003 17:14:18 -0000
@@ -120,6 +120,49 @@
 	return True;
 }
 
+Bool style_ids_are_equals(style_id_t a, style_id_t b)
+{
+	if (SID_GET_HAS_NAME(a) && SID_GET_HAS_NAME(b) &&
+	    !strcmp(SID_GET_NAME(a), SID_GET_NAME(b)))
+	{
+		return True;
+	}
+	if (SID_GET_HAS_WINDOW_ID(a) && SID_GET_HAS_WINDOW_ID(b) &&
+	    SID_GET_WINDOW_ID(a) == SID_GET_WINDOW_ID(b))
+	{
+		return True;
+	}
+	return False;
+}
+
+Bool style_id_equals_id(window_style s, style_id_t id)
+{
+	return style_ids_are_equals(SGET_ID(s), id);
+}
+
+Bool styles_have_same_id(window_style s, window_style t)
+{
+	return style_ids_are_equals(SGET_ID(s), SGET_ID(t));
+}
+
+Bool fw_match_style_id(FvwmWindow *fw, style_id_t s_id)
+{
+	if (SID_GET_HAS_NAME(s_id) &&
+	    (matchWildcards(SID_GET_NAME(s_id), fw->class.res_class) == TRUE ||
+	     matchWildcards(SID_GET_NAME(s_id), fw->class.res_name) == TRUE ||
+	     matchWildcards(SID_GET_NAME(s_id), fw->name.name) == TRUE))
+	{
+		return True;
+	}
+	if (SID_GET_HAS_WINDOW_ID(s_id) &&
+	    SID_GET_WINDOW_ID(s_id) == (XID)FW_W(fw))
+	{
+		return True;
+	}
+
+	return False;
+}
+
 static void remove_icon_boxes_from_style(window_style *pstyle)
 {
 	if (SHAS_ICON_BOXES(&pstyle->flags))
@@ -724,7 +767,7 @@
 	}
 }
 
-static Bool remove_all_of_style_from_list(char *style_ref)
+static Bool remove_all_of_style_from_list(style_id_t style_id)
 {
 	window_style *nptr = all_styles;
 	window_style *next;
@@ -735,7 +778,7 @@
 	{
 		next = SGET_NEXT_STYLE(*nptr);
 		/* Check if it's to be wiped */
-		if (!strcmp(SGET_NAME(*nptr), style_ref))
+		if (style_id_equals_id(*nptr, style_id))
 		{
 			remove_style_from_list(nptr, True);
 			is_changed = True;
@@ -776,14 +819,14 @@
 		cmp = SGET_PREV_STYLE(*cur);
 		while (cmp)
 		{
-			if (strcmp(SGET_NAME(*cur), SGET_NAME(*cmp)) != 0)
+			if (!styles_have_same_id(*cur, *cmp))
 			{
 				blockor((char *)&interflags,
 					(char *)&interflags,
 					(char *)&cmp->flag_mask,
 					sizeof(style_flags));
 				blockor((char *)&interflags,
-					(char *)&interflags,
+				(char *)&interflags,
 					(char *)&cmp->flag_default,
 					sizeof(style_flags));
 				cmp = SGET_PREV_STYLE(*cmp);
@@ -3665,7 +3708,8 @@
 			for (add_style = all_styles; add_style;
 			     add_style = SGET_NEXT_STYLE(*add_style))
 			{
-				if (StrEquals(token, SGET_NAME(*add_style)))
+				if (SGET_ID_HAS_NAME(*add_style) &&
+				    StrEquals(token, SGET_NAME(*add_style)))
 				{
 					/* match style */
 					hit = 1;
@@ -3887,7 +3931,7 @@
  * must be freed in ProcessDestroyStyle().
  */
 
-static void __style_command(F_CMD_ARGS, char *prefix)
+static void __style_command(F_CMD_ARGS, char *prefix, Bool is_window_style)
 {
 	/* temp area to build name list */
 	window_style *ps;
@@ -3904,14 +3948,25 @@
 	/* default StartsOnPage behavior for initial capture */
 	ps->flags.capture_honors_starts_on_page = 1;
 
-	/* parse style name */
-	action = GetNextToken(action, &SGET_NAME(*ps));
-	/* in case there was no argument! */
-	if (SGET_NAME(*ps) == NULL)
+	if (!is_window_style)
 	{
-		free(ps);
-		return;
+		/* parse style name */
+		action = GetNextToken(action, &SGET_NAME(*ps));
+		/* in case there was no argument! */
+		if (SGET_NAME(*ps) == NULL)
+		{
+			free(ps);
+			return;
+		}
+		SSET_ID_HAS_NAME(*ps, True);
+	}
+	else
+	{
+		SSET_WINDOW_ID(*ps, (XID)FW_W(exc->w.fw));
+		SSET_ID_HAS_WINDOW_ID(*ps, True);
+		CopyString(&SGET_NAME(*ps), ""); /* safe */
 	}
+
 	if (action == NULL)
 	{
 		free(SGET_NAME(*ps));
@@ -3922,7 +3977,7 @@
 	parse_and_set_window_style(action, prefix, ps);
 
 	/* capture default icons */
-	if (StrEquals(SGET_NAME(*ps), "*"))
+	if (SGET_ID_HAS_NAME(*ps) && StrEquals(SGET_NAME(*ps), "*"))
 	{
 		if (ps->flags.has_icon == 1)
 		{
@@ -3938,7 +3993,7 @@
 		}
 	}
 	if (last_style_in_list &&
-	    strcmp(SGET_NAME(*ps), SGET_NAME(*last_style_in_list)) == 0)
+	    styles_have_same_id(*ps, *last_style_in_list))
 	{
 		/* merge with previous style */
 		merge_styles(last_style_in_list, ps, True);
@@ -4037,18 +4092,7 @@
 	/* look thru all styles in order defined. */
 	for (nptr = all_styles; nptr != NULL; nptr = SGET_NEXT_STYLE(*nptr))
 	{
-		/* If name/res_class/res_name match, merge */
-		if (matchWildcards(SGET_NAME(*nptr),fw->class.res_class) ==
-		    TRUE)
-		{
-			merge_styles(styles, nptr, False);
-		}
-		else if (matchWildcards(SGET_NAME(*nptr),fw->class.res_name) ==
-			 TRUE)
-		{
-			merge_styles(styles, nptr, False);
-		}
-		else if (matchWildcards(SGET_NAME(*nptr),fw->name.name) == TRUE)
+		if (fw_match_style_id(fw, SGET_ID(*nptr)))
 		{
 			merge_styles(styles, nptr, False);
 		}
@@ -4770,32 +4814,30 @@
 
 void CMD_Style(F_CMD_ARGS)
 {
-	__style_command(F_PASS_ARGS, NULL);
+	__style_command(F_PASS_ARGS, NULL, False);
 
 	return;
 }
 
-void CMD_FocusStyle(F_CMD_ARGS)
+void CMD_WindowStyle(F_CMD_ARGS)
 {
-	__style_command(F_PASS_ARGS, "FP");
+	__style_command(F_PASS_ARGS, NULL, True);
 
 	return;
 }
 
-void CMD_DestroyStyle(F_CMD_ARGS)
+void CMD_FocusStyle(F_CMD_ARGS)
 {
-	char *name;
-	FvwmWindow *t;
+	__style_command(F_PASS_ARGS, "FP", False);
 
-	/* parse style name */
-	name = PeekToken(action, &action);
+	return;
+}
 
-	/* in case there was no argument! */
-	if (name == NULL)
-		return;
+void style_destroy_style(style_id_t s_id)
+{
+	FvwmWindow *t;
 
-	/* Do it */
-	if (remove_all_of_style_from_list(name))
+	if (remove_all_of_style_from_list(s_id))
 	{
 		/* compact the current list of styles */
 		Scr.flags.do_need_style_list_update = 1;
@@ -4808,9 +4850,7 @@
 	for (t = Scr.FvwmRoot.next; t != NULL && t != &Scr.FvwmRoot;
 	     t = t->next)
 	{
-		if (matchWildcards(name, t->class.res_class) == TRUE ||
-		    matchWildcards(name, t->class.res_name) == TRUE ||
-		    matchWildcards(name, t->name.name) == TRUE)
+		if (fw_match_style_id(t, s_id))
 		{
 			SET_STYLE_DELETED(t, 1);
 			Scr.flags.do_need_window_update = 1;
@@ -4820,6 +4860,40 @@
 	return;
 }
 
+void CMD_DestroyStyle(F_CMD_ARGS)
+{
+	char *name;
+	style_id_t s_id;
+
+	/* parse style name */
+	name = PeekToken(action, &action);
+
+	/* in case there was no argument! */
+	if (name == NULL)
+		return;
+
+	memset(&s_id, 0, sizeof(style_id_t));
+	SID_SET_NAME(s_id, name);
+	SID_SET_HAS_NAME(s_id, True);
+
+	/* Do it */
+	style_destroy_style(s_id);
+	return;
+}
+
+void CMD_DestroyWindowStyle(F_CMD_ARGS)
+{
+	style_id_t s_id;
+
+	memset(&s_id, 0, sizeof(style_id_t));
+	SID_SET_WINDOW_ID(s_id, (XID)FW_W(exc->w.fw));
+	SID_SET_HAS_WINDOW_ID(s_id, True);
+
+	/* Do it */
+	style_destroy_style(s_id);
+	return;
+}
+
 void print_styles(int verbose)
 {
 	window_style *nptr;
@@ -4835,7 +4909,15 @@
 		count++;
 		if (verbose)
 		{
-			fprintf(stderr,"    * %s\n", SGET_NAME(*nptr));
+			if (SGET_ID_HAS_NAME(*nptr))
+			{
+				fprintf(stderr,"    * %s\n", SGET_NAME(*nptr));
+			}
+			else
+			{
+				fprintf(stderr,"    * 0x%lx\n",
+					(unsigned long)SGET_WINDOW_ID(*nptr));
+			}
 		}
 	}
 	fprintf(stderr,"  Number of styles: %i\n", count);
Index: fvwm/style.h
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/style.h,v
retrieving revision 1.75
diff -u -r1.75 style.h
--- fvwm/style.h	21 May 2003 22:58:58 -0000	1.75
+++ fvwm/style.h	7 Jun 2003 17:14:20 -0000
@@ -351,6 +351,24 @@
 #define S_SET_EWMH_MAXIMIZE_MODE(c,x) \
 	((c).s.ewmh_maximize_mode = (x))
 
+/* access to style_id */
+#define SID_GET_NAME(id) \
+	((id).name)
+#define SID_SET_NAME(id,x) \
+	((id).name = (x))
+#define SID_GET_WINDOW_ID(id) \
+	((id).window_id)
+#define SID_SET_WINDOW_ID(id,x) \
+	((id).window_id = (x))
+#define SID_SET_HAS_NAME(id,x) \
+        ((id).flags.has_name = !!(x))
+#define SID_GET_HAS_NAME(id) \
+        ((id).flags.has_name)
+#define SID_SET_HAS_WINDOW_ID(id,x) \
+        ((id).flags.has_window_id = !!(x))
+#define SID_GET_HAS_WINDOW_ID(id) \
+        ((id).flags.has_window_id)
+
 /* access to other parts of a style (call with the style itself) */
 #define SGET_NEXT_STYLE(s) \
 	((s).next)
@@ -360,10 +378,24 @@
 	((s).prev)
 #define SSET_PREV_STYLE(s,x) \
 	((s).prev = (x))
+#define SGET_ID(s) \
+        ((s).id)
 #define SGET_NAME(s) \
-	((s).name)
+	SID_GET_NAME(SGET_ID(s))
 #define SSET_NAME(s,x) \
-	((s).name = (x))
+	SID_SET_NAME(SGET_ID(s),x)
+#define SGET_WINDOW_ID(s) \
+	SID_GET_WINDOW_ID(SGET_ID(s))
+#define SSET_WINDOW_ID(s,x) \
+	SID_SET_WINDOW_ID(SGET_ID(s),x)
+#define SSET_ID_HAS_NAME(s,x) \
+        SID_SET_HAS_NAME(SGET_ID(s), x)
+#define SGET_ID_HAS_NAME(s) \
+        SID_GET_HAS_NAME(SGET_ID(s))
+#define SSET_ID_HAS_WINDOW_ID(s,x) \
+        SID_SET_HAS_WINDOW_ID(SGET_ID(s),x)
+#define SGET_ID_HAS_WINDOW_ID(s) \
+        SID_GET_HAS_WINDOW_ID(SGET_ID(s))
 #define SGET_ICON_NAME(s) \
 	((s).icon_name)
 #define SSET_ICON_NAME(s,x) \
@@ -562,6 +594,7 @@
 void update_icon_title_cs_hi_style(FvwmWindow *fw, window_style *pstyle);
 void update_icon_background_cs_style(FvwmWindow *fw, window_style *pstyle);
 void free_icon_boxes(icon_boxes *ib);
+void style_destroy_style(style_id_t s_id);
 void print_styles(int verbose);
 
 #endif /* _STYLE_ */
Index: tests/purify/purify.fvwm2rc
===================================================================
RCS file: /home/cvs/fvwm/fvwm/tests/purify/purify.fvwm2rc,v
retrieving revision 1.41
diff -u -r1.41 purify.fvwm2rc
--- tests/purify/purify.fvwm2rc	26 May 2003 10:24:21 -0000	1.41
+++ tests/purify/purify.fvwm2rc	7 Jun 2003 17:14:33 -0000
@@ -1878,6 +1878,29 @@
 # The man page says not to do this, to-do-2.4 #130
  + I DestroyStyle *
 
+DestroyFunc WindowStyleAndUpdate
+AddToFunc CommandAndUpdate I Echo $*
++ I $*
++ I UpdateStyles
+
+AddTest "Test WindowStyle" WindowStyle-Func
+AddToFunc WindowStyle-Func
++ I CommandAndUpdate Next (window*) WindowStyle BorderWidth 5, HandleWidth 5
++ I CommandAndUpdate Next (window*) WindowStyle NoTitle
++ I CommandAndUpdate Next (window*) WindowStyle NoTitle
++ I CommandAndUpdate Next (window*) WindowStyle NoTitle, gffgghf
++ I CommandAndUpdate All (window*)  WindowStyle Sticky
++ I CommandAndUpdate All (window*)  WindowStyle Sticky
++ I CommandAndUpdate All (window*)  WindowStyle Slippery
++ I CommandAndUpdate All (window*)  WindowStyle Slippery
++ I CommandAndUpdate Next (window*) WindowStyle
++ I CommandAndUpdate Next (window*) WindowStyle
++ I CommandAndUpdate Next (window*) WindowStyle nvhjvhjvhvhv jbjhbjkbjb
++ I CommandAndUpdate Next (window*) DestroyWindowStyle
++ I CommandAndUpdate Next (window*) DestroyWindowStyle pouf
++ I CommandAndUpdate All (window*) DestroyWindowStyle
++ I CommandAndUpdate All (window*) DestroyWindowStyle
+
 DestroyFunc AddButtonStyleAndUpdate
 AddToFunc AddButtonStyleAndUpdate I Echo AddButtonStyle $*
 + I AddButtonStyle $*

---

From: Dan Espen <[email-masked]>
Date: Sat, 07 Jun 2003 14:17:41 -0400

> doc and some tests in purify.fvwm2rc.

You documented the command, and supplied test cases, so those objections
are withdrawn.

There was same talk about StyleById being temporary and you chose
WindowStyle instead of
using a null pattern to select the context window:

You used:

Next (window*) WindowStyle BorderWidth 7

instead of:

Next (window*) Style [] BorderWidth 7

I think I prefer the null pattern, but only slightly.

I don't think I have any other objections.
The code you added looks pretty good.

> PS: What are the rules for the votes? 

We never had formal rules.
I think these are fair rules:

anyone on the list can vote.
votes from people that contribute more should count for more.
votes should include reasons, the reasons matter too.

-- 
Dan Espen                           E-mail: [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: Sun, 8 Jun 2003 01:54:14 +0000

> > doc and some tests in purify.fvwm2rc.

Can you please clarify one thing. You previously said that after applying
many WindowStyle commands and switching themes the style list didn't grow.
Do I understand you correctly? Because it is the Dominik's reason against
the patch (you seem to cover several other reasons with your new patch).

> I think I prefer the null pattern, but only slightly.

I started to write an answer some weeks ago, but I didn't have a time to
complete it. I don't have the time now too. But I will try.

There were ideas 2 years ago to have powerful conditions, but instead of
duplicating them in both conditional commands and Style command, to get
rid of Style and introduce WindowStyle instead. I.e. currently unexistent

  Style (Class XTerm) NoButton 6

is replaced with:

  Next (Class XTerm) WindowStyle NoButton 6

that is placed in StyleFunction and applied to all new windows.

So, at least by these ideas, it is not clear whether to have Style
command at all in the long run. On the contrary, having WindowStyle that
does not receive any condition parameters, but works on the context
window instead is a good and complete thing.

The second reason against 2 different cases of Style (one that works on
window context and one that ignores it) is no well-defined behaviour.

I understand that you want to have class/resource patterns in Style.
Then what should this command mean for you:

  Next (XTerm) Style (Resource mozilla) TitleAtBottom

1) Only windows that are both "XTerm" and "mozilla" are affected.
2) Any mozilla is affected if there are some xterms on the screen.

I would prefer to leave the current Style as is, i.e. to ignore any
window context for the sake of consistency.

The third reason to have a separate WindowStyle command is that it
may eventually replace all state commands, i.e. "Stick" is the same as
"WindowStyle Sticky". But this is not really a separate reason, because
you may say that "Style [] Sticky" will do it. It is possible, but not
as clean as "WindowStyle Sticky" for me.

Regards,
Mikhael.

---

From: Dan Espen <[email-masked]>
Date: Sat, 07 Jun 2003 22:18:20 -0400

> as clean as "WindowStyle Sticky" for me.

I think I'm starting to understand.

The Style command stays around, it affects
all current windows that match and any new window that
match.

The windowstyle command only affects the context
window that the Next, All, etc. window select.

The command you show above:

doesn't make sense.  It sould only get executed if there
was an XTerm window somewhere, but then it would change
Mozilla windows, and any future Mozilla windows.

WindowStyle does make sense as a separate command.
When it gets processed, it gets turned into a style
with an ID so that if there is a restart, it can get reapplied
to the window.

In the case you give above:

I don't see how that can replace the Style command, unless
we also saved the "Next" part of the command.
Even then, its not quite the same, Next only executes
when it matches something.  The current Style command
doesn't have to match anything to work.

---

From: Mikhael Goikhman <[email-masked]>
Date: Sun, 8 Jun 2003 02:28:19 +0000

> doesn't have to match anything to work.

Well, the original idea that removes Style command completely was:

  AddToFunc StyleFunction I Next (Class XTerm) WindowStyle NoButton 6

and StyleFunction is applied on all new windows.

But I would not discuss this idea now. I am sure we will have many
discussion after 2.6.0. :)  Even if we don't implement this idea and
implement "Style (Class XTerm, Resource rxvt)" syntax instead, still
I think it is good to have 2 separate commands: Style that ignores
window context and WindowStyle that requires window context.

Regards,
Mikhael.

---

From: Dan Espen <[email-masked]>
Date: Sat, 07 Jun 2003 22:37:23 -0400

> and StyleFunction is applied on all new windows.

Something like an "AddWindowHook".

> window context and WindowStyle that requires window context.

Agreed.

---

From: Olivier Chapuis <[email-masked]>
Date: Sun, 8 Jun 2003 10:18:05 +0200

> the patch (you seem to cover several other reasons with your new patch).

Yes, with fvwm-themes I've tried to switch and add some window style,
switch and add some window style ..etc and the style list does not
grow (in a not reasonable way). However, one can succeed to found a
way for making the style list to explode (with a limited number of
name) with or without the WindowStyle command. I think you have to do
very strange things for this (with or without the WindowStyle
command).

By the way, I think we may improve a bit __simplify_style_list, by
remplacing:

if (!styles_have_same_id(*cur, *cmp))
{
	blockor((char *)&interflags,
		(char *)&interflags,
		(char *)&cmp->flag_mask,
		sizeof(style_flags));
	blockor((char *)&interflags,
		(char *)&interflags,
		(char *)&cmp->flag_default,
		sizeof(style_flags));
	cmp = SGET_PREV_STYLE(*cmp);
	continue;
}

by something like:

Bool one_fw_can_match_both_ids(window_style s, window_style t)
{
	if (SGET_ID_HAS_WINDOW_ID(s) && SGET_ID_HAS_WINDOW_ID(t) &&
	    SGET_WINDOW_ID(s) != SGET_WINDOW_ID(t))
	{
		return False;
	}
	if (SGET_ID_HAS_NAME(s) && SGET_ID_HAS_NAME(t))
	{
		if !(there exists a string which match 
		     SGET_NAME(s) and SGET_NAME(t))
		{
			return False;
		}
	}
	return True;
}

if (!styles_have_same_id(*cur, *cmp))
{
	if (one_fw_can_match_both_ids(*cur,*cmp))
	{
		blockor((char *)&interflags,
			(char *)&interflags,
			(char *)&cmp->flag_mask,
			sizeof(style_flags));
		blockor((char *)&interflags,
			(char *)&interflags,
			(char *)&cmp->flag_default,
			sizeof(style_flags));
	}
	cmp = SGET_PREV_STYLE(*cmp);
	continue;
}

but I've to think more and we have to found a cleaver algo for: 
 "there exists a string which match SGET_NAME(s) and SGET_NAME(t))"

Regards, Olivier

---

From: Tim Phipps <[email-masked]>
Date: Sun, 08 Jun 2003 14:24:58 +0100

>unclear vote (Dan). So I ask for more votes and clarification
I've got some free time now and I was thinking of implementing the 
WindowStyle command that was proposed ages ago. I think this means I 
vote no (not very strongly) but I'd appreciate some help in reviewing 
the proposal in the light of the current state of Fvwm.

Cheers,
Tim.

---

From: Mikhael Goikhman <[email-masked]>
Date: Sun, 8 Jun 2003 13:37:42 +0000

> the proposal in the light of the current state of Fvwm.

Can you please provide some arguments?

Do you see your proposed WindowStyle command any different (in terms of
syntax and meaning) from the new command added by the posted patch?
For me they are identical, so your ideas become more real in the long run.

Or you mean that there may be the different implementation after 2.6.0?
Because starting to replace Style now before 2.6.0 is not a possibility.

Regards,
Mikhael.

---

From: Mikhael Goikhman <[email-masked]>
Date: Sun, 8 Jun 2003 13:40:52 +0000

> and StyleFunction is applied on all new windows.

There is a typo, it should be of course ThisWindow, not Next:

  AddToFunc StyleFunction I ThisWindow (Class XTerm) WindowStyle NoButton 6

Regards,
Mikhael.

---

From: Tim Phipps <[email-masked]>
Date: Mon, 09 Jun 2003 12:08:39 +0100

>Because starting to replace Style now before 2.6.0 is not a possibility.
I don't mean to change or remove the Style command so I don't want to 
wait until after 2.6.0. The old proposal is to create a new command 
"WindowStyle" that sets the style of the target window. It would be 
called as "WindowStyle Sticky" in which case it operates on the target 
window or as "WindowID XXX (<conditions>) WindowStyle Sticky" in which 
case it operates on the window XXX.  The WindowStyle command would not 
do any pattern matching, It would use the pattern matching of WindowId 
so that if the WindowId command were to be upgraded to use a 
"Class=<pattern>" that syntax would be usable in the WindowStyle and 
Style commands for free.

The old proposal is also to change the Style command to manipulate a 
"StyleFunction" function rather than have it maintain a style list. The 
StyleFunction would be run on every window that is created and would 
consist of a sequence of "WindowId XXX (<condition>) WindowStyle YYY" 
lines. The proposal would have to implement a command to delete lines 
from a function as a method of controlling the growth of the style list. 
As a debug aid there would be a PrintFunction command which would 
probably be useful on its own anyway.

As I see it the new StyleByID proposal would complicate the (already 
complicated?) Style function and style list management. The old proposal 
would simplify the style list management.

The new proposal would require extra code to handle the deletion of 
windows, the old proposal does not need to do this.

The new proposal may change the behaviour of Recapture by making the 
StyleById settings more powerful than simple Style commands, I'm not 
sure if this is the intention. If it isn't then the Recapture command 
would have to delete all the Style "id=XXX" settings from the style list 
or do something clever.

Cheers,
Tim.

---

From: Mikhael Goikhman <[email-masked]>
Date: Mon, 9 Jun 2003 11:59:58 +0000

On 09 Jun 2003 12:08:39 +0100, Tim Phipps wrote:
> > > WindowStyle command that was proposed ages ago. I think this means
> > > I vote no (not very strongly) but I'd appreciate some help in
> > > reviewing the proposal in the light of the current state of Fvwm.
> > 

The updated Olivier's patch in the parent message does exactly this.
No less and no more. Actually it also adds DestroyWindowStyle command.

> probably be useful on its own anyway.

Should changing "the Style command to manipulate a StyleFunction" be done
before 2.6.0 or after it?

I suggest to use "ThisWindow" as a better syntax than "WindowId $[w.id]".

> would simplify the style list management.

While I agree that the old proposal may be a good thing, it is unclear
whether its implementation is easier or open to less bugs than the
Olivier's patch.

> would have to delete all the Style "id=XXX" settings from the style list 

As I understand the patch, window id entries are not more powerful than
the usual Style entries. There was a discussion to make them more
powerful by supplying an optional WindowStyle Permanent prefix.

For example this should reduce the style list to only Sticky entry:

  Next (XTerm) WindowStyle !Sticky
  Style XTerm Sticky

For me, the Olivier's patch is the safe thing to apply now before 2.6.0.
Trying to change the Style command by replacing style list with
StyleFunction before 2.6.0 is problematic. This maybe done after the new
stable release or it requires a separate voting. At least I hope you
agree with the syntax and meaning of the WindowStyle command as added by
the Olivier patch, this is more important than the actual implementation.

Regards,
Mikhael.

---

From: Mikhael Goikhman <[email-masked]>
Date: Mon, 9 Jun 2003 14:25:34 +0000

> the Olivier patch, this is more important than the actual implementation.

I think that now I understand what you meant better.

Recapture should restore only Style setting for the window and not
WindowStyle settings. This is a small difference between your WindowStyle
and Olivier's WindowStyle. I don't know which behaviour I prefer, both
may be reasonable in different situations. I may even think about 3
usages of WindowStyle in the future, but this is not important now:

  WindowStyle           <flag> <setting>  # just like with Olivier patch
  WindowStyle Permanent <flag> <setting>  # use the separate win-id list
  WindowStyle OnlyState <flag> <setting>  # do not store it in any lists

Just noticed, there is a bug in WindowStyle_proposal.txt:

  # Style XXX YYY is equivalent to SetOneStyle XXX YYY
  AddToFunc SetOneStyle  
  + I AddToFunc StyleFunction I ThisWindow ($0) WindowStyle $1 $2      
  + I All StyleFunction

The last line should be:

  + I All ($0) WindowStyle $1 $2

There are some large issues with WindowStyle_proposal.txt to solve, like
reduction of the StyleFunction otherwise it will grow infinitelly when
switching themes, here the current Style is more optimal. Also currently
multiple Style commands are queued and executed at once unless
UpdateStyles is given.

I really prefer the Olivier solution before 2.6.0. I am not even sure it
is not the best for the long run. Style list with all its optimizations
is not nessesarily a bad thing, so adding WindowStyle to it may be good.

Regards,
Mikhael.

---

From: Tim Phipps <[email-masked]>
Date: Fri, 13 Jun 2003 17:00:39 +0100

>is not nessesarily a bad thing, so adding WindowStyle to it may be good.
Thinking some more about this I think the patch should be applied. It 
doesn't stop us changing the internal format of the style list in the 
future and if it works well there will be no need anyway.

Cheers,
Tim.

---

From: [email-masked]
Date: Thu, 19 Jun 2003 15:50:34 +0200

> before 2.6.0 or after it?

It should be done neither before nor after 2.6.0.  Given the state
of the function execution code, that would yield *horrible*
performance.

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: [email-masked]
Date: Thu, 19 Jun 2003 15:51:50 +0200

On Sat, Jun 07, 2003 at 07:20:12PM +0200, Olivier Chapuis wrote:
> Dominik regarding the code.

> So, Dominik I even hope you revert your vote (very little hope
> ...).

I don't know about your hope, but the chance that I do is zero. It
is not the right time.

> Also: the cmds are named WindowStyle and

Bye

Dominik ^_^  ^_^

---

From: Olivier Chapuis <[email-masked]>
Date: Tue, 24 Jun 2003 06:40:50 +0200

> I don't know about your hope, but the chance that I do is zero. It

The reason of my hope is that the new version of the patch take in
account the reasons why you reject it. You give 7 arguments.  The new
patch handle arguments 3, 4 and 6. Moreover, I've worked on argument 5
and style list simplification has been improved.  About 1 and 2 I can
just say that I do not _think_ that the patch can cause instability.
Finally, I do not understand argument 7.

But, maybe the most important is that there is 4 votes for the
patch (Dan, Mikhael, Olivier and Tim) and one vote against (Dominik).
So, I think we should apply the patch.

Regards, Olivier

---

From: Dominik Vogt <[email-masked]>
Date: Tue, 24 Jun 2003 09:39:47 +0200

On Tue, Jun 24, 2003 at 06:40:50AM +0200, Olivier Chapuis wrote:
> account the reasons why you reject it.

Nothing on earth can make me agree to adding huge features in
feature freeze.

> You give 7 arguments.  The new patch handle arguments 3, 4 and 6.

> Moreover, I've worked on argument 5 and style list
> simplification has been improved.

As I said before, I reject making facts now that can never be
changed again.  The way the style list works has always been a
mistake.  No matter how much time you spend on optimising the
style list code, it will always be a memory leak.  This logic is
due for replacement, not for expansion.

> About 1 and 2 I can just say that I do not _think_ that the
> patch can cause instability.

It has already consumed much time in pointless arguments that
could have been spent stabilising the code.  There is a direct
relationship between my motivation to work on the remaining
problems and the amount of distractions on this list that have
nothing to do with it.

> Finally, I do not understand argument 7.

It's an ad-hoc syntax that *will* be thrown away right when we
start thinking.  I have many enhancements in mind that have no
chance of being implemented with this kind of just-throw-in-a-new-
command-whenever-it-seems-to-work-for-the-moment approach.

> patch (Dan, Mikhael, Olivier and Tim) and one vote against (Dominik).

Of course Dan can speak for himself, but according to the mail
archive he did neither vote for nor against the patch.  Not that I
think it matters.

> So, I think we should apply the patch.

Then take the consequences and abandon the idea of a stable 2.6
release.

Bye

Dominik ^_^  ^_^

---

From: Olivier Chapuis <[email-masked]>
Date: Thu, 26 Jun 2003 09:07:27 +0200

> Nothing on earth can make me agree to adding huge features in

It is not a huge feature, it is one of the few features that fvwm miss
to become a perfect window manager. So let us have it!!
  
> due for replacement, not for expansion.

I will say that this has almost nothing to do with the WindowStyle
cmd. This is a general problem regarding styles. Any way, I will
be interested to see a natural situation where the style list
go to infinity (with a fixed number of names). 

> command-whenever-it-seems-to-work-for-the-moment approach.

I do not see the problem. If I follow your logic the Style command
will drastically change. Moreover, I cannot imagine that we
rework the style stuff and that at the end it will be not possible
to set "window attributes" on a given window.
  
> archive he did neither vote for nor against the patch.  Not that I

In general Dan does not vote. He gives arguments. At the end there is
no arguments against. But yes it is possible that Dan use abstention.

> Then take the consequences and abandon the idea of a stable 2.6

Why?

Regards, Olivier

---

From: Dan Espen <[email-masked]>
Date: Fri, 27 Jun 2003 22:07:37 -0400

> to become a perfect window manager. So let us have it!!

Looking at the patch, I agree, its not huge.
Its not minor either.

> go to infinity (with a fixed number of names). 

I don't agree that the Style code is fundamentally broken.
But I know from past experience that Dominik's standards are
higher than mine.  To me, the style code does the job
so I don't see a problem with it.  The way matches work
may seem odd, but I don't think there's a better way.

I don't understand the comment about the memory leaks.
I remember Dominik doing some work to combine styles as they
were issued.  There shouldn't be any leaking going on.

> no arguments against. But yes it is possible that Dan use abstention.

I agree with Dominik, my vote hardly matters.
As I said before, I think votes should be in proportion to contribution.

In this case, I've been agreeing with both sides.
Dominik has some very good points.
I don't agree about the Style code being a problem and therefore
shouldn't be extended, but I do agree that we need to freeze
features to create a stable release.

I think Oliviers point about this nicely rounding out
the feature set, and not being a big change is
very good.  Also, he supplied test cases, so
it shouldn't hold back 2.6 on that account.

> > Then take the consequences and abandon the idea of a stable 2.6

I don't want to put words in Dominiks' mouth, but I think
he's been pretty clear, he doesn't want this patch applied.
See his sentence above about motivation.

Considering Dominik's strong opinion, it might be a good idea
to hold that patch for a while.
Use your judgement.

Did I vote?

I guess I should, even if it doesn't count for much.

Damn, I can't make up my mind.
I give up, I was hoping I could come up with some argument
or line of reasoning that would settle this, but I admit,
its beyond me.  I'm not going to vote.

Try to see the other persons viewpoint and then ask yourself,
can I give in on this a little for the sake of the team.

---

From: [email-masked]
Date: Sun, 29 Jun 2003 13:02:58 +0200

On Fri, Jun 27, 2003 at 10:07:37PM -0400, Dan Espen wrote:
[snip]
> I agree with Dominik, my vote hardly matters.

I didn't say your vote hardly matters.  I said the difference
between abstaining and voting for the patch hardly matters in
this specific situation since most people seem to be mad about
committing the patch anyway.

> As I said before, I think votes should be in proportion to

Really, I think once voting has become arguing it isn't worth the
effort anymore.  The idea was to reach a consensus while in
feature freenze or even code freeze.  But this requires talking
with each other.  We have not done this in a long time, and
frankly, because of the events around the licensing argument, I
can not stand to talk with some of the people here anymore.
All that still can happen in this thread is to alienate me even
further from the project.

Dominik ^_^  ^_^