Please review patch to add '\headerfile' command conversion to HTML

[Dmitri Gribenko]

Hi Fariborz,

On Thu, Jan 31, 2013 at 9:01 PM, jahanian <fjahanian at apple.com> wrote:
> [Comment parsing]. Currently, comment parser does not recognize \headerfile, so merges its text to the following text.
> This patch recognizes \headerfile command and, according to our requirement, places its text into
> a separate <Para>…</Para> tag. Note that this is not a complete implementation of \headerfile
> command. Our immediate need is to provide separate paragraph for its text and not mix it with the \brief text which follows it.
> Please review.

Index: include/clang/AST/CommentCommandTraits.h
===================================================================
--- include/clang/AST/CommentCommandTraits.h	(revision 174067)
+++ include/clang/AST/CommentCommandTraits.h	(working copy)
@@ -96,6 +96,9 @@
   ///   \fn void f(int a);
   /// \endcode
   unsigned IsDeclarationCommand : 1;
+
+  /// \brief True if this is a \headerfile documentation
+  unsigned IsHeaderfileCommand : 1;

(1) "True if this as a \\headerfile-like command."

(2) Please move this after IsDeprecatedCommand, so that it is near
other marker bits for different kinds of block commands.  (This change
affects TableGen backend, too.)

Index: include/clang/AST/CommentCommands.td
===================================================================
--- include/clang/AST/CommentCommands.td	(revision 174067)
+++ include/clang/AST/CommentCommands.td	(working copy)
@@ -19,6 +19,7 @@
   bit IsVerbatimBlockEndCommand = 0;
   bit IsVerbatimLineCommand = 0;
   bit IsDeclarationCommand = 0;
+  bit IsHeaderfileCommand = 0;
 }

Please place after IsDeprecatedCommand.

@@ -72,6 +73,7 @@

 // Doxygen
 def Tparam : BlockCommand<"tparam"> { let IsTParamCommand = 1; }
+def Headerfile : BlockCommand<"headerfile"> { let IsHeaderfileCommand = 1; }

Please move after def Deprecated : BlockCommand<"deprecated">.

+    HeaderfileCommand(NULL){

Space before "{".

@@ -485,6 +486,12 @@
       return;
     }
     PrevCommand = ReturnsCommand;
+  } else if (Info->IsHeaderfileCommand) {
+    if (!HeaderfileCommand) {
+      HeaderfileCommand = Command;
+      return;
+    }
+    PrevCommand = HeaderfileCommand;
   } else {
     // We don't want to check this command for duplicates.
     return;

Please add a test for the "duplicate command" warning for \headerfile.
 It should go into test/Sema/warn-documentation.cpp, after
test_duplicate_returns4.  Nothing fancy, just a single test (because
there are no aliases for \headerfile, unlike \return -- \returnS
pair):

// expected-warning at +2 ...
/// \headerfile ""
/// \headerfile foo.h
int test_duplicate_headerfile1(int);

We do want a warning here, right?

+// CHECK-NEXT:           (CXComment_Text Text=[ Device.h ])
+// CHECK-NEXT:           (CXComment_Text Text=[<Foundation])
+// CHECK-NEXT:           (CXComment_Text Text=[/Device.h>])))

Nice that our HTML parser is forgiving enough not to recognize this as
plain text...  Please also add tests for:

\headerfile <stdio.h>
\headerfile <algorithm>

     Result << "<Abstract>";
+    if (Parts.Headerfile)
+      visit(Parts.Headerfile);

I don't think it is good to stuff this into the brief description.  It
actually contradicts with your goal:

> Our immediate need is to provide separate paragraph for its text and not mix it with the \brief text which follows it.

Brief description should be short, and should start with something
immediately helpful.  I think we should introduce a new tag, like
<Headerfile>.  It looks like a good idea, since we already have
<Declaration>.  Please put it before <Declaration> in the schema.

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/