[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: Please review patch to add '\headerfile' command conversion to HTML
From:       Dmitri Gribenko <gribozavr () gmail ! com>
Date:       2013-01-31 19:31:00
Message-ID: CA+Y5xYcBjOknJO6_S_tdE-5Uxr3Sv05sPJWdz4nTpsgbx4SJ9A () mail ! gmail ! com
[Download RAW message or body]

Hi Fariborz,

On Thu, Jan 31, 2013 at 9:01 PM, jahanian <fjahanian@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@+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@gmail.com>*/

_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread]