ENH: Fix generated documentation internal links.

  - Previously all links started in 'command_' which led to conflicts
    and was confusing for non-command items.
  - Use a per-section name that is meaningful to humans.
  - Fix link id names to be valid HTML.

4 files changed, 126 insertions, 9 deletions

diff --git a/Source/cmDocumentationFormatter.cxx b/Source/cmDocumentationFormatter.cxx
index 6b6f8d0..9c63323 100644
--- a/Source/cmDocumentationFormatter.cxx
+++ b/Source/cmDocumentationFormatter.cxx
@@ -71,3 +71,86 @@ void cmDocumentationFormatter::PrintFormatted(std::ostream& os,
     }
 }
 
+//----------------------------------------------------------------------------
+std::string
+cmDocumentationFormatter::ComputeSectionLinkPrefix(std::string const& name)
+{
+  // Map from section name to a prefix for links pointing within the
+  // section.  For example, the commands section should have HTML
+  // links to each command with names like #command:ADD_EXECUTABLE.
+  if(name.find("Command") != name.npos)
+    {
+    return "command";
+    }
+  else if(name.find("Propert") != name.npos)
+    {
+    if(name.find("Global") != name.npos)
+      {
+      return "prop_global";
+      }
+    else if(name.find("Direct") != name.npos)
+      {
+      return "prop_dir";
+      }
+    else if(name.find("Target") != name.npos)
+      {
+      return "prop_tgt";
+      }
+    else if(name.find("Test") != name.npos)
+      {
+      return "prop_test";
+      }
+    else if(name.find("Source") != name.npos)
+      {
+      return "prop_sf";
+      }
+    return "property";
+    }
+  else if(name.find("Variable") != name.npos)
+    {
+    return "variable";
+    }
+  else if(name.find("Polic") != name.npos)
+    {
+    return "policy";
+    }
+  else if(name.find("Module") != name.npos)
+    {
+    return "module";
+    }
+  else if(name.find("Name") != name.npos)
+    {
+    return "name";
+    }
+  else if(name.find("Usage") != name.npos)
+    {
+    return "usage";
+    }
+  else if(name.find("Description") != name.npos)
+    {
+    return "desc";
+    }
+  else if(name.find("Generators") != name.npos)
+    {
+    return "gen";
+    }
+  else if(name.find("Options") != name.npos)
+    {
+    return "opt";
+    }
+  else if(name.find("Copyright") != name.npos)
+    {
+    return "copy";
+    }
+  else if(name.find("See Also") != name.npos)
+    {
+    return "see";
+    }
+  else
+    {
+    std::cerr
+      << "WARNING: ComputeSectionLinkPrefix failed for \"" << name << "\""
+      << std::endl;
+    return "other";
+    }
+}
diff --git a/Source/cmDocumentationFormatter.h b/Source/cmDocumentationFormatter.h
index 33f95f2..29eb2d0 100644
--- a/Source/cmDocumentationFormatter.h
+++ b/Source/cmDocumentationFormatter.h
@@ -62,6 +62,9 @@ public:
   virtual void PrintIndex(std::ostream& ,
                           std::vector<const cmDocumentationSection *>&)
     {}
+
+  /** Compute a prefix for links into a section (#<prefix>_SOMETHING).  */
+  std::string ComputeSectionLinkPrefix(std::string const& name);
 };
 
 #endif
diff --git a/Source/cmDocumentationFormatterDocbook.cxx b/Source/cmDocumentationFormatterDocbook.cxx
index 65f4bf8..1c648a6 100644
--- a/Source/cmDocumentationFormatterDocbook.cxx
+++ b/Source/cmDocumentationFormatterDocbook.cxx
@@ -129,6 +129,8 @@ void cmDocumentationFormatterDocbook
       }
     }
 
+  std::string prefix = this->ComputeSectionLinkPrefix(name);
+
   const std::vector<cmDocumentationEntry> &entries = 
     section.GetEntries();
 
