All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Add clang-format file for Xen Hypervisor format
@ 2017-04-12 10:49 Ishani Chugh
  2017-04-12 11:26 ` Lars Kurth
  2017-04-12 15:16 ` Jan Beulich
  0 siblings, 2 replies; 7+ messages in thread
From: Ishani Chugh @ 2017-04-12 10:49 UTC (permalink / raw)
  To: xen-devel; +Cc: lars.kurth.xen, Ishani Chugh, cardoe

Signed-off-by: Ishani Chugh <chugh.ishani@research.iiit.ac.in>
---
 tools/clang-format/tests/.clang-format | 88 ++++++++++++++++++++++++++++++++++
 tools/clang-format/tests/correct.c     | 73 ++++++++++++++++++++++++++++
 tools/clang-format/tests/test.c        | 69 ++++++++++++++++++++++++++
 3 files changed, 230 insertions(+)
 create mode 100644 tools/clang-format/tests/.clang-format
 create mode 100644 tools/clang-format/tests/correct.c
 create mode 100644 tools/clang-format/tests/test.c

diff --git a/tools/clang-format/tests/.clang-format b/tools/clang-format/tests/.clang-format
new file mode 100644
index 0000000..2229910
--- /dev/null
+++ b/tools/clang-format/tests/.clang-format
@@ -0,0 +1,88 @@
+---
+Language:        Cpp
+# BasedOnStyle:  LLVM
+AccessModifierOffset: 0
+AlignAfterOpenBracket: Align
+AlignConsecutiveAssignments: false
+AlignConsecutiveDeclarations: false
+AlignEscapedNewlinesLeft: true
+AlignOperands:   true
+AlignTrailingComments: true
+AllowAllParametersOfDeclarationOnNextLine: true
+AllowShortBlocksOnASingleLine: false
+AllowShortCaseLabelsOnASingleLine: false
+AllowShortFunctionsOnASingleLine: All
+AllowShortIfStatementsOnASingleLine: false
+AllowShortLoopsOnASingleLine: false
+AlwaysBreakAfterDefinitionReturnType: None
+AlwaysBreakAfterReturnType: None
+AlwaysBreakBeforeMultilineStrings: false
+AlwaysBreakTemplateDeclarations: true
+BinPackArguments: false
+BinPackParameters: false
+BraceWrapping:   
+  AfterClass:      true
+  AfterControlStatement: true
+  AfterEnum:       true
+  AfterFunction:   true
+  AfterNamespace:  true
+  AfterObjCDeclaration: true
+  AfterStruct:     true
+  AfterUnion:      true
+  BeforeCatch:     true
+  BeforeElse:      true
+  IndentBraces:    false
+BreakBeforeBinaryOperators: None
+BreakBeforeBraces: Custom
+BreakBeforeTernaryOperators: true
+BreakConstructorInitializersBeforeComma: false
+ColumnLimit:     80
+CommentPragmas:  '^ IWYU pragma:'
+ContinuationIndentWidth: 4
+Cpp11BracedListStyle: true
+DerivePointerAlignment: false
+DisableFormat:   false
+ExperimentalAutoDetectBinPacking: false
+ForEachMacros:   [ foreach, Q_FOREACH, BOOST_FOREACH ]
+IncludeCategories: 
+  - Regex:           '^"(llvm|llvm-c|clang|clang-c)/'
+    Priority:        2
+  - Regex:           '^(<|"(gtest|isl|json)/)'
+    Priority:        3
+  - Regex:           '.*'
+    Priority:        1
+IndentCaseLabels: false
+IndentWidth:     4
+IndentWrappedFunctionNames: false
+KeepEmptyLinesAtTheStartOfBlocks: false
+MacroBlockBegin: ''
+MacroBlockEnd:   ''
+MaxEmptyLinesToKeep: 1
+NamespaceIndentation: None
+ObjCBlockIndentWidth: 2
+ObjCSpaceAfterProperty: false
+ObjCSpaceBeforeProtocolList: true
+PenaltyBreakBeforeFirstCallParameter: 19
+PenaltyBreakComment: 300
+PenaltyBreakFirstLessLess: 120
+PenaltyBreakString: 1000
+PenaltyExcessCharacter: 1000000
+PenaltyReturnTypeOnItsOwnLine: 60
+PointerAlignment: Left
+ReflowComments:  true
+SortIncludes:    false
+SpaceAfterCStyleCast: false
+SpaceBeforeAssignmentOperators: true
+SpaceBeforeParens: ControlStatements
+SpaceInEmptyParentheses: false
+SpacesBeforeTrailingComments: 1
+SpacesInAngles:  false
+SpacesInContainerLiterals: true
+SpacesInCStyleCastParentheses: true
+SpacesInParentheses: true
+SpacesInSquareBrackets: false
+Standard:        Cpp11
+TabWidth:        4
+UseTab:          Never
+...
+
diff --git a/tools/clang-format/tests/correct.c b/tools/clang-format/tests/correct.c
new file mode 100644
index 0000000..882d50a
--- /dev/null
+++ b/tools/clang-format/tests/correct.c
@@ -0,0 +1,73 @@
+/*
+This is used to check if includes are sorted or not
+*/
+#include <unistd.h>
+#include <stdio.h>
+#define true 1
+
+struct sample
+{
+    int a;
+};
+
+/*
+This is used to check support for small function in a line
+*/
+int a( int x, int y ) {}
+
+int main()
+{
+    // This will test indentation
+
+    if ( true )
+    {
+        if ( true )
+        {
+            printf( "this should be indented\n" );
+            if ( true )
+            {
+                printf( "this should be indented too\n" );
+            }
+        }
+    }
+
+    /*
+    This will check whitespaces format
+    */
+
+    if ( ( 1 & 2 ) == 42 )
+        printf( "dfadsf\n" );
+
+    /*
+    This will test that whitespaces are not added for . and -> and the left position of *
+    */
+    struct sample ob;
+    struct sample* ob1;
+    ob.a = 10;
+    ob1->a = 10;
+    int x;
+    int y;
+    char* p;
+
+    /*
+    This will break the line. It also checks addition of spaces in binary operators
+    */
+    if ( 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 &&
+         1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 )
+        p = "adsaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+            "aaaaaaaaaaaaaaaaaaaaa";
+    printf( "adddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"
+            "dddddddddddddddddadad" );
+
+    for ( ;; )
+    {
+        int z = 5;
+        break;
+    }
+    /*
+    Check for template break in template
+    */
+    template <class P>
+    void updateMenuList( P*, struct menu* );
+    return 0;
+}
\ No newline at end of file
diff --git a/tools/clang-format/tests/test.c b/tools/clang-format/tests/test.c
new file mode 100644
index 0000000..2e7eaf2
--- /dev/null
+++ b/tools/clang-format/tests/test.c
@@ -0,0 +1,69 @@
+/*
+This is used to check if includes are sorted or not
+*/
+#include<unistd.h>
+#include<stdio.h>
+#define true 1
+
+struct sample{
+	int a;
+};
+
+/*
+This is used to check support for small function in a line
+*/
+int a(int x, int y)
+{}
+
+int main()
+{
+	//This will test indentation
+
+if(true)
+{
+if(true)
+{
+printf("this should be indented\n");
+if(true)
+{
+printf("this should be indented too\n");
+			}
+}
+}
+
+/*
+This will check whitespaces format
+*/
+
+if((1&2)==42)
+printf("dfadsf\n" );
+
+/*
+This will test that whitespaces are not added for . and -> and the left position of *
+*/
+ struct sample ob;
+ struct sample* ob1;
+ob.a = 10;
+ob1->a =10;
+	int x;
+	int y;
+	char *p;
+
+/*
+This will break the line. It also checks addition of spaces in binary operators
+*/
+	if (1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2)
+	p = "adsaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+	printf("addddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddadad");
+
+	for(;;)
+		{
+		int z = 5;
+break;
+}
+/*
+Check for template break in template
+*/
+template <class P>	void updateMenuList(P*, struct menu*);
+	return 0;
+}
\ No newline at end of file
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add clang-format file for Xen Hypervisor format
  2017-04-12 10:49 [PATCH] Add clang-format file for Xen Hypervisor format Ishani Chugh
@ 2017-04-12 11:26 ` Lars Kurth
  2017-04-12 11:51   ` Ishani
  2017-04-12 15:16 ` Jan Beulich
  1 sibling, 1 reply; 7+ messages in thread
From: Lars Kurth @ 2017-04-12 11:26 UTC (permalink / raw)
  To: Ishani Chugh; +Cc: xen-devel, cardoe

Hi Ishani,

> On 12 Apr 2017, at 11:49, Ishani Chugh <chugh.ishani@research.iiit.ac.in> wrote:

Here you would normally add a short description of the patch (also called cover letter). In this case, you probably want to describe:
a) What you have done
b) Link to https://wiki.xenproject.org/wiki/Outreach_Program_Projects#Code_Standards_Checking_using_clang-format, https://docs.google.com/document/d/1tCcwZ9K38ToLGTPHZkfs2sS4s4YulrGoH8LIHwBMbg4/edit and https://docs.google.com/document/d/10NJn-QvO1TvyJJJGE2PD6FtElYCT3neBAffIqeWHdiE/edit
c) You can also link to your github repo
d) You may want to add the two command lines or some instructions showing how to use the patch

> Signed-off-by: Ishani Chugh <chugh.ishani@research.iiit.ac.in>
> ---
> tools/clang-format/tests/.clang-format | 88 ++++++++++++++++++++++++++++++++++
> tools/clang-format/tests/correct.c     | 73 ++++++++++++++++++++++++++++
> tools/clang-format/tests/test.c        | 69 ++++++++++++++++++++++++++
> 3 files changed, 230 insertions(+)
> create mode 100644 tools/clang-format/tests/.clang-format
> create mode 100644 tools/clang-format/tests/correct.c
> create mode 100644 tools/clang-format/tests/test.c
> 
> diff --git a/tools/clang-format/tests/.clang-format b/tools/clang-format/tests/.clang-format
> new file mode 100644
> index 0000000..2229910
> --- /dev/null
> +++ b/tools/clang-format/tests/.clang-format
> @@ -0,0 +1,88 @@
> +---
> +Language:        Cpp
> +# BasedOnStyle:  LLVM

This would be a good place to list in a comment, which coding style patterns the file covers. In other words, everything that has a yes in https://docs.google.com/document/d/1tCcwZ9K38ToLGTPHZkfs2sS4s4YulrGoH8LIHwBMbg4/edit

The other thing, which is a little unclear is whether the file applies to 
- http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=CODING_STYLE only
- http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/CODING_STYLE only
- or both

I think it is OK to just cover one file.

> +AccessModifierOffset: 0
> +AlignAfterOpenBracket: Align
> +AlignConsecutiveAssignments: false
> +AlignConsecutiveDeclarations: false
> +AlignEscapedNewlinesLeft: true
> +AlignOperands:   true
> +AlignTrailingComments: true
> +AllowAllParametersOfDeclarationOnNextLine: true
> +AllowShortBlocksOnASingleLine: false
> +AllowShortCaseLabelsOnASingleLine: false
> +AllowShortFunctionsOnASingleLine: All
> +AllowShortIfStatementsOnASingleLine: false
> +AllowShortLoopsOnASingleLine: false
> +AlwaysBreakAfterDefinitionReturnType: None
> +AlwaysBreakAfterReturnType: None
> +AlwaysBreakBeforeMultilineStrings: false
> +AlwaysBreakTemplateDeclarations: true
> +BinPackArguments: false
> +BinPackParameters: false
> +BraceWrapping:   
> +  AfterClass:      true
> +  AfterControlStatement: true
> +  AfterEnum:       true
> +  AfterFunction:   true
> +  AfterNamespace:  true
> +  AfterObjCDeclaration: true
> +  AfterStruct:     true
> +  AfterUnion:      true
> +  BeforeCatch:     true
> +  BeforeElse:      true
> +  IndentBraces:    false
> +BreakBeforeBinaryOperators: None
> +BreakBeforeBraces: Custom
> +BreakBeforeTernaryOperators: true
> +BreakConstructorInitializersBeforeComma: false
> +ColumnLimit:     80
> +CommentPragmas:  '^ IWYU pragma:'
> +ContinuationIndentWidth: 4
> +Cpp11BracedListStyle: true
> +DerivePointerAlignment: false
> +DisableFormat:   false
> +ExperimentalAutoDetectBinPacking: false
> +ForEachMacros:   [ foreach, Q_FOREACH, BOOST_FOREACH ]
> +IncludeCategories: 
> +  - Regex:           '^"(llvm|llvm-c|clang|clang-c)/'
> +    Priority:        2
> +  - Regex:           '^(<|"(gtest|isl|json)/)'
> +    Priority:        3
> +  - Regex:           '.*'
> +    Priority:        1
> +IndentCaseLabels: false
> +IndentWidth:     4
> +IndentWrappedFunctionNames: false
> +KeepEmptyLinesAtTheStartOfBlocks: false
> +MacroBlockBegin: ''
> +MacroBlockEnd:   ''
> +MaxEmptyLinesToKeep: 1
> +NamespaceIndentation: None
> +ObjCBlockIndentWidth: 2
> +ObjCSpaceAfterProperty: false
> +ObjCSpaceBeforeProtocolList: true
> +PenaltyBreakBeforeFirstCallParameter: 19
> +PenaltyBreakComment: 300
> +PenaltyBreakFirstLessLess: 120
> +PenaltyBreakString: 1000
> +PenaltyExcessCharacter: 1000000
> +PenaltyReturnTypeOnItsOwnLine: 60
> +PointerAlignment: Left
> +ReflowComments:  true
> +SortIncludes:    false
> +SpaceAfterCStyleCast: false
> +SpaceBeforeAssignmentOperators: true
> +SpaceBeforeParens: ControlStatements
> +SpaceInEmptyParentheses: false
> +SpacesBeforeTrailingComments: 1
> +SpacesInAngles:  false
> +SpacesInContainerLiterals: true
> +SpacesInCStyleCastParentheses: true
> +SpacesInParentheses: true
> +SpacesInSquareBrackets: false
> +Standard:        Cpp11
> +TabWidth:        4
> +UseTab:          Never
> +...
> +
> diff --git a/tools/clang-format/tests/correct.c b/tools/clang-format/tests/correct.c
> new file mode 100644
> index 0000000..882d50a
> --- /dev/null
> +++ b/tools/clang-format/tests/correct.c
> @@ -0,0 +1,73 @@

I think it would be extremely useful, to add a reference in a comment to the coding standard document for each tested code-snippet. 
I am going to add an example under main()

> +/*
> +This is used to check if includes are sorted or not
> +*/
> +#include <unistd.h>
> +#include <stdio.h>
> +#define true 1
> +
> +struct sample
> +{
> +    int a;
> +};
> +
> +/*
> +This is used to check support for small function in a line
> +*/
> +int a( int x, int y ) {}
> +
> +int main()
> +{
> +    // This will test indentation

Coming back to my earlier point, this tests http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=CODING_STYLE section Indentation (lines 17-32)
So you could say

      // This tests Indentation
      // See xen.git CODING_STYLE section Indentation (lines 17-32)

> +
> +    if ( true )
> +    {
> +        if ( true )
> +        {
> +            printf( "this should be indented\n" );
> +            if ( true )
> +            {
> +                printf( "this should be indented too\n" );
> +            }
> +        }
> +    }
> +
> +    /*
> +    This will check whitespaces format
> +    */
> +
> +    if ( ( 1 & 2 ) == 42 )
> +        printf( "dfadsf\n" );
> +
> +    /*
> +    This will test that whitespaces are not added for . and -> and the left position of *
> +    */
> +    struct sample ob;
> +    struct sample* ob1;
> +    ob.a = 10;
> +    ob1->a = 10;
> +    int x;
> +    int y;
> +    char* p;
> +
> +    /*
> +    This will break the line. It also checks addition of spaces in binary operators
> +    */
> +    if ( 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 &&
> +         1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 )
> +        p = "adsaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
> +            "aaaaaaaaaaaaaaaaaaaaa";
> +    printf( "adddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"
> +            "dddddddddddddddddadad" );
> +
> +    for ( ;; )
> +    {
> +        int z = 5;
> +        break;
> +    }
> +    /*
> +    Check for template break in template
> +    */
> +    template <class P>
> +    void updateMenuList( P*, struct menu* );
> +    return 0;
> +}
> \ No newline at end of file
> diff --git a/tools/clang-format/tests/test.c b/tools/clang-format/tests/test.c
> new file mode 100644
> index 0000000..2e7eaf2
> --- /dev/null
> +++ b/tools/clang-format/tests/test.c

How is this file different from correct.c? 
It looks to me that test.c is a wrongly formatted version of correct.c and when you pass it into clang-format it is re-formatted to match correct.c

If so, you want to include this information in the cover letter (see above). Or you could write little bash script which shows how this works

> @@ -0,0 +1,69 @@
> +/*
> +This is used to check if includes are sorted or not
> +*/
> +#include<unistd.h>
> +#include<stdio.h>
> +#define true 1
> +
> +struct sample{
> +	int a;
> +};
> +
> +/*
> +This is used to check support for small function in a line
> +*/
> +int a(int x, int y)
> +{}
> +
> +int main()
> +{
> +	//This will test indentation
> +
> +if(true)
> +{
> +if(true)
> +{
> +printf("this should be indented\n");
> +if(true)
> +{
> +printf("this should be indented too\n");
> +			}
> +}
> +}
> +
> +/*
> +This will check whitespaces format
> +*/
> +
> +if((1&2)==42)
> +printf("dfadsf\n" );
> +
> +/*
> +This will test that whitespaces are not added for . and -> and the left position of *
> +*/
> + struct sample ob;
> + struct sample* ob1;
> +ob.a = 10;
> +ob1->a =10;
> +	int x;
> +	int y;
> +	char *p;
> +
> +/*
> +This will break the line. It also checks addition of spaces in binary operators
> +*/
> +	if (1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2)
> +	p = "adsaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
> +	printf("addddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddadad");
> +
> +	for(;;)
> +		{
> +		int z = 5;
> +break;
> +}
> +/*
> +Check for template break in template
> +*/
> +template <class P>	void updateMenuList(P*, struct menu*);
> +	return 0;
> +}
> \ No newline at end of file
> -- 
> 2.7.4
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add clang-format file for Xen Hypervisor format
  2017-04-12 11:26 ` Lars Kurth
