RE: [Mono-dev] PageThemeFileParser.AddDirective
Hi Chris,
Please review the new patch.
BTW, what do you think on refactoring ThrowParseException into throw
ParseException? I mean to create the exception in the helper function,
but to throw it explicitely?
Thank you,
Andrew.
Index: PageThemeFileParser.cs
===
--- PageThemeFileParser.cs (revision 59261)
+++ PageThemeFileParser.cs (working copy)
@@ -51,8 +51,11 @@
}
internal override void AddDirective (string directive,
Hashtable atts)
- {
- ThrowParseException (Unknown directive: +
directive);
+ {
+ if (string.CompareOrdinal (directive,
Register) == 0)
+ base.AddDirective (directive, atts);
+ else
+ ThrowParseException (Unknown directive:
+ directive);
}
internal override Type DefaultBaseType {
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
RE: [Mono-dev] PageThemeFileParser.AddDirective
I think CompareOrdinal is more appropriate here, so if you don't mind I will commit my patch. I actually already committed a patch that works more or less like the one you've written here. Chris ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
RE: [Mono-dev] PageThemeFileParser.AddDirective
I disagree that it is more appropriate - it's case sensitive. Chris On Tue, 2006-04-11 at 06:21 -0700, Andrew Skiba wrote: I think CompareOrdinal is more appropriate here, so if you don't mind I will commit my patch. I actually already committed a patch that works more or less like the one you've written here. Chris ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
RE: [Mono-dev] PageThemeFileParser.AddDirective
I disagree that it is more appropriate - it's case sensitive. I checked, and found that you are right and directives are case insensitive. FYI TemplateControlParser uses case-sensitive comparison everywhere. But anyway OrdinalComparison is more appropriate in this patch (with ignore case), you can look here for an example of unexpected results: ms-help://MS.VSCC.v80/MS.MSDN.v80/MS.NETDEVFX.v20.en/cpref2/html/M_Syste m_String_Compare_2_16e9e883.htm So please review the fixed patch. Regards, Andrew. PageThemeFileParser.patch Description: PageThemeFileParser.patch ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
RE: [Mono-dev] PageThemeFileParser.AddDirective
Look, I have reviewed the patch. 1) There's no need for the patch, as there's already code in the repo that does what's needed. 2) You're suggesting changing to a different string method than what is used in all of the other cases where we compare directive strings. Check out all the other parser classes and their overridden AddDirective methods. If you're not going to change all of them, don't change any of them. 3) I can't read ms-help urls. Chris On Tue, 2006-04-11 at 08:31 -0700, Andrew Skiba wrote: I disagree that it is more appropriate - it's case sensitive. I checked, and found that you are right and directives are case insensitive. FYI TemplateControlParser uses case-sensitive comparison everywhere. But anyway OrdinalComparison is more appropriate in this patch (with ignore case), you can look here for an example of unexpected results: ms-help://MS.VSCC.v80/MS.MSDN.v80/MS.NETDEVFX.v20.en/cpref2/html/M_Syste m_String_Compare_2_16e9e883.htm So please review the fixed patch. Regards, Andrew. ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
RE: [Mono-dev] PageThemeFileParser.AddDirective
On Tue, 2006-04-11 at 08:31 -0700, Andrew Skiba wrote: FYI TemplateControlParser uses case-sensitive comparison everywhere. where? these should be fixed. Chris ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
RE: [Mono-dev] PageThemeFileParser.AddDirective
Sorry, my bad. Did not notice that TagAttributes.GetDictionary creates Hashtable with InvariantCultureIgnoreCase parameter. The url without ms-help is http://msdn2.microsoft.com/en-us/library/zkcaxw5y.aspx -Original Message- From: Chris Toshok [mailto:[EMAIL PROTECTED] Sent: Tuesday, April 11, 2006 19:36 To: Andrew Skiba Cc: Mono-devel-list@lists.ximian.com Subject: RE: [Mono-dev] PageThemeFileParser.AddDirective On Tue, 2006-04-11 at 08:31 -0700, Andrew Skiba wrote: FYI TemplateControlParser uses case-sensitive comparison everywhere. where? these should be fixed. Chris ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
[Mono-dev] PageThemeFileParser.AddDirective
Hello, Chris. I attach skin file, that works perfectly in dotnet, but fails in mono. If I remove PageThemeFileParser.AddDirective, then TemplateControlParser.AddDirective perfectly performs the job. PageThemeFileParser.AddDirective overrides TemplateControlParser.AddDirective to make adding directives impossible. Why did you decide to block normal AddDirective functionality? Thank you in advance. Andrew. SkinFile.skin Description: SkinFile.skin ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] PageThemeFileParser.AddDirective
Because most directives are illegal in skin files (Page, Assembly, etc), I assumed that all were. Obviously this isn't the case :) After more investigation, it looks like Register is the only directive allowed. Does this agree with your assessment? By the way, I'm subscribed to this list, so there's no reason to CC me. Chris On Mon, 2006-04-10 at 00:59 -0700, Andrew Skiba wrote: Hello, Chris. I attach skin file, that works perfectly in dotnet, but fails in mono. If I remove PageThemeFileParser.AddDirective, then TemplateControlParser.AddDirective perfectly performs the job. PageThemeFileParser.AddDirective overrides TemplateControlParser.AddDirective to make adding directives impossible. Why did you decide to block normal AddDirective functionality? Thank you in advance. Andrew. ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
