[Mono-devel-list] Re: Improved and fixed mcs /doc patch

[Marek Safar]

Hello Eno,

>[Sorry, am resending; devel-list rejected the previous post due to
>the message size.]
>
>  
>
>>>>I love these tests. Just add comparison to makefile and it will be great.
>>>>        
>>>>
>>>Actually we can't. They are literally different. But we can still add
>>>comparison code by using canonicalization or something else.
>>>      
>>>
>>I don't understand where is the problem, with line ending or somewhere
>>    
>>
>else.
>
>For example, <foo></foo> and <foo/>.
>  
>
Hmm, unfortunately I still don't understand. I thought that we write xml
in same format every time. If not, then we will have to compare it in DOM.
For me it is the biggest issue.

>  
>
>>>>It would be really good to have this CreateMemberComment inside of
>>>>        
>>>>
>class derived from MemberCore instead of in the lexer.
>  
>
>>>>e.g
>>>>
>>>>method.SetComment (xml_data);
>>>>
>>>>
>>>>        
>>>>
>>No, original idea was little different. Look at this.
>>
>>+ if (Lexer.xml_comment_buffer.Length > 0)
>>+ e.Documentation = CreateMemberComment ("E:" + MakeName (e.MemberName));
>>
>>It seems to me that you are doing things that should be done inside a
>>class outside. When I have operation that works just with class members
>>why it is outside of class.
>>    
>>
>
>If you mean having setter of XmlElement outside MemberCore, it is
>not so straight to dare avoid. cs-parser.jay uses XmlElement
>tmpComment that is set to MemberCore in the later phase than creation.
>
>If there is strong reason that I should review if all FixupDocument
>works fine by modifying the signature to not have setter, including
>possible future need of change wrt tokenization improvements,
>I'llfix them (currently it sounds like kind of wasting time).
>  
>
No it is not critical, just tip how to simplify your code.

>  
>
>>>No. It is true only when /doc is passed to the command line
>>>(otherwise that error should not be put).
>>>      
>>>
>
>  
>
>>OK, but I thought that public virtual void FixupDocument (DeclSpace ds)
>>is called only when RootContext.NeedDocument is true.;
>>    
>>
>
>Ah, true ;-)
>
>  
>
>>>mhm, didn't notice. Maybe having separate tests for warnings and
>>>having option warnaserror would be nicer.
>>>      
>>>
>>No, I want to be sure that you didn't break any of error tests.
>>    
>>
>
>Ah, OK. But I've added some of them (included this time).
>
>  
>
>>>Code (with reorganizing tests) will follow, maybe tomorrow.
>>>      
>>>
>
>Sorry but actually am having wierd build problem on 2.0 System.dll
>for a while (something weird still happens) and forgot this one.
>
>I wonder if I still had better not to commit the changes.
>
>  
>
I think we need also Miguel statement.

Marek