Re: [PATCH] Add clang-format file for Xen Hypervisor format

[Ishani <chugh.ishani@research.iiit.ac.in>]

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