@ 2017-04-12 11:51   ` Ishani
  2017-04-12 12:14     ` Lars Kurth
  0 siblings, 1 reply; 7+ messages in thread
From: Ishani @ 2017-04-12 11:51 UTC (permalink / raw)
  To: Lars Kurth; +Cc: xen-devel, cardoe

Hello,

Thanks for the comments.
I have created a markdown file for clang-format doc which I will add. 

>If so, you want to include this information in the cover letter (see above). Or you >could write little bash script which shows how this works

Can you elaborate on the task of bash script? Should it give the difference between files?

Shall I create a new patch by incorporating the suggested changes?

Regards,
Ishani

----- Original Message -----
From: "Lars Kurth" <lars.kurth.xen@gmail.com>
To: "Ishani Chugh" <chugh.ishani@research.iiit.ac.in>
Cc: "xen-devel" <xen-devel@lists.xenproject.org>, "cardoe" <cardoe@cardoe.com>
Sent: Wednesday, April 12, 2017 4:56:53 PM
Subject: Re: [PATCH] Add clang-format file for Xen Hypervisor format

Hi Ishani,

> On 12 Apr 2017, at 11:49, Ishani Chugh <chugh.ishani@research.iiit.ac.in> wrote:

Here you would normally add a short description of the patch (also called cover letter). In this case, you probably want to describe:
a) What you have done
b) Link to https://wiki.xenproject.org/wiki/Outreach_Program_Projects#Code_Standards_Checking_using_clang-format, https://docs.google.com/document/d/1tCcwZ9K38ToLGTPHZkfs2sS4s4YulrGoH8LIHwBMbg4/edit and https://docs.google.com/document/d/10NJn-QvO1TvyJJJGE2PD6FtElYCT3neBAffIqeWHdiE/edit
c) You can also link to your github repo
d) You may want to add the two command lines or some instructions showing how to use the patch