@@ -138,7 +140,7 @@ void cmDocumentationFormatterDocbook
     {
     if(op->Name.size())
       {
-      os << "    <listitem><link linkend=\"command_";
+      os << "    <listitem><link linkend=\"" << prefix << "_";
       cmDocumentationPrintDocbookEscapes(os, op->Name.c_str());
       os << "\"><emphasis><literal>";
       cmDocumentationPrintDocbookEscapes(os, op->Name.c_str());
@@ -156,15 +158,15 @@ void cmDocumentationFormatterDocbook
         {
         if(op->Name.size())
           {
-          os << "    <para id=\"command_";
+          os << "    <para id=\"" << prefix << "_";
           cmDocumentationPrintDocbookEscapes(os, op->Name.c_str());
 
-          // make sure that each id exists only once, e.g. 
-          // command_COMPILE_DEFINITIONS exists at least twice. Since it seems
+          // make sure that each id exists only once.  Since it seems
           // not easily possible to determine which link refers to which id, 
           // we have at least to make sure that the duplicated id's get a 
           // different name (by appending an increasing number), Alex
-          std::string id = "command_";
+          std::string id = prefix;
+          id += "_";
           id += op->Name;
           if (this->EmittedLinkIds.find(id) == this->EmittedLinkIds.end())
             {
diff --git a/Source/cmDocumentationFormatterHTML.cxx b/Source/cmDocumentationFormatterHTML.cxx
index cb8fac1..a40ce99 100644
--- a/Source/cmDocumentationFormatterHTML.cxx
+++ b/Source/cmDocumentationFormatterHTML.cxx
@@ -52,6 +52,31 @@ static void cmDocumentationPrintHTMLChar(std::ostream& os, char c)
 }
 
 //----------------------------------------------------------------------------
+bool cmDocumentationHTMLIsIdChar(char c)
+{
+  // From the HTML specification:
+  //   ID and NAME tokens must begin with a letter ([A-Za-z]) and may
+  //   be followed by any number of letters, digits ([0-9]), hyphens
+  //   ("-"), underscores ("_"), colons (":"), and periods (".").
+  return ((c >= 'A' && c <= 'Z') ||
+          (c >= 'a' && c <= 'z') ||
+          (c >= '0' && c <= '9') ||
+          c == '-' || c == '_' || c == ':' || c == '.');
+}
+
+//----------------------------------------------------------------------------
+void cmDocumentationPrintHTMLId(std::ostream& os, const char* begin)
+{
+  for(const char* c = begin; *c; ++c)
+    {
+    if(cmDocumentationHTMLIsIdChar(*c))
+      {
+      os << *c;
+      }
+    }
+}
+
+//----------------------------------------------------------------------------
 const char* cmDocumentationPrintHTMLLink(std::ostream& os, const char* begin)
 {
   // Look for the end of the link.
@@ -97,6 +122,8 @@ void cmDocumentationFormatterHTML
     os << "<h2><a name=\"section_" << name << "\"/>" << name << "</h2>\n";
     }
 
+  std::string prefix = this->ComputeSectionLinkPrefix(name);
+
   const std::vector<cmDocumentationEntry> &entries = 
     section.GetEntries();
 
@@ -106,8 +133,9 @@ void cmDocumentationFormatterHTML
     {
     if(op->Name.size())
       {
-      os << "    <li><a href=\"#command_"
-         << op->Name.c_str() << "\"><b><code>";
+      os << "    <li><a href=\"#" << prefix << ":";
+      cmDocumentationPrintHTMLId(os, op->Name.c_str());
+      os << "\"><b><code>";
       this->PrintHTMLEscapes(os, op->Name.c_str());
       os << "</code></b></a></li>";
       }
@@ -125,8 +153,9 @@ void cmDocumentationFormatterHTML
         os << "  <li>\n";
         if(op->Name.size())
           {
-          os << "    <a name=\"command_"<< 
-            op->Name.c_str() << "\"><b><code>";
+          os << "    <a name=\"" << prefix << ":";
+          cmDocumentationPrintHTMLId(os, op->Name.c_str());
+          os << "\"><b><code>";
           this->PrintHTMLEscapes(os, op->Name.c_str());
           os << "</code></b></a>: ";
           }