Bug 14672 - mdoc does not store namespace summary and remark elements
Summary: mdoc does not store namespace summary and remark elements
Status: RESOLVED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: Samples ()
Version: 6.9.5.x
Hardware: PC Mac OS
: --- normal
Target Milestone: Untriaged
Assignee: Bo Pang
URL:
Depends on:
Blocks:
 
Reported: 2013-09-12 00:54 UTC by Larry O'Brien
Modified: 2013-09-25 08:33 UTC (History)
3 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
Proper namespace remarks (223.25 KB, image/png)
2013-09-12 00:54 UTC, Larry O'Brien
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Larry O'Brien 2013-09-12 00:54:43 UTC
Created attachment 4849 [details]
Proper namespace remarks

Currently, mdoc does not store namespace <summary> and <remark> elements taken from the `ns-{namespace}.xml` files. For instance, the current mdoc produces the following at the beginning of xml.summary.MonoTouch.UIKit : 

  <elements>
  <summary />
  <remarks />
  <class name="DraggingEventArgs" fullname="MonoTouch.UIKit.DraggingEventArgs" assembly="monotouch">

The end result of this is that when the documentation browser (macdoc) views a namespace element, you only see the table containing the namespace's types, not the namespace summary and (sometimes extensive) remarks.

The problem seems to be that EcmaDoc.PopulateTreeFromIndex() forgot to fill in the nodes:

    if (!nsSummaries.TryGetValue (nsName, out nsElements))
       nsSummaries[nsName] = nsElements = new XElement ("elements",
         new XElement ("summary"),
         new XElement ("remarks"));
    foreach (var type in ns.Elements ("Type")) {

I suggest that before the foreach the nodes be filled with something like:

	 XElement nsElements;
	 if (!nsSummaries.TryGetValue (nsName, out nsElements))
	 {
	  nsSummaries[nsName] = nsElements = new XElement ("elements",
	                           new XElement ("summary"),
	                           new XElement ("remarks"));

	  try
	  {
	   var nsFileName = String.Format("ns-{0}.xml", nsName);
	   var nsEl = XElement.Load(Path.Combine (asm, nsFileName));
	   var summary = nsEl.Descendants("summary").First();
	   var remarks = nsEl.Descendants("remarks").First();
   
	   nsElements.Element("summary").ReplaceWith(summary);
	   nsElements.Element("remarks").ReplaceWith(remarks);
	  }
	  catch(Exception x)
	  {
	   Console.WriteLine("Error reading namespace XML for " + nsName);
	  }
	 }

I've validated that this works on Mac, with the data in /ios-api-docs, but I don't know if there are portability concerns or other use-cases that need to be considered.
Comment 1 Jonathan Pryor 2013-09-14 23:24:15 UTC
Quick look, there's already a method to turn a ns-*.xml file into a <namespace/>: EcmaProvider.ExtractNamespaceSummary().

We should take a page from ecma-provider.cs:68 and fill in nsSummaries within EcmaProvider.PopulateTree(), before the `foreach (var asm in directories)` block (untested):

    foreach (var asm in directories) {
        foreach (var f in Directory.EnumerateFiles (asm, "ns-*.xml")) {
            var ns = ExtractNamespaceSummary (f);
            nsSummaries [ns.Attribute("ns").Value] = ns.Element("summary");
        }
    }

Two problems here: firstly, no merging is done, and some form of summary+remarks merging may be desirable. Secondly, <remarks/> is skipped in this.

Finally, EcmaDoc.PopulateTreeFromIndex() is busted: if the nsSummaries parameter is not null, it may assigned to it, but the nsSummaries parameter isn't `ref`, so if it is assigned to it won't be preserved anywhere. It also doesn't appear to add anything _useful_ to the dictionary, which suggests that my idea of pre-filling nsSummaries makes more sense.
Comment 2 Miguel de Icaza [MSFT] 2013-09-15 10:12:32 UTC
The most puzzling thing is that the documentation *is* rendered by the Web version, but not on the desktop version.   

I am OK making the patch for Mono, we can always roll out the docs compiled with a modified Mono.

Larry, interested in doing this hack?
Comment 3 Larry O'Brien 2013-09-15 13:29:48 UTC
Yay! My first mono pull request! \o/
Comment 4 Larry O'Brien 2013-09-15 14:05:51 UTC
(In reply to comment #1)
> We should take a page from ecma-provider.cs:68 and fill in nsSummaries within
> EcmaProvider.PopulateTree(), before the `foreach (var asm in directories)`

I believe this function produces the XML from which the *assembly* summary is built (the all-namespaces view) and it would be more appropriate to fill the ns-summaries within PopulateTreeFromIndexFile(), which is responsible for the namespace summary. 

As far as the ref broken-ness goes, I think there's definitely a defect, because EcmaUncompiledHelpSource passes null and PopulateTreeFromIndex() has the explicit comment "// nsSummaries is allowed to be null if the user doesn't care about it". But that means that you get different sets of files if you're compiled or uncompiled (you don't get xml.summary.{namespace} if uncompiled). That seems like it must lead to trouble.
Comment 5 Larry O'Brien 2013-09-16 16:31:14 UTC
addressed in mono commit:

a60ff0fd1e181cfde5b862cc491c682c11b9660a
and
9454174ee5aa7ce9c0443422e7321eb9af172f4b
Comment 6 Rolf Bjarne Kvinge [MSFT] 2013-09-25 08:33:52 UTC
Closing according to comment #5.