[{"id":18634,"web_url":"https://patchwork.libcamera.org/comment/18634/","msgid":"<633eba9c-9d97-4ab2-178b-c3f35d3abf1f@ideasonboard.com>","date":"2021-08-09T16:20:11","subject":"Re: [libcamera-devel] [PATCH v2] clang-format: Regroup sort orders","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Kieran,\n\nOn 09/08/2021 18:14, Kieran Bingham wrote:\n> Utilise the clang-format header sort to provide a regex based pattern\n> match for our header inclusion coding style.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> A sample of all headers from the project (+ a couple of extra tests)\n> parsed through this can be seen at \n> https://paste.ubuntu.com/p/Bt87C6M77y/\n> \n> This was generated by:\n> git grep -h \"^#include\" | sort | uniq > all-headers.cpp\n> git add all-headers.cpp\n> git commit all-headers.cpp -m \"Header Test\"\n> ./utils/checkstyle.py | patch -p0\n> \n> While some false positives are visible in this list\n> (for instance in c/system headers section)\n> this provides a great deal of the checks automatically, and the\n> remaining cases may have to be spotted during review.\n> \n> .clang-format | 49 +++++++++++++++++++++++++++++++---\n> Documentation/coding-style.rst | 20 +++++++++++---\n> 2 files changed, 62 insertions(+), 7 deletions(-)\n> \n> diff --git a/.clang-format b/.clang-format\n> index 3a8a896e373d..56cfbfa1bec4 100644\n> --- a/.clang-format\n> +++ b/.clang-format\n> @@ -66,10 +66,53 @@ ExperimentalAutoDetectBinPacking: false\n> FixNamespaceComments: true\n> ForEachMacros:\n> - 'udev_list_entry_foreach'\n> -IncludeBlocks: Preserve\n> +SortIncludes: true\n> +IncludeBlocks: Regroup\n> IncludeCategories:\n> - - Regex: '.*'\n> - Priority: 1\n> + # Headers matching the name of the component are matched automatically.\n> + # Priority 1\n> + # Headers in <> with an extension. (+system libraries)\n> + - Regex: '<([A-Za-z0-9\\-_])+\\.h>'\n> + Priority: 2 \n> + - Regex: ''\n> + Priority: 2\n> + # Qt includes (match before C++ standard library)\n> + - Regex: ''\n> + Priority: 10\n> + CaseSensitive: true\n> + # C++ standard library includes (no extension)\n> + - Regex: '<([A-Za-z0-9\\-_/])+>'\n> + Priority: 3\n> + # Linux headers, as a second group/subset of system headers\n> + - Regex: ''\n> + Priority: 4\n> + # Headers for libcamera Base support\n> + - Regex: ''\n> + Priority: 5\n> + - Regex: ''\n> + Priority: 6\n> + # Public API Headers for libcamera, which are not in a subdir (i.e. ipa/,internal/)\n> + - Regex: ''\n> + Priority: 7\n> + # IPA Interfaces\n> + - Regex: ''\n> + Priority: 8\n> + # libcamera Internal headers in \"\" \n> + - Regex: '\"libcamera/internal/.*\\.h\"'\n> + Priority: 9\n> + # Other libraries headers with one group per library (.h or .hpp)\n> + - Regex: '<.*/.*\\.hp*>'\n> + Priority: 10\n> + # local modular includes \"path/file.h\" (.h or .hpp)\n> + - Regex: '\"(.*/)+.*\\.hp*\"'\n> + Priority: 11\n> + # Other local headers \"file.h\" with extension (.h or .hpp)\n> + - Regex: '\".*.hp*\"'\n> + Priority: 12\n> + # Any unmatched line, separated from the last group\n> + - Regex:\t '\"*\"'\n> + Priority: 100\n> +\n> IncludeIsMainRegex: '(_test)?$'\n> IndentCaseLabels: false\n> IndentPPDirectives: None\n> diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst\n> index 30c1778ed8bf..e4f1bb26024e 100644\n> --- a/Documentation/coding-style.rst\n> +++ b/Documentation/coding-style.rst\n> @@ -70,19 +70,31 @@ macro. For .cpp files, if the file implements an API declared in a header file,\n> that header file shall be included first in order to ensure it is\n> self-contained.\n> \n> +While the following list is extensive, it documents the expected behaviour\n> +defined by the clang-format configuraiton and tooling should assist with\n> +ordering.\n\ns/configuraiton/configuration\n\nReviewed-by: Jean-Michel Hautbois \n\n> The headers shall be grouped and ordered as follows:\n> \n> 1. The header declaring the API being implemented (if any)\n> -2. The C and C++ system and standard library headers\n> -3. Other libraries' headers, with one group per library\n> -4. Other project's headers\n> +2. The C standard library and system headers\n> +3. The C++ standard library headers\n> +4. Linux kernel headers\n> +5. The libcamera base private header if required\n> +6. The libcamera base library headers\n> +7. The libcamera public API headers\n> +8. The libcamera IPA interfaces\n> +9. The internal libcamera headers\n> +10. Other libraries' headers, with one group per library\n> +11. Local headers grouped by subdirectory\n> +12. Any local headers\n> \n> Groups of headers shall be separated by a single blank line. Headers within\n> each group shall be sorted alphabetically.\n> \n> System and library headers shall be included with angle brackets. Project\n> headers shall be included with angle brackets for the libcamera public API\n> -headers, and with double quotes for other libcamera headers.\n> +headers, and with double quotes for internal libcamera headers.\n> \n> \n> C++ Specific Rules\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2ECBEC3240\n\tfor ;\n\tMon, 9 Aug 2021 16:20:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9FE16687EB;\n\tMon, 9 Aug 2021 18:20:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 35FB760269\n\tfor ;\n\tMon, 9 Aug 2021 18:20:14 +0200 (CEST)","from tatooine.ideasonboard.com (unknown\n\t[IPv6:2a01:e0a:169:7140:14c5:c0eb:b1fb:db22])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C8940466;\n\tMon, 9 Aug 2021 18:20:13 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"A0cQAU6j\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628526013;\n\tbh=rPa26tfeUokF/6xnALHWjYWnLzS1Alv2iC65nqA6bNI=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=A0cQAU6jBQLB5cxo4qgFIpCLhRk3opLbTWgDzcUyBbRnGzDsZL/w7zVQ6JAa+e112\n\tlHEg8hWN4zg1bO6x5UtmgfWHxZV1kC6P3mMWY41OV2OZoKsuHvNuXE6ygSoireC5ZJ\n\tgK8iRPCALqNOKfN0DN5nuA2DmhvoWX//HpY2LvcU=","To":"Kieran Bingham ,\n\tlibcamera devel ","References":"<20210809161425.1885984-1-kieran.bingham@ideasonboard.com>","From":"Jean-Michel Hautbois ","Message-ID":"<633eba9c-9d97-4ab2-178b-c3f35d3abf1f@ideasonboard.com>","Date":"Mon, 9 Aug 2021 18:20:11 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"<20210809161425.1885984-1-kieran.bingham@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2] clang-format: Regroup sort orders","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18636,"web_url":"https://patchwork.libcamera.org/comment/18636/","msgid":"","date":"2021-08-09T16:36:12","subject":"Re: [libcamera-devel] [PATCH v2] clang-format: Regroup sort orders","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Mon, Aug 09, 2021 at 05:14:25PM +0100, Kieran Bingham wrote:\n> Utilise the clang-format header sort to provide a regex based pattern\n> match for our header inclusion coding style.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> A sample of all headers from the project (+ a couple of extra tests)\n> parsed through this can be seen at \n> https://paste.ubuntu.com/p/Bt87C6M77y/\n\nThere are a few headers there such as a.h and b.h that don't seem to\nexist in libcamera. Did you run the commands below on a test branch by\nany chance ?\n\n> This was generated by:\n> git grep -h \"^#include\" | sort | uniq > all-headers.cpp\n> git add all-headers.cpp\n> git commit all-headers.cpp -m \"Header Test\"\n> ./utils/checkstyle.py | patch -p0\n> \n> While some false positives are visible in this list\n> (for instance in c/system headers section)\n> this provides a great deal of the checks automatically, and the\n> remaining cases may have to be spotted during review.\n\nThere's also\n\n#include \n#include \n#include \n#include \n#include \n#include \n\nin the system headers. I wonder if we should add a rule for those.\n\nI've also spotted\n\n#include \"linux/bcm2835-isp.h\"\n\nthat should be replaced with\n\n#include \n\n> .clang-format | 49 +++++++++++++++++++++++++++++++---\n> Documentation/coding-style.rst | 20 +++++++++++---\n> 2 files changed, 62 insertions(+), 7 deletions(-)\n> \n> diff --git a/.clang-format b/.clang-format\n> index 3a8a896e373d..56cfbfa1bec4 100644\n> --- a/.clang-format\n> +++ b/.clang-format\n> @@ -66,10 +66,53 @@ ExperimentalAutoDetectBinPacking: false\n> FixNamespaceComments: true\n> ForEachMacros:\n> - 'udev_list_entry_foreach'\n> -IncludeBlocks: Preserve\n> +SortIncludes: true\n> +IncludeBlocks: Regroup\n> IncludeCategories:\n> - - Regex: '.*'\n> - Priority: 1\n> + # Headers matching the name of the component are matched automatically.\n> + # Priority 1\n> + # Headers in <> with an extension. (+system libraries)\n> + - Regex: '<([A-Za-z0-9\\-_])+\\.h>'\n> + Priority: 2 \n> + - Regex: ''\n> + Priority: 2\n> + # Qt includes (match before C++ standard library)\n> + - Regex: ''\n> + Priority: 10\n> + CaseSensitive: true\n> + # C++ standard library includes (no extension)\n> + - Regex: '<([A-Za-z0-9\\-_/])+>'\n> + Priority: 3\n> + # Linux headers, as a second group/subset of system headers\n> + - Regex: ''\n> + Priority: 4\n> + # Headers for libcamera Base support\n> + - Regex: ''\n> + Priority: 5\n> + - Regex: ''\n> + Priority: 6\n> + # Public API Headers for libcamera, which are not in a subdir (i.e. ipa/,internal/)\n> + - Regex: ''\n> + Priority: 7\n> + # IPA Interfaces\n> + - Regex: ''\n> + Priority: 8\n> + # libcamera Internal headers in \"\" \n> + - Regex: '\"libcamera/internal/.*\\.h\"'\n> + Priority: 9\n> + # Other libraries headers with one group per library (.h or .hpp)\n> + - Regex: '<.*/.*\\.hp*>'\n> + Priority: 10\n> + # local modular includes \"path/file.h\" (.h or .hpp)\n> + - Regex: '\"(.*/)+.*\\.hp*\"'\n> + Priority: 11\n> + # Other local headers \"file.h\" with extension (.h or .hpp)\n> + - Regex: '\".*.hp*\"'\n> + Priority: 12\n> + # Any unmatched line, separated from the last group\n> + - Regex:\t '\"*\"'\n> + Priority: 100\n> +\n> IncludeIsMainRegex: '(_test)?$'\n> IndentCaseLabels: false\n> IndentPPDirectives: None\n> diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst\n> index 30c1778ed8bf..e4f1bb26024e 100644\n> --- a/Documentation/coding-style.rst\n> +++ b/Documentation/coding-style.rst\n> @@ -70,19 +70,31 @@ macro. For .cpp files, if the file implements an API declared in a header file,\n> that header file shall be included first in order to ensure it is\n> self-contained.\n> \n> +While the following list is extensive, it documents the expected behaviour\n> +defined by the clang-format configuraiton and tooling should assist with\n> +ordering.\n> +\n> The headers shall be grouped and ordered as follows:\n> \n> 1. The header declaring the API being implemented (if any)\n> -2. The C and C++ system and standard library headers\n> -3. Other libraries' headers, with one group per library\n> -4. Other project's headers\n> +2. The C standard library and system headers\n> +3. The C++ standard library headers\n\nWhat's the rationale for splitting the C and C++ headers ?\n\n> +4. Linux kernel headers\n> +5. The libcamera base private header if required\n> +6. The libcamera base library headers\n> +7. The libcamera public API headers\n> +8. The libcamera IPA interfaces\n> +9. The internal libcamera headers\n> +10. Other libraries' headers, with one group per library\n> +11. Local headers grouped by subdirectory\n> +12. Any local headers\n\nThere are substantial changes here, does this fix the documentation to\nmatch what we actually do already, or does it change the current\npractice ?\n\n> Groups of headers shall be separated by a single blank line. Headers within\n> each group shall be sorted alphabetically.\n> \n> System and library headers shall be included with angle brackets. Project\n> headers shall be included with angle brackets for the libcamera public API\n> -headers, and with double quotes for other libcamera headers.\n> +headers, and with double quotes for internal libcamera headers.\n> \n> \n> C++ Specific Rules","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 34499C3240\n\tfor ;\n\tMon, 9 Aug 2021 16:36:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7B7E468826;\n\tMon, 9 Aug 2021 18:36:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 25BFB60269\n\tfor ;\n\tMon, 9 Aug 2021 18:36:14 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 91B47466;\n\tMon, 9 Aug 2021 18:36:13 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"SXy2M7RY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628526973;\n\tbh=LBQhII2ETueR1QNrCni9ivqG2tbtPBvLdz9Cy6lZJLE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SXy2M7RYvR07xYcFfbLNLBn0xcq5SCuvT1aowk3TQNI7qKrkha6m7W/oPj+uiQrLN\n\tATcIquSSeuoABADOOHW9vhbBesy+3MBDvpRGPGziTWaZ751bAwlqOkhab6CytKkraf\n\tbjez4O1tEw22O+N67QSxE6SYWMsVshztO7suMMs0=","Date":"Mon, 9 Aug 2021 19:36:12 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210809161425.1885984-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210809161425.1885984-1-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] clang-format: Regroup sort orders","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18640,"web_url":"https://patchwork.libcamera.org/comment/18640/","msgid":"<4f1b1b2c-a249-0cd5-dc04-6882943eca9a@ideasonboard.com>","date":"2021-08-09T17:05:36","subject":"Re: [libcamera-devel] [PATCH v2] clang-format: Regroup sort orders","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 09/08/2021 17:36, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Mon, Aug 09, 2021 at 05:14:25PM +0100, Kieran Bingham wrote:\n>> Utilise the clang-format header sort to provide a regex based pattern\n>> match for our header inclusion coding style.\n>>\n>> Signed-off-by: Kieran Bingham \n>> ---\n>> A sample of all headers from the project (+ a couple of extra tests)\n>> parsed through this can be seen at \n>> https://paste.ubuntu.com/p/Bt87C6M77y/\n> \n> There are a few headers there such as a.h and b.h that don't seem to\n> exist in libcamera. Did you run the commands below on a test branch by\n> any chance ?\n\nIntentionally, yes as stated in the three lines above your query \" (+ a\ncouple of extra tests)\"\n\n\n> \n>> This was generated by:\n>> git grep -h \"^#include\" | sort | uniq > all-headers.cpp\n>> git add all-headers.cpp\n>> git commit all-headers.cpp -m \"Header Test\"\n>> ./utils/checkstyle.py | patch -p0\n>>\n>> While some false positives are visible in this list\n>> (for instance in c/system headers section)\n>> this provides a great deal of the checks automatically, and the\n>> remaining cases may have to be spotted during review.\n> \n> There's also\n> \n> #include \n> #include \n> #include \n> #include \n> #include \n> #include \n> \n> in the system headers. I wonder if we should add a rule for those.\n\nWe would have to add that set explicitly. That doesn't seem to scale, as\nthere would likely always be some extra files we'd have to add, and I\ndon't think we should be maintaining a list of 'all files' in a regex.\n\nAs you'll see, I've added an exception rule for the QT headers, as it\ncan match generically to prevent QT headers ending up in the C++ subset.\n\n\n> I've also spotted\n> \n> #include \"linux/bcm2835-isp.h\"\n> \n> that should be replaced with\n> \n> #include \n\n\nSure - but that's a separate patch ;-) now sent.\n\n\n>> .clang-format | 49 +++++++++++++++++++++++++++++++---\n>> Documentation/coding-style.rst | 20 +++++++++++---\n>> 2 files changed, 62 insertions(+), 7 deletions(-)\n>>\n>> diff --git a/.clang-format b/.clang-format\n>> index 3a8a896e373d..56cfbfa1bec4 100644\n>> --- a/.clang-format\n>> +++ b/.clang-format\n>> @@ -66,10 +66,53 @@ ExperimentalAutoDetectBinPacking: false\n>> FixNamespaceComments: true\n>> ForEachMacros:\n>> - 'udev_list_entry_foreach'\n>> -IncludeBlocks: Preserve\n>> +SortIncludes: true\n>> +IncludeBlocks: Regroup\n>> IncludeCategories:\n>> - - Regex: '.*'\n>> - Priority: 1\n>> + # Headers matching the name of the component are matched automatically.\n>> + # Priority 1\n>> + # Headers in <> with an extension. (+system libraries)\n>> + - Regex: '<([A-Za-z0-9\\-_])+\\.h>'\n>> + Priority: 2 \n>> + - Regex: ''\n>> + Priority: 2\n>> + # Qt includes (match before C++ standard library)\n>> + - Regex: ''\n>> + Priority: 10\n>> + CaseSensitive: true\n>> + # C++ standard library includes (no extension)\n>> + - Regex: '<([A-Za-z0-9\\-_/])+>'\n>> + Priority: 3\n>> + # Linux headers, as a second group/subset of system headers\n>> + - Regex: ''\n>> + Priority: 4\n>> + # Headers for libcamera Base support\n>> + - Regex: ''\n>> + Priority: 5\n>> + - Regex: ''\n>> + Priority: 6\n>> + # Public API Headers for libcamera, which are not in a subdir (i.e. ipa/,internal/)\n>> + - Regex: ''\n>> + Priority: 7\n>> + # IPA Interfaces\n>> + - Regex: ''\n>> + Priority: 8\n>> + # libcamera Internal headers in \"\" \n>> + - Regex: '\"libcamera/internal/.*\\.h\"'\n>> + Priority: 9\n>> + # Other libraries headers with one group per library (.h or .hpp)\n>> + - Regex: '<.*/.*\\.hp*>'\n>> + Priority: 10\n>> + # local modular includes \"path/file.h\" (.h or .hpp)\n>> + - Regex: '\"(.*/)+.*\\.hp*\"'\n>> + Priority: 11\n>> + # Other local headers \"file.h\" with extension (.h or .hpp)\n>> + - Regex: '\".*.hp*\"'\n>> + Priority: 12\n>> + # Any unmatched line, separated from the last group\n>> + - Regex:\t '\"*\"'\n>> + Priority: 100\n>> +\n>> IncludeIsMainRegex: '(_test)?$'\n>> IndentCaseLabels: false\n>> IndentPPDirectives: None\n>> diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst\n>> index 30c1778ed8bf..e4f1bb26024e 100644\n>> --- a/Documentation/coding-style.rst\n>> +++ b/Documentation/coding-style.rst\n>> @@ -70,19 +70,31 @@ macro. For .cpp files, if the file implements an API declared in a header file,\n>> that header file shall be included first in order to ensure it is\n>> self-contained.\n>> \n>> +While the following list is extensive, it documents the expected behaviour\n>> +defined by the clang-format configuraiton and tooling should assist with\n>> +ordering.\n>> +\n>> The headers shall be grouped and ordered as follows:\n>> \n>> 1. The header declaring the API being implemented (if any)\n>> -2. The C and C++ system and standard library headers\n>> -3. Other libraries' headers, with one group per library\n>> -4. Other project's headers\n>> +2. The C standard library and system headers\n>> +3. The C++ standard library headers\n> \n> What's the rationale for splitting the C and C++ headers ?\n\nAt the moment just matching them. Though I thought we did so already\nsomewhere... but I can't find any examples of that, but there are plenty\nof examples of them mixed, so I will re-join groups 2 and 3.\n\n\n>> +4. Linux kernel headers\n>> +5. The libcamera base private header if required\n>> +6. The libcamera base library headers\n>> +7. The libcamera public API headers\n>> +8. The libcamera IPA interfaces\n>> +9. The internal libcamera headers\n>> +10. Other libraries' headers, with one group per library\n>> +11. Local headers grouped by subdirectory\n>> +12. Any local headers\n> \n> There are substantial changes here, does this fix the documentation to\n> match what we actually do already, or does it change the current\n> practice ?\n\nIt aims to better match what current expected practice is ... at least\nhow I see it ...\n\nSpecifically - the base interfaces are all described like that in the\nimplementation, because I used a variant of this .clang-format to help\nautomate that split work.\n\nThere are lots of rules in the headers already (like\nlibcamera/base/private.h should be in it's own group, before the other\nbase headers) which this patch codifies.\n\nThere are many times where it's not easily obvious where an include\nshould go. This tries to specify that for the readers, and ... make it\nso they don't have to look it up anyway, as the tooling will organise it\nanyway.\n\n\n\n>> Groups of headers shall be separated by a single blank line. Headers within\n>> each group shall be sorted alphabetically.\n>> \n>> System and library headers shall be included with angle brackets. Project\n>> headers shall be included with angle brackets for the libcamera public API\n>> -headers, and with double quotes for other libcamera headers.\n>> +headers, and with double quotes for internal libcamera headers.\n>> \n>> \n>> C++ Specific Rules\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4BBB2C3240\n\tfor ;\n\tMon, 9 Aug 2021 17:05:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 01FF768855;\n\tMon, 9 Aug 2021 19:05:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B6E9960269\n\tfor ;\n\tMon, 9 Aug 2021 19:05:39 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 583B3466;\n\tMon, 9 Aug 2021 19:05:38 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"R7IFtQau\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628528739;\n\tbh=p3AYaxRFYbPJogWA49GkLboogpXMZ89u5szwuv6lWJE=;\n\th=From:To:Cc:References:Subject:Date:In-Reply-To:From;\n\tb=R7IFtQauj6MkOsRqwTGMlpqcy0/JsogiTGpdBM3VoV7kMSs1pJP4C01Zk05Jwp0Tk\n\tzh7XZ1FREgOKFmjIvTWImrCwy6VAXh67X9RXDFl82XYkvfxtov2AeQsyvjJo2qK9Ia\n\tC4w0d8VDjtlMHRemcIGZvGZZ1fMwVGfwRMXKmhE8=","From":"Kieran Bingham ","To":"Laurent Pinchart ","References":"<20210809161425.1885984-1-kieran.bingham@ideasonboard.com>\n\t","Message-ID":"<4f1b1b2c-a249-0cd5-dc04-6882943eca9a@ideasonboard.com>","Date":"Mon, 9 Aug 2021 18:05:36 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2] clang-format: Regroup sort orders","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18641,"web_url":"https://patchwork.libcamera.org/comment/18641/","msgid":"","date":"2021-08-09T17:09:10","subject":"Re: [libcamera-devel] [PATCH v2] clang-format: Regroup sort orders","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Mon, Aug 09, 2021 at 06:05:36PM +0100, Kieran Bingham wrote:\n> On 09/08/2021 17:36, Laurent Pinchart wrote:\n> > On Mon, Aug 09, 2021 at 05:14:25PM +0100, Kieran Bingham wrote:\n> >> Utilise the clang-format header sort to provide a regex based pattern\n> >> match for our header inclusion coding style.\n> >>\n> >> Signed-off-by: Kieran Bingham \n> >> ---\n> >> A sample of all headers from the project (+ a couple of extra tests)\n> >> parsed through this can be seen at \n> >> https://paste.ubuntu.com/p/Bt87C6M77y/\n> > \n> > There are a few headers there such as a.h and b.h that don't seem to\n> > exist in libcamera. Did you run the commands below on a test branch by\n> > any chance ?\n> \n> Intentionally, yes as stated in the three lines above your query \" (+ a\n> couple of extra tests)\"\n\nOops indeed.\n\n> >> This was generated by:\n> >> git grep -h \"^#include\" | sort | uniq > all-headers.cpp\n> >> git add all-headers.cpp\n> >> git commit all-headers.cpp -m \"Header Test\"\n> >> ./utils/checkstyle.py | patch -p0\n> >>\n> >> While some false positives are visible in this list\n> >> (for instance in c/system headers section)\n> >> this provides a great deal of the checks automatically, and the\n> >> remaining cases may have to be spotted during review.\n> > \n> > There's also\n> > \n> > #include \n> > #include \n> > #include \n> > #include \n> > #include \n> > #include \n> > \n> > in the system headers. I wonder if we should add a rule for those.\n> \n> We would have to add that set explicitly. That doesn't seem to scale, as\n> there would likely always be some extra files we'd have to add, and I\n> don't think we should be maintaining a list of 'all files' in a regex.\n\nAs we have few external dependencies I thought this list may change\ninfrequently enough that it could be maintainable (especially\nconsidering that most external dependencies will have a directory name\nas a prefix), but I also understand the opposite argument.\n\n> As you'll see, I've added an exception rule for the QT headers, as it\n> can match generically to prevent QT headers ending up in the C++ subset.\n> \n> > I've also spotted\n> > \n> > #include \"linux/bcm2835-isp.h\"\n> > \n> > that should be replaced with\n> > \n> > #include \n> \n> Sure - but that's a separate patch ;-) now sent.\n\nThanks.\n\n> >> .clang-format | 49 +++++++++++++++++++++++++++++++---\n> >> Documentation/coding-style.rst | 20 +++++++++++---\n> >> 2 files changed, 62 insertions(+), 7 deletions(-)\n> >>\n> >> diff --git a/.clang-format b/.clang-format\n> >> index 3a8a896e373d..56cfbfa1bec4 100644\n> >> --- a/.clang-format\n> >> +++ b/.clang-format\n> >> @@ -66,10 +66,53 @@ ExperimentalAutoDetectBinPacking: false\n> >> FixNamespaceComments: true\n> >> ForEachMacros:\n> >> - 'udev_list_entry_foreach'\n> >> -IncludeBlocks: Preserve\n> >> +SortIncludes: true\n> >> +IncludeBlocks: Regroup\n> >> IncludeCategories:\n> >> - - Regex: '.*'\n> >> - Priority: 1\n> >> + # Headers matching the name of the component are matched automatically.\n> >> + # Priority 1\n> >> + # Headers in <> with an extension. (+system libraries)\n> >> + - Regex: '<([A-Za-z0-9\\-_])+\\.h>'\n> >> + Priority: 2 \n> >> + - Regex: ''\n> >> + Priority: 2\n> >> + # Qt includes (match before C++ standard library)\n> >> + - Regex: ''\n> >> + Priority: 10\n> >> + CaseSensitive: true\n> >> + # C++ standard library includes (no extension)\n> >> + - Regex: '<([A-Za-z0-9\\-_/])+>'\n> >> + Priority: 3\n> >> + # Linux headers, as a second group/subset of system headers\n> >> + - Regex: ''\n> >> + Priority: 4\n> >> + # Headers for libcamera Base support\n> >> + - Regex: ''\n> >> + Priority: 5\n> >> + - Regex: ''\n> >> + Priority: 6\n> >> + # Public API Headers for libcamera, which are not in a subdir (i.e. ipa/,internal/)\n> >> + - Regex: ''\n> >> + Priority: 7\n> >> + # IPA Interfaces\n> >> + - Regex: ''\n> >> + Priority: 8\n> >> + # libcamera Internal headers in \"\" \n> >> + - Regex: '\"libcamera/internal/.*\\.h\"'\n> >> + Priority: 9\n> >> + # Other libraries headers with one group per library (.h or .hpp)\n> >> + - Regex: '<.*/.*\\.hp*>'\n> >> + Priority: 10\n> >> + # local modular includes \"path/file.h\" (.h or .hpp)\n> >> + - Regex: '\"(.*/)+.*\\.hp*\"'\n> >> + Priority: 11\n> >> + # Other local headers \"file.h\" with extension (.h or .hpp)\n> >> + - Regex: '\".*.hp*\"'\n> >> + Priority: 12\n> >> + # Any unmatched line, separated from the last group\n> >> + - Regex:\t '\"*\"'\n> >> + Priority: 100\n> >> +\n> >> IncludeIsMainRegex: '(_test)?$'\n> >> IndentCaseLabels: false\n> >> IndentPPDirectives: None\n> >> diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst\n> >> index 30c1778ed8bf..e4f1bb26024e 100644\n> >> --- a/Documentation/coding-style.rst\n> >> +++ b/Documentation/coding-style.rst\n> >> @@ -70,19 +70,31 @@ macro. For .cpp files, if the file implements an API declared in a header file,\n> >> that header file shall be included first in order to ensure it is\n> >> self-contained.\n> >> \n> >> +While the following list is extensive, it documents the expected behaviour\n> >> +defined by the clang-format configuraiton and tooling should assist with\n> >> +ordering.\n> >> +\n> >> The headers shall be grouped and ordered as follows:\n> >> \n> >> 1. The header declaring the API being implemented (if any)\n> >> -2. The C and C++ system and standard library headers\n> >> -3. Other libraries' headers, with one group per library\n> >> -4. Other project's headers\n> >> +2. The C standard library and system headers\n> >> +3. The C++ standard library headers\n> > \n> > What's the rationale for splitting the C and C++ headers ?\n> \n> At the moment just matching them. Though I thought we did so already\n> somewhere... but I can't find any examples of that, but there are plenty\n> of examples of them mixed, so I will re-join groups 2 and 3.\n\nAck.\n\n> >> +4. Linux kernel headers\n> >> +5. The libcamera base private header if required\n> >> +6. The libcamera base library headers\n> >> +7. The libcamera public API headers\n> >> +8. The libcamera IPA interfaces\n> >> +9. The internal libcamera headers\n> >> +10. Other libraries' headers, with one group per library\n> >> +11. Local headers grouped by subdirectory\n> >> +12. Any local headers\n> > \n> > There are substantial changes here, does this fix the documentation to\n> > match what we actually do already, or does it change the current\n> > practice ?\n> \n> It aims to better match what current expected practice is ... at least\n> how I see it ...\n> \n> Specifically - the base interfaces are all described like that in the\n> implementation, because I used a variant of this .clang-format to help\n> automate that split work.\n> \n> There are lots of rules in the headers already (like\n> libcamera/base/private.h should be in it's own group, before the other\n> base headers) which this patch codifies.\n> \n> There are many times where it's not easily obvious where an include\n> should go. This tries to specify that for the readers, and ... make it\n> so they don't have to look it up anyway, as the tooling will organise it\n> anyway.\n\nFine with me. Could you mention in the commit message that the rules are\ncreated to match the existing practices, and that the documentation is\nupdated accordingly ? With that,\n\nReviewed-by: Laurent Pinchart \n\n> >> Groups of headers shall be separated by a single blank line. Headers within\n> >> each group shall be sorted alphabetically.\n> >> \n> >> System and library headers shall be included with angle brackets. Project\n> >> headers shall be included with angle brackets for the libcamera public API\n> >> -headers, and with double quotes for other libcamera headers.\n> >> +headers, and with double quotes for internal libcamera headers.\n> >> \n> >> \n> >> C++ Specific Rules","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 31292C3240\n\tfor ;\n\tMon, 9 Aug 2021 17:09:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 90C4968826;\n\tMon, 9 Aug 2021 19:09:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8179160269\n\tfor ;\n\tMon, 9 Aug 2021 19:09:12 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F016E466;\n\tMon, 9 Aug 2021 19:09:11 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Ou/I8s6Y\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628528952;\n\tbh=SOsDgADYYlnHKN68kJiJfjt0FyhOKZhW7eLWLc2Csc4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Ou/I8s6Yan8CXdBVMVL0rIGdy9pNdGWLlKXejqjQRnQNZqYaxpqjI0JK+pRrziqG+\n\tv9Ei74qJR3jCGsxrWzfGRBmB3SrPHSPxr//zvp34su5KR1CGZrVFeeiTx7GPwxydG+\n\t6NbHWCxqYI4u9/uRUjRUAerkA4/d00AnAx7vN1Bw=","Date":"Mon, 9 Aug 2021 20:09:10 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210809161425.1885984-1-kieran.bingham@ideasonboard.com>\n\t\n\t<4f1b1b2c-a249-0cd5-dc04-6882943eca9a@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<4f1b1b2c-a249-0cd5-dc04-6882943eca9a@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] clang-format: Regroup sort orders","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18642,"web_url":"https://patchwork.libcamera.org/comment/18642/","msgid":"","date":"2021-08-09T17:13:18","subject":"Re: [libcamera-devel] [PATCH v2] clang-format: Regroup sort orders","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 09/08/2021 18:09, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Mon, Aug 09, 2021 at 06:05:36PM +0100, Kieran Bingham wrote:\n>> On 09/08/2021 17:36, Laurent Pinchart wrote:\n>>> On Mon, Aug 09, 2021 at 05:14:25PM +0100, Kieran Bingham wrote:\n>>>> Utilise the clang-format header sort to provide a regex based pattern\n>>>> match for our header inclusion coding style.\n>>>>\n>>>> Signed-off-by: Kieran Bingham \n>>>> ---\n>>>> A sample of all headers from the project (+ a couple of extra tests)\n>>>> parsed through this can be seen at \n>>>> https://paste.ubuntu.com/p/Bt87C6M77y/\n>>>\n>>> There are a few headers there such as a.h and b.h that don't seem to\n>>> exist in libcamera. Did you run the commands below on a test branch by\n>>> any chance ?\n>>\n>> Intentionally, yes as stated in the three lines above your query \" (+ a\n>> couple of extra tests)\"\n> \n> Oops indeed.\n> \n>>>> This was generated by:\n>>>> git grep -h \"^#include\" | sort | uniq > all-headers.cpp\n>>>> git add all-headers.cpp\n>>>> git commit all-headers.cpp -m \"Header Test\"\n>>>> ./utils/checkstyle.py | patch -p0\n>>>>\n>>>> While some false positives are visible in this list\n>>>> (for instance in c/system headers section)\n>>>> this provides a great deal of the checks automatically, and the\n>>>> remaining cases may have to be spotted during review.\n>>>\n>>> There's also\n>>>\n>>> #include \n>>> #include \n>>> #include \n>>> #include \n>>> #include \n>>> #include \n>>>\n>>> in the system headers. I wonder if we should add a rule for those.\n>>\n>> We would have to add that set explicitly. That doesn't seem to scale, as\n>> there would likely always be some extra files we'd have to add, and I\n>> don't think we should be maintaining a list of 'all files' in a regex.\n> \n> As we have few external dependencies I thought this list may change\n> infrequently enough that it could be maintainable (especially\n> considering that most external dependencies will have a directory name\n> as a prefix), but I also understand the opposite argument.\n\nIndeed, dependencies with a directory name are better, as they already\nget matched to group 10 (9, now that I've merged C and C++).\n\n\n\n>> As you'll see, I've added an exception rule for the QT headers, as it\n>> can match generically to prevent QT headers ending up in the C++ subset.\n>>\n>>> I've also spotted\n>>>\n>>> #include \"linux/bcm2835-isp.h\"\n>>>\n>>> that should be replaced with\n>>>\n>>> #include \n>>\n>> Sure - but that's a separate patch ;-) now sent.\n> \n> Thanks.\n> \n>>>> .clang-format | 49 +++++++++++++++++++++++++++++++---\n>>>> Documentation/coding-style.rst | 20 +++++++++++---\n>>>> 2 files changed, 62 insertions(+), 7 deletions(-)\n>>>>\n>>>> diff --git a/.clang-format b/.clang-format\n>>>> index 3a8a896e373d..56cfbfa1bec4 100644\n>>>> --- a/.clang-format\n>>>> +++ b/.clang-format\n>>>> @@ -66,10 +66,53 @@ ExperimentalAutoDetectBinPacking: false\n>>>> FixNamespaceComments: true\n>>>> ForEachMacros:\n>>>> - 'udev_list_entry_foreach'\n>>>> -IncludeBlocks: Preserve\n>>>> +SortIncludes: true\n>>>> +IncludeBlocks: Regroup\n>>>> IncludeCategories:\n>>>> - - Regex: '.*'\n>>>> - Priority: 1\n>>>> + # Headers matching the name of the component are matched automatically.\n>>>> + # Priority 1\n>>>> + # Headers in <> with an extension. (+system libraries)\n>>>> + - Regex: '<([A-Za-z0-9\\-_])+\\.h>'\n>>>> + Priority: 2 \n>>>> + - Regex: ''\n>>>> + Priority: 2\n>>>> + # Qt includes (match before C++ standard library)\n>>>> + - Regex: ''\n>>>> + Priority: 10\n>>>> + CaseSensitive: true\n>>>> + # C++ standard library includes (no extension)\n>>>> + - Regex: '<([A-Za-z0-9\\-_/])+>'\n>>>> + Priority: 3\n>>>> + # Linux headers, as a second group/subset of system headers\n>>>> + - Regex: ''\n>>>> + Priority: 4\n>>>> + # Headers for libcamera Base support\n>>>> + - Regex: ''\n>>>> + Priority: 5\n>>>> + - Regex: ''\n>>>> + Priority: 6\n>>>> + # Public API Headers for libcamera, which are not in a subdir (i.e. ipa/,internal/)\n>>>> + - Regex: ''\n>>>> + Priority: 7\n>>>> + # IPA Interfaces\n>>>> + - Regex: ''\n>>>> + Priority: 8\n>>>> + # libcamera Internal headers in \"\" \n>>>> + - Regex: '\"libcamera/internal/.*\\.h\"'\n>>>> + Priority: 9\n>>>> + # Other libraries headers with one group per library (.h or .hpp)\n>>>> + - Regex: '<.*/.*\\.hp*>'\n>>>> + Priority: 10\n>>>> + # local modular includes \"path/file.h\" (.h or .hpp)\n>>>> + - Regex: '\"(.*/)+.*\\.hp*\"'\n>>>> + Priority: 11\n>>>> + # Other local headers \"file.h\" with extension (.h or .hpp)\n>>>> + - Regex: '\".*.hp*\"'\n>>>> + Priority: 12\n>>>> + # Any unmatched line, separated from the last group\n>>>> + - Regex:\t '\"*\"'\n>>>> + Priority: 100\n>>>> +\n>>>> IncludeIsMainRegex: '(_test)?$'\n>>>> IndentCaseLabels: false\n>>>> IndentPPDirectives: None\n>>>> diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst\n>>>> index 30c1778ed8bf..e4f1bb26024e 100644\n>>>> --- a/Documentation/coding-style.rst\n>>>> +++ b/Documentation/coding-style.rst\n>>>> @@ -70,19 +70,31 @@ macro. For .cpp files, if the file implements an API declared in a header file,\n>>>> that header file shall be included first in order to ensure it is\n>>>> self-contained.\n>>>> \n>>>> +While the following list is extensive, it documents the expected behaviour\n>>>> +defined by the clang-format configuraiton and tooling should assist with\n>>>> +ordering.\n>>>> +\n>>>> The headers shall be grouped and ordered as follows:\n>>>> \n>>>> 1. The header declaring the API being implemented (if any)\n>>>> -2. The C and C++ system and standard library headers\n>>>> -3. Other libraries' headers, with one group per library\n>>>> -4. Other project's headers\n>>>> +2. The C standard library and system headers\n>>>> +3. The C++ standard library headers\n>>>\n>>> What's the rationale for splitting the C and C++ headers ?\n>>\n>> At the moment just matching them. Though I thought we did so already\n>> somewhere... but I can't find any examples of that, but there are plenty\n>> of examples of them mixed, so I will re-join groups 2 and 3.\n> \n> Ack.\n\nDone, but somehow in the big merged list - it looks ugly ;-)\n\nI liked seeing them split.\n\n\n> \n>>>> +4. Linux kernel headers\n>>>> +5. The libcamera base private header if required\n>>>> +6. The libcamera base library headers\n>>>> +7. The libcamera public API headers\n>>>> +8. The libcamera IPA interfaces\n>>>> +9. The internal libcamera headers\n>>>> +10. Other libraries' headers, with one group per library\n>>>> +11. Local headers grouped by subdirectory\n>>>> +12. Any local headers\n>>>\n>>> There are substantial changes here, does this fix the documentation to\n>>> match what we actually do already, or does it change the current\n>>> practice ?\n>>\n>> It aims to better match what current expected practice is ... at least\n>> how I see it ...\n>>\n>> Specifically - the base interfaces are all described like that in the\n>> implementation, because I used a variant of this .clang-format to help\n>> automate that split work.\n>>\n>> There are lots of rules in the headers already (like\n>> libcamera/base/private.h should be in it's own group, before the other\n>> base headers) which this patch codifies.\n>>\n>> There are many times where it's not easily obvious where an include\n>> should go. This tries to specify that for the readers, and ... make it\n>> so they don't have to look it up anyway, as the tooling will organise it\n>> anyway.\n> \n> Fine with me. Could you mention in the commit message that the rules are\n> created to match the existing practices, and that the documentation is\n> updated accordingly ? With that,\n\nSure.\n\nThanks.\n\n\n\n> \n> Reviewed-by: Laurent Pinchart \n> \n>>>> Groups of headers shall be separated by a single blank line. Headers within\n>>>> each group shall be sorted alphabetically.\n>>>> \n>>>> System and library headers shall be included with angle brackets. Project\n>>>> headers shall be included with angle brackets for the libcamera public API\n>>>> -headers, and with double quotes for other libcamera headers.\n>>>> +headers, and with double quotes for internal libcamera headers.\n>>>> \n>>>> \n>>>> C++ Specific Rules\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 141CEBD87D\n\tfor ;\n\tMon, 9 Aug 2021 17:13:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7D16160269;\n\tMon, 9 Aug 2021 19:13:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F269460269\n\tfor ;\n\tMon, 9 Aug 2021 19:13:21 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 66FADDD;\n\tMon, 9 Aug 2021 19:13:21 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"bOKCsR/Y\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628529201;\n\tbh=vuTUUtqlevFYC4V1y2eCpzSD9qT7Nzrd15svi7Jtpko=;\n\th=From:Subject:To:Cc:References:Date:In-Reply-To:From;\n\tb=bOKCsR/YKE5dfDNh3BQRjm0cWpEE5mWr0MV/ophxkf4lTHqqk0raDZR60VR2zwX5R\n\tl0HPz8h6dQG1ykKHYJ5+1VRDJ63sQA7KvlSsblV/uU/3AYc2HSUg8HnOn/IJkLJe0R\n\tDLkeNBtRD6wqoXWA3REsnjBSp0dJfdJKaB2vO/54=","From":"Kieran Bingham ","To":"Laurent Pinchart ","References":"<20210809161425.1885984-1-kieran.bingham@ideasonboard.com>\n\t\n\t<4f1b1b2c-a249-0cd5-dc04-6882943eca9a@ideasonboard.com>\n\t","Message-ID":"","Date":"Mon, 9 Aug 2021 18:13:18 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2] clang-format: Regroup sort orders","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]