> Signed-off-by: Ishani Chugh <chugh.ishani@research.iiit.ac.in>
> ---
> tools/clang-format/tests/.clang-format | 88 ++++++++++++++++++++++++++++++++++
> tools/clang-format/tests/correct.c     | 73 ++++++++++++++++++++++++++++
> tools/clang-format/tests/test.c        | 69 ++++++++++++++++++++++++++
> 3 files changed, 230 insertions(+)
> create mode 100644 tools/clang-format/tests/.clang-format
> create mode 100644 tools/clang-format/tests/correct.c
> create mode 100644 tools/clang-format/tests/test.c
> 
> diff --git a/tools/clang-format/tests/.clang-format b/tools/clang-format/tests/.clang-format
> new file mode 100644
> index 0000000..2229910
> --- /dev/null
> +++ b/tools/clang-format/tests/.clang-format
> @@ -0,0 +1,88 @@
> +---
> +Language:        Cpp
> +# BasedOnStyle:  LLVM

This would be a good place to list in a comment, which coding style patterns the file covers. In other words, everything that has a yes in https://docs.google.com/document/d/1tCcwZ9K38ToLGTPHZkfs2sS4s4YulrGoH8LIHwBMbg4/edit

The other thing, which is a little unclear is whether the file applies to 
- http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=CODING_STYLE only
- http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/CODING_STYLE only
- or both

I think it is OK to just cover one file.

> +AccessModifierOffset: 0
> +AlignAfterOpenBracket: Align
> +AlignConsecutiveAssignments: false
> +AlignConsecutiveDeclarations: false
> +AlignEscapedNewlinesLeft: true
> +AlignOperands:   true
> +AlignTrailingComments: true
> +AllowAllParametersOfDeclarationOnNextLine: true
> +AllowShortBlocksOnASingleLine: false
> +AllowShortCaseLabelsOnASingleLine: false
> +AllowShortFunctionsOnASingleLine: All
> +AllowShortIfStatementsOnASingleLine: false
> +AllowShortLoopsOnASingleLine: false
> +AlwaysBreakAfterDefinitionReturnType: None
> +AlwaysBreakAfterReturnType: None
> +AlwaysBreakBeforeMultilineStrings: false
> +AlwaysBreakTemplateDeclarations: true
> +BinPackArguments: false
> +BinPackParameters: false
> +BraceWrapping:   
> +  AfterClass:      true
> +  AfterControlStatement: true
> +  AfterEnum:       true
> +  AfterFunction:   true
> +  AfterNamespace:  true
> +  AfterObjCDeclaration: true
> +  AfterStruct:     true
> +  AfterUnion:      true
> +  BeforeCatch:     true
> +  BeforeElse:      true
> +  IndentBraces:    false
> +BreakBeforeBinaryOperators: None
> +BreakBeforeBraces: Custom
> +BreakBeforeTernaryOperators: true
> +BreakConstructorInitializersBeforeComma: false
> +ColumnLimit:     80
> +CommentPragmas:  '^ IWYU pragma:'
> +ContinuationIndentWidth: 4
> +Cpp11BracedListStyle: true
> +DerivePointerAlignment: false
> +DisableFormat:   false
> +ExperimentalAutoDetectBinPacking: false
> +ForEachMacros:   [ foreach, Q_FOREACH, BOOST_FOREACH ]
> +IncludeCategories: 
> +  - Regex:           '^"(llvm|llvm-c|clang|clang-c)/'
> +    Priority:        2
> +  - Regex:           '^(<|"(gtest|isl|json)/)'
> +    Priority:        3
> +  - Regex:           '.*'
> +    Priority:        1
> +IndentCaseLabels: false
> +IndentWidth:     4
> +IndentWrappedFunctionNames: false
> +KeepEmptyLinesAtTheStartOfBlocks: false
> +MacroBlockBegin: ''
> +MacroBlockEnd:   ''
> +MaxEmptyLinesToKeep: 1
> +NamespaceIndentation: None
> +ObjCBlockIndentWidth: 2
> +ObjCSpaceAfterProperty: false
> +ObjCSpaceBeforeProtocolList: true
> +PenaltyBreakBeforeFirstCallParameter: 19
> +PenaltyBreakComment: 300
> +PenaltyBreakFirstLessLess: 120
> +PenaltyBreakString: 1000
> +PenaltyExcessCharacter: 1000000
> +PenaltyReturnTypeOnItsOwnLine: 60
> +PointerAlignment: Left
> +ReflowComments:  true
> +SortIncludes:    false
> +SpaceAfterCStyleCast: false
> +SpaceBeforeAssignmentOperators: true
> +SpaceBeforeParens: ControlStatements
> +SpaceInEmptyParentheses: false
> +SpacesBeforeTrailingComments: 1
> +SpacesInAngles:  false
> +SpacesInContainerLiterals: true
> +SpacesInCStyleCastParentheses: true
> +SpacesInParentheses: true
> +SpacesInSquareBrackets: false
> +Standard:        Cpp11
> +TabWidth:        4
> +UseTab:          Never
> +...
> +
> diff --git a/tools/clang-format/tests/correct.c b/tools/clang-format/tests/correct.c
> new file mode 100644
> index 0000000..882d50a
> --- /dev/null
> +++ b/tools/clang-format/tests/correct.c
> @@ -0,0 +1,73 @@

