[Mono-devel-list] daily(?) /doc patches

[Atsushi Eno]

Hello,

Marek Safar wrote:
> Hello,
> 
> It looks that nothing from my previous recommendations has been accepted.
> In this stage is the patch too "buggy".

Mmm, sorry but it seems like I haven't received your mail. I haven't
seen the message.

Please post to the list as well.

I once wrote to you why I keep the patch as is, after receiving your
list of warnings numbers:

 > Well, as for existing patch, Miguel has already reviewed and approved
 > with some comments, immediate before I met breakage. So I'll first
 > commit (if it is said OK) existing stuff without improving those
 > errors. They will be improved on the second stage.

It is OK for me but not sure if I had better modify the patch heavily.
The last one seems not trivial though.

Atsushi Eno

>>Since we have missed almost all the chances to commit /doc patches
>>because it had been soon obsolete against the latest CVS HEAD, I
>>decided to put the patches here. If the patch is not applicable for
>>the latest HEAD, tell me and I'll make diff and upload it onto this
>>directory:
>>http://primates.ximian.com/~atsushi/mcs-doc-patches/
>>
>>I won't modify (and hadn't made) the change content of the patches.
>>
>>Atsushi Eno
>>
>>_______________________________________________
>>Mono-devel-list mailing list
>>Mono-devel-list at lists.ximian.com
>>http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>
>>
>> 
>>
> 
> 
> 
> ------------------------------------------------------------------------
> 
> Subject:
> Re: [Mono-devel-list] the latest /doc patch
> From:
> Marek Safar <marek.safar at seznam.cz>
> Date:
> Tue, 10 Aug 2004 17:35:59 +0100
> To:
> Atsushi Eno <atsushi at ximian.com>
> 
> To:
> Atsushi Eno <atsushi at ximian.com>
> CC:
> Martin Baulig <martin at ximian.com>, Miguel de Icaza <miguel at ximian.com>
> 
> 
> Hello Eno,
> 
>> Hi,
>>
>> Since mcs got more stable than last week, I made the latest patch
>> for /doc support. I modified a bit (as Miguel asked me for the
>> previous patch; added some commends, removed --options) but basic
>> part of patch is unchanged.
>>
>> I have had no problem  for a month or so since the last trouble
>> that prevented me from commiting the last ready-to-go patch.
>>
>> I also added some tests wrt doc comments. It however just tests
>> if it does not break compilation (i.e. not checking result docs).
>>  
>>
> What is missing on the tests are test
> My suggestion is to create for every xml-???.cs also
> xml-???.xml.expected and add to every Main method test whether generated
> xml is same as expected.
> To be sure that this patch is still working with same results.
> 
> Tip: Use "// Compiler options: " syntax for /doc compiler option
> 
>> I've received a list of to-be-supported warning messages from
>> Marek (thanks to him) and those works will be done later.
>>
>>
>>  
>>
> 1,
> 
> Error message for 2006 should be better explanatory e.g.  "Command-line
> syntax error:  Missing ':<text>' for '/doc' switch"
> 
> 2,
> 
> You cannot write simple Report.Warning. You always have to test
> WarningLevel for your Warning. Use IF or Warning method with Message
> parameter.
> 
> To Miguel: It is that I was talking about !
> 
> +    } catch (XmlException ex) {
> +        Report.Warning (1570, Lexer.Location, "XML comment is not
> well-formed: " + ex.Message);
> +        XmlComment com = doc.CreateComment ("FIXME: Invalid
> documentation markup was found for member " + fullyQualifiedName);
> +        doc.DocumentElement.LastChild.AppendChild (com);
> +    }
> 
> +void WarnIncorrectXmlComment (string token_type)
> +{
> +    Report.Warning (1587, Lexer.Location, "XML comment is not
> acceptable on  " + token_type);
> +    Lexer.ConsumeXmlComment ();
> }
> 
> 3,
> 
> For this cases can be better use StringBuilder or String.Format. It is
> extremely memory consuming especially on windows.
> 
> +    Documentation.SetAttribute ("name", "M:" + container.Name + "." + 
> Name + "(" + para1 + (para2 != null ? "," + para2 : null) +
> 
> or this
> 
> +                string paramSpec = null;
> +                Parameter [] plist = Parameters.FixedParameters;
> +                if (plist != null) {
> +                    foreach (Parameter p in plist)
> +                        paramSpec += (paramSpec != null ? "," : "(") +
> container.ResolveType (p.TypeName, false, Location).FullName;
> +                }
> 
> BTW: This code is really buggy. See point 6 for further hints and please
> be aware of FullName usage because it uses "+" instead of "." separator.
> You can use methods GetSignatureForError either we will have same
> troubles (like special handling for operators, indexers, ref & out
> params, __arglist params, etc...) twice because your code seems to do
> exactly same.
> 
> Try this code
> 
>    public class Test
>    {
>         public class A {}
> 
>        public static void Main ()
>        {
>        }
> 
>        /// here is a documentation
>        public static void Foo (A a)
>        {
>        }
>    }
> 
> 4,
> Why don't create for "M:", "F:" etc, some nice constants ?
> 
> 5,
> This code generate buggy xml.
> BTW: your xml-017.cs test
> 
>    public class Test
>    {
>        public static void Main ()
>        {
>        }
> 
>        /// here is a documentation with parameters (and has same name)
>        public static void Foo (params string [] param)
>        {
>        }
>    }
> 
> 6,
> The most important problem. Why do you call RootContext.FixupDocument ()
> before types are Defined. This is very dangerous and you will have to
> duplicate code what is in Define methods.
> And this simple code now crash compiler.
> 
>    public class Test
>    {
>        /// here is a documentation with parameters (and has same name)
>        public static void Foo (BadType t)
>        {
>        }
>    }
> 
> Why do not reuse bool Define () methods or DefineContainerMembers method
> where you can simply share a lot of code and don't traverse thought
> whole code tree.
> 
> 
> I hope, I didn't flood you ;-)
> 
> Marek
> 
> 
>