Re: style matching in current CVS
On Thu, Apr 06, 2006 at 11:13:03AM +0200, Dominik Vogt wrote:
First, this code from style.c generates a warning:
if (SID_GET_HAS_NAME(s_id) (fw-style_name == NULL
(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)) ||
matchWildcards(SID_GET_NAME(s_id), fw-style_name ) == TRUE)
{
return True;
}
style.c: In function `fw_match_style_id':
style.c:176: warning: suggest parentheses around within ||
Second, the code may access a NULL pointer style_name if neither
of the first three matchWildcards is true.
Also, I think even if style_name is not NULL, the usual style name
matching should be done:
if (SID_GET_HAS_NAME(s_id))
{
if (matchWildcards(SID_GET_NAME(s_id), fw-class.res_class) ==
TRUE)
{
return True;
}
if (matchWildcards(SID_GET_NAME(s_id), fw-class.res_name) ==
TRUE)
{
return True;
}
if (matchWildcards(SID_GET_NAME(s_id), fw-name.name) == TRUE)
{
return True;
}
if (fw-style_name == NULL
matchWildcards(SID_GET_NAME(s_id), fw-class.res_class) ==
TRUE)
{
return True;
}
}
(Note that multi-line conditions with mixed and || operators
are hard to read. If you happen to edit such a condition, please
split it into separate conditions.)
(I have committed this code to CVS, but feel free to change it).
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt, [EMAIL PROTECTED]
signature.asc
Description: Digital signature
Re: style matching in current CVS
On Thu, 6 Apr 2006, Dominik Vogt wrote:
On Thu, Apr 06, 2006 at 11:13:03AM +0200, Dominik Vogt wrote:
First, this code from style.c generates a warning:
if (SID_GET_HAS_NAME(s_id) (fw-style_name == NULL
(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)) ||
matchWildcards(SID_GET_NAME(s_id), fw-style_name ) == TRUE)
{
return True;
}
style.c: In function `fw_match_style_id':
style.c:176: warning: suggest parentheses around within ||
Second, the code may access a NULL pointer style_name if neither
of the first three matchWildcards is true.
I just noticed this warning and logic error myself and were about to
change it.
Also, I think even if style_name is not NULL, the usual style name
matching should be done:
You are probably right. It was just that you said it should use it instead
of the normal matching. However, that probably only creates a lot of more
problems than anything else, so I think that the way you changed it to is
good. I have updated the manpage to reflect the change.
Just some conciderations: The manpage states that styles are search for in
a specific order. I really fail to see how this information is useful
while examining the logic, other than for the face that matchig early will
save some comparitions. Am I missing something?
if (SID_GET_HAS_NAME(s_id))
{
if (matchWildcards(SID_GET_NAME(s_id), fw-class.res_class) ==
TRUE)
{
return True;
}
if (matchWildcards(SID_GET_NAME(s_id), fw-class.res_name) ==
TRUE)
{
return True;
}
if (matchWildcards(SID_GET_NAME(s_id), fw-name.name) == TRUE)
{
return True;
}
if (fw-style_name == NULL
matchWildcards(SID_GET_NAME(s_id), fw-class.res_class) ==
TRUE)
{
return True;
}
}
(Note that multi-line conditions with mixed and || operators
are hard to read. If you happen to edit such a condition, please
split it into separate conditions.)
I see your point and will do that in the future. It should prevent errors
as the one above.
(I have committed this code to CVS, but feel free to change it).
I've committed a copy-paste-error fix (you never compared with
style_name).
/Viktor
Re: style matching in current CVS
On Thu, Apr 06, 2006 at 11:42:17AM +0200, Viktor Griph wrote: Just some conciderations: The manpage states that styles are search for in a specific order. I really fail to see how this information is useful while examining the logic, other than for the face that matchig early will save some comparitions. Am I missing something? Do you mean this text: They are searched in the reverse order stated. When two conflicting styles apply to the same window, the style that was changed last wins. This just means that style lines issued later override earlier styles. The text is confusing, though. I've committed a copy-paste-error fix (you never compared with style_name). :-P By the way, the function fw_match_style_id wasn't mentioned in your original ChangeLog entry. Please try to always mention *all* changed functions. This simplifies code maintenance enormously. Ciao Dominik ^_^ ^_^ -- Dominik Vogt, [EMAIL PROTECTED] signature.asc Description: Digital signature
Re: style matching in current CVS
On Thu, 6 Apr 2006, Dominik Vogt wrote: On Thu, Apr 06, 2006 at 11:42:17AM +0200, Viktor Griph wrote: Just some conciderations: The manpage states that styles are search for in a specific order. I really fail to see how this information is useful while examining the logic, other than for the face that matchig early will save some comparitions. Am I missing something? Do you mean this text: They are searched in the reverse order stated. When two conflicting styles apply to the same window, the style that was changed last wins. I've always thought the first sentece They are searched in the reverse order stated. referre back to the order given in the previous statement, i.e I've read stated as stated here. Now that you say it it is fullt pssible, and probably makes more sence to interpet it as some sort of double information the the second sentence you quoted. This just means that style lines issued later override earlier styles. The text is confusing, though. I've committed a copy-paste-error fix (you never compared with style_name). :-P By the way, the function fw_match_style_id wasn't mentioned in your original ChangeLog entry. Please try to always mention *all* changed functions. This simplifies code maintenance enormously. I thougt I did that. I must have overwritten the changelog entry for fw_match_style_id with some subsequent addition to an alrady open changelog witout reverting to a recently saved one. Ciao Dominik ^_^ ^_^ -- Dominik Vogt, [EMAIL PROTECTED]
Re: style matching in current CVS
On Thu, Apr 06, 2006 at 12:13:17PM +0200, Viktor Griph wrote: On Thu, 6 Apr 2006, Dominik Vogt wrote: On Thu, Apr 06, 2006 at 11:42:17AM +0200, Viktor Griph wrote: Just some conciderations: The manpage states that styles are search for in a specific order. I really fail to see how this information is useful while examining the logic, other than for the face that matchig early will save some comparitions. Am I missing something? Do you mean this text: They are searched in the reverse order stated. When two conflicting styles apply to the same window, the style that was changed last wins. I've always thought the first sentece They are searched in the reverse order stated. referre back to the order given in the previous statement, i.e I've read stated as stated here. Now that you say it it is fullt pssible, and probably makes more sence to interpet it as some sort of double information the the second sentence you quoted. Hm, I think this text has to be rewritten. Facts are: * Styles issued later override styles issued earlier. * Multiple styles in a single style line are parsed from left to right, i.e. Style * MouseFocus, SloppyFocus is equivalent to Style * MouseFocus Style * SloppyFocus (SloopyFocus wins). Ciao Dominik ^_^ ^_^ -- Dominik Vogt, [EMAIL PROTECTED] signature.asc Description: Digital signature
Re: style matching in current CVS
On 06 Apr 2006 12:47:59 +0200, Dominik Vogt wrote: On Thu, Apr 06, 2006 at 12:13:17PM +0200, Viktor Griph wrote: On Thu, 6 Apr 2006, Dominik Vogt wrote: Do you mean this text: They are searched in the reverse order stated. When two conflicting styles apply to the same window, the style that was changed last wins. I think it will help if we use different terms instead of one term style meaning different things. I always use term style option to mean individual bit(s), style to mean named structure, and Style to mean fvwm command. So, how about this: When two opposite style options are specified for the same window (like Sticky and Slippery), then the last specified option wins. Example: Style gnome-terminal SloppyFocus, Slippery Style gnome-* Slippery, SloppyFocus, MouseFocus, HandleWidth 6 Style *-terminal Sticky This results to gnome-terminal to be Sticky and use MouseFocus. In this example, the Style gnome-terminal SloppyFocus, Slippery line is effectivelly optimized out and does not consume memory. Regards, Mikhael.
Re: style matching in current CVS
On 06Apr2006 12:58, mikhael goikhman [EMAIL PROTECTED] wrote: | So, how about this: | | When two opposite style options are specified for the same window | (like Sticky and Slippery), then the last specified option wins. Please say opposing instead of opposite. Opposite suggests exact opposites (Thing vs NoThing). Opposing just implies conflict i.e. style options that have multiple effects, with overlapping effects in reverse sense (yes, this sentence itself reads badly). Clarification: suppose style option foo sets behaviours A and B, bah sets not-A and not-B, and baz sets A and not-B. foo and bah are opposite. foo and baz are opposing or conflicting. Cheers, -- Cameron Simpson [EMAIL PROTECTED] DoD#743 http://www.cskk.ezoshosting.com/cs/ Churchill's Commentary on Man: Man will occasionally stumble over the truth, but most of the time he will pick himself up and continue on.
Re: style matching in current CVS
On 07 Apr 2006 11:42:43 +1000, Cameron Simpson wrote: On 06Apr2006 12:58, mikhael goikhman [EMAIL PROTECTED] wrote: | | When two opposite style options are specified for the same window | (like Sticky and Slippery), then the last specified option wins. Please say opposing instead of opposite. Yes, of course. Regards, Mikhael.