I think it would be extremely useful, to add a reference in a comment to the coding standard document for each tested code-snippet. 
I am going to add an example under main()

> +/*
> +This is used to check if includes are sorted or not
> +*/
> +#include <unistd.h>
> +#include <stdio.h>
> +#define true 1
> +
> +struct sample
> +{
> +    int a;
> +};
> +
> +/*
> +This is used to check support for small function in a line
> +*/
> +int a( int x, int y ) {}
> +
> +int main()
> +{
> +    // This will test indentation

Coming back to my earlier point, this tests http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=CODING_STYLE section Indentation (lines 17-32)
So you could say

      // This tests Indentation
      // See xen.git CODING_STYLE section Indentation (lines 17-32)

> +
> +    if ( true )
> +    {
> +        if ( true )
> +        {
> +            printf( "this should be indented\n" );
> +            if ( true )
> +            {
> +                printf( "this should be indented too\n" );
> +            }
> +        }
> +    }
> +
> +    /*
> +    This will check whitespaces format
> +    */
> +
> +    if ( ( 1 & 2 ) == 42 )
> +        printf( "dfadsf\n" );
> +
> +    /*
> +    This will test that whitespaces are not added for . and -> and the left position of *
> +    */
> +    struct sample ob;
> +    struct sample* ob1;
> +    ob.a = 10;
> +    ob1->a = 10;
> +    int x;
> +    int y;
> +    char* p;
> +
> +    /*
> +    This will break the line. It also checks addition of spaces in binary operators
> +    */
> +    if ( 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 &&
> +         1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 && 1 > 2 )
> +        p = "adsaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
> +            "aaaaaaaaaaaaaaaaaaaaa";
> +    printf( "adddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"
> +            "dddddddddddddddddadad" );
> +
> +    for ( ;; )
> +    {
> +        int z = 5;
> +        break;
> +    }
> +    /*
> +    Check for template break in template
> +    */
> +    template <class P>
> +    void updateMenuList( P*, struct menu* );
> +    return 0;
> +}
> \ No newline at end of file
> diff --git a/tools/clang-format/tests/test.c b/tools/clang-format/tests/test.c
> new file mode 100644
> index 0000000..2e7eaf2
> --- /dev/null
> +++ b/tools/clang-format/tests/test.c

How is this file different from correct.c? 
It looks to me that test.c is a wrongly formatted version of correct.c and when you pass it into clang-format it is re-formatted to match correct.c

If so, you want to include this information in the cover letter (see above). Or you could write little bash script which shows how this works

> @@ -0,0 +1,69 @@
> +/*
> +This is used to check if includes are sorted or not
> +*/
> +#include<unistd.h>
> +#include<stdio.h>
> +#define true 1
> +
> +struct sample{
> +	int a;
> +};
> +
> +/*
> +This is used to check support for small function in a line
> +*/
> +int a(int x, int y)
> +{}
> +
> +int main()
> +{
> +	//This will test indentation
> +
> +if(true)
> +{
> +if(true)
> +{
> +printf("this should be indented\n");
> +if(true)
> +{
> +printf("this should be indented too\n");
> +			}
> +}
> +}
> +
> +/*
> +This will check whitespaces format
> +*/
> +
> +if((1&2)==42)
> +printf("dfadsf\n" );
> +
> +/*
> +This will test that whitespaces are not added for . and -> and the left position of *
> +*/
> + struct sample ob;
> + struct sample* ob1;
> +ob.a = 10;
> +ob1->a =10;
> +	int x;
> +	int y;
> +	char *p;
> +
> +/*
> +This will break the line. It also checks addition of spaces in binary operators
> +*/
> +	if (1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2 && 1>2)
> +	p = "adsaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
> +	printf("addddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddadad");
> +
> +	for(;;)
> +		{
> +		int z = 5;
> +break;
> +}
> +/*
> +Check for template break in template
> +*/
> +template <class P>	void updateMenuList(P*, struct menu*);
> +	return 0;
> +}
> \ No newline at end of file
> -- 
> 2.7.4
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add clang-format file for Xen Hypervisor format
  2017-04-12 11:51   ` Ishani
@ 2017-04-12 12:14     ` Lars Kurth
  0 siblings, 0 replies; 7+ messages in thread
From: Lars Kurth @ 2017-04-12 12:14 UTC (permalink / raw)
  To: Ishani; +Cc: xen-devel, cardoe


> On 12 Apr 2017, at 12:51, Ishani <chugh.ishani@research.iiit.ac.in> wrote:
> 
> Hello,
> 
> Thanks for the comments.
> I have created a markdown file for clang-format doc which I will add. 
> 
>> If so, you want to include this information in the cover letter (see above). Or you >could write little bash script which shows how this works
> 
> Can you elaborate on the task of bash script? Should it give the difference between files?

Please don't top-post (see https://en.wikipedia.org/wiki/Posting_style#Top-posting) but use https://en.wikipedia.org/wiki/Posting_style#Interleaved_style instead
Top-posting makes it really hard to follow a review on a mailing list. In this case I had to go down into the e-mail thread and identify the context of "If so, you want to include this information in the cover letter ..."

I added the reply in the right place below.

> Shall I create a new patch by incorporating the suggested changes?

Yes, basically, you want to follow https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Review.2C_Rinse_.26_Repeat

> Regards,
> Ishani
> 
> ----- Original Message -----
> From: "Lars Kurth" <lars.kurth.xen@gmail.com>
> To: "Ishani Chugh" <chugh.ishani@research.iiit.ac.in>
> Cc: "xen-devel" <xen-devel@lists.xenproject.org>, "cardoe" <cardoe@cardoe.com>
> Sent: Wednesday, April 12, 2017 4:56:53 PM
> Subject: Re: [PATCH] Add clang-format file for Xen Hypervisor format
> 
> Hi Ishani,
> 
>> On 12 Apr 2017, at 11:49, Ishani Chugh <chugh.ishani@research.iiit.ac.in> wrote:
> 
> Here you would normally add a short description of the patch (also called cover letter). In this case, you probably want to describe:
> a) What you have done
> b) Link to https://wiki.xenproject.org/wiki/Outreach_Program_Projects#Code_Standards_Checking_using_clang-format, https://docs.google.com/document/d/1tCcwZ9K38ToLGTPHZkfs2sS4s4YulrGoH8LIHwBMbg4/edit and https://docs.google.com/document/d/10NJn-QvO1TvyJJJGE2PD6FtElYCT3neBAffIqeWHdiE/edit
> c) You can also link to your github repo
> d) You may want to add the two command lines or some instructions showing how to use the patch
> 
>> Signed-off-by: Ishani Chugh <chugh.ishani@research.iiit.ac.in>
>> ---
>> tools/clang-format/tests/.clang-format | 88 ++++++++++++++++++++++++++++++++++
>> tools/clang-format/tests/correct.c     | 73 ++++++++++++++++++++++++++++
>> tools/clang-format/tests/test.c        | 69 ++++++++++++++++++++++++++
>> 3 files changed, 230 insertions(+)
>> create mode 100644 tools/clang-format/tests/.clang-format
>> create mode 100644 tools/clang-format/tests/correct.c
>> create mode 100644 tools/clang-format/tests/test.c
>> 
>> diff --git a/tools/clang-format/tests/.clang-format b/tools/clang-format/tests/.clang-format
>> new file mode 100644
>> index 0000000..2229910
>> --- /dev/null
>> +++ b/tools/clang-format/tests/.clang-format
>> @@ -0,0 +1,88 @@
>> +---
>> +Language:        Cpp
>> +# BasedOnStyle:  LLVM
> 
> This would be a good place to list in a comment, which coding style patterns the file covers. In other words, everything that has a yes in https://docs.google.com/document/d/1tCcwZ9K38ToLGTPHZkfs2sS4s4YulrGoH8LIHwBMbg4/edit
> 
> The other thing, which is a little unclear is whether the file applies to 
> - http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=CODING_STYLE only
> - http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/CODING_STYLE only
> - or both
> 
> I think it is OK to just cover one file.

...

>> diff --git a/tools/clang-format/tests/test.c b/tools/clang-format/tests/test.c
>> new file mode 100644
>> index 0000000..2e7eaf2
>> --- /dev/null
>> +++ b/tools/clang-format/tests/test.c
> 
> How is this file different from correct.c? 
> It looks to me that test.c is a wrongly formatted version of correct.c and when you pass it into clang-format it is re-formatted to match correct.c
> 
> If so, you want to include this information in the cover letter (see above). Or you could write little bash script which shows how this works

But to answer your question: this was just a suggestion to save you explaining in text what each file does. If I assume correctly, something like

#!/bin/sh
#
clang-format -i -style=.clang-format test.c > _test.c
if cmp -s _test.c correct.c"
then
   echo "test.c was transformed correctly"
else
   echo "test.c was transformed incorrectly"
fi

should explain what the two .c files do and are intended for. Note that I didn't test this (I am not familiar with the clang-format syntax). 

Cheers
Lars
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add clang-format file for Xen Hypervisor format
  2017-04-12 10:49 [PATCH] Add clang-format file for Xen Hypervisor format Ishani Chugh
  2017-04-12 11:26 ` Lars Kurth
@ 2017-04-12 15:16 ` Jan Beulich
  2017-04-12 15:38   ` Ishani
  1 sibling, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2017-04-12 15:16 UTC (permalink / raw)
  To: Ishani Chugh; +Cc: lars.kurth.xen, xen-devel, cardoe

>>> On 12.04.17 at 12:49, <chugh.ishani@research.iiit.ac.in> wrote:
> --- /dev/null
> +++ b/tools/clang-format/tests/correct.c

Is this file supposed to be in correct Xen style? Its name suggests so,
but its contents don't.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add clang-format file for Xen Hypervisor format
  2017-04-12 15:16 ` Jan Beulich
@ 2017-04-12 15:38   ` Ishani
  2017-04-18 15:37     ` Lars Kurth
  0 siblings, 1 reply; 7+ messages in thread
From: Ishani @ 2017-04-12 15:38 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Lars Kurth, xen-devel, cardoe

Hello,

I seemed to have overwritten the correct file by the output of the clang-format tool. My main idea is to compare output file and correct file and take their comparison. Since there are features which are partially implemented, they are not bound to be same. Will it be better to refine the tests into multiple files referring to features that are fully implemented, partially implemented and not supported?
Thank you for pointing out the error. I will rectify it in the following patch.

Regards,
Ishani

----- Original Message -----
From: "Jan Beulich" <JBeulich@suse.com>
To: "Ishani Chugh" <chugh.ishani@research.iiit.ac.in>
Cc: "cardoe" <cardoe@cardoe.com>, "Lars Kurth" <lars.kurth.xen@gmail.com>, "xen-devel" <xen-devel@lists.xenproject.org>
Sent: Wednesday, April 12, 2017 8:46:48 PM
Subject: Re: [Xen-devel] [PATCH] Add clang-format file for Xen Hypervisor format

>>> On 12.04.17 at 12:49, <chugh.ishani@research.iiit.ac.in> wrote:
> --- /dev/null
> +++ b/tools/clang-format/tests/correct.c

Is this file supposed to be in correct Xen style? Its name suggests so,
but its contents don't.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add clang-format file for Xen Hypervisor format
  2017-04-12 15:38   ` Ishani
@ 2017-04-18 15:37     ` Lars Kurth
  0 siblings, 0 replies; 7+ messages in thread
From: Lars Kurth @ 2017-04-18 15:37 UTC (permalink / raw)
  To: Ishani; +Cc: xen-devel, cardoe, Jan Beulich

Ishani,

> On 12 Apr 2017, at 16:38, Ishani <chugh.ishani@research.iiit.ac.in> wrote:
> 
> Hello,
> 
> I seemed to have overwritten the correct file by the output of the clang-format tool. My main idea is to compare output file and correct file and take their comparison. Since there are features which are partially implemented, they are not bound to be same. Will it be better to refine the tests into multiple files referring to features that are fully implemented, partially implemented and not supported?

I think for now this is fine. This is really just a test to see how you get on with the community workflow and conventions.

Regards
Lars
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-04-18 15:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-12 10:49 [PATCH] Add clang-format file for Xen Hypervisor format Ishani Chugh
2017-04-12 11:26 ` Lars Kurth
2017-04-12 11:51   ` Ishani
2017-04-12 12:14     ` Lars Kurth
2017-04-12 15:16 ` Jan Beulich
2017-04-12 15:38   ` Ishani
2017-04-18 15:37     ` Lars Kurth

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.