[{"id":24208,"web_url":"https://patchwork.libcamera.org/comment/24208/","msgid":"<20220728073311.vherciswimeweuvm@uno.localdomain>","date":"2022-07-28T07:33:11","subject":"Re: [libcamera-devel] [PATCH v5 3/9] libcamera: yaml_parser: Add\n\tgetList() function","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Thu, Jul 28, 2022 at 01:21:43AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> From: Florian Sylvestre \n>\n> Allow to retrieve a YAML list of any already supported types in a\n> std::vector.\n>\n> Signed-off-by: Florian Sylvestre \n> Tested-by: Naushir Patuck \n> Reviewed-by: Laurent Pinchart \n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/internal/yaml_parser.h | 16 +++++++\n> src/libcamera/yaml_parser.cpp | 53 ++++++++++++++++++++++++\n> test/yaml-parser.cpp | 6 +++\n> 3 files changed, 75 insertions(+)\n>\n> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> index 61f2223223a7..4b3da0f03512 100644\n> --- a/include/libcamera/internal/yaml_parser.h\n> +++ b/include/libcamera/internal/yaml_parser.h\n> @@ -174,6 +174,22 @@ public:\n> \t\treturn get().value_or(defaultValue);\n> \t}\n>\n> +#ifndef __DOXYGEN__\n> +\ttemplate +\t\t typename std::enable_if_t<\n\nnit: as you're using the enable_if_t helper, I guess the 'typename' keyword\ncan be dropped ? (gcc agrees)\n\n> +\t\t\t std::is_same_v ||\n> +\t\t\t std::is_same_v ||\n> +\t\t\t std::is_same_v ||\n> +\t\t\t std::is_same_v ||\n> +\t\t\t std::is_same_v ||\n> +\t\t\t std::is_same_v ||\n> +\t\t\t std::is_same_v ||\n> +\t\t\t std::is_same_v> * = nullptr>\n> +#else\n> +\ttemplate\n> +#endif\n\nWhy this #ifdef ? There are no overloads to confuse doxygen with,\nright ?\n\n> +\tstd::optional> getList() const;\n> +\n> \tDictAdapter asDict() const { return DictAdapter{ dictionary_ }; }\n> \tListAdapter asList() const { return ListAdapter{ list_ }; }\n>\n> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\n> index 4299f5abd38a..cb4f1eb29a77 100644\n> --- a/src/libcamera/yaml_parser.cpp\n> +++ b/src/libcamera/yaml_parser.cpp\n> @@ -293,6 +293,59 @@ std::optional YamlObject::get() const\n>\n> #endif /* __DOXYGEN__ */\n>\n> +/**\n> + * \\fn template YamlObject::getList() const\n> + * \\brief Parse the YamlObject as a list of \\a T\n> + *\n> + * This function parses the value of the YamlObject as a list of \\a T objects,\n> + * and returns the value as a \\a std::vector. If parsing fails, std::nullopt\n> + * is returned.\n> + *\n> + * \\return The YamlObject value as a std::vector, or std::nullopt if parsing\n> + * failed\n> + */\n> +\n> +#ifndef __DOXYGEN__\n> +\n> +template +\t typename std::enable_if_t<\n> +\t\t std::is_same_v ||\n> +\t\t std::is_same_v ||\n> +\t\t std::is_same_v ||\n> +\t\t std::is_same_v ||\n> +\t\t std::is_same_v ||\n> +\t\t std::is_same_v ||\n> +\t\t std::is_same_v ||\n> +\t\t std::is_same_v> *>\n> +std::optional> YamlObject::getList() const\n> +{\n> +\tif (type_ != Type::List)\n> +\t\treturn {};\n> +\n> +\tstd::vector values;\n> +\tvalues.reserve(list_.size());\n> +\n> +\tfor (const YamlObject &entry : asList()) {\n> +\t\tconst auto value = entry.get();\n\nShould we be concerned about the type of value ?\nIf not\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> +\t\tif (!value)\n> +\t\t\treturn {};\n> +\t\tvalues.emplace_back(*value);\n> +\t}\n> +\n> +\treturn values;\n> +}\n> +\n> +template std::optional> YamlObject::getList() const;\n> +template std::optional> YamlObject::getList() const;\n> +template std::optional> YamlObject::getList() const;\n> +template std::optional> YamlObject::getList() const;\n> +template std::optional> YamlObject::getList() const;\n> +template std::optional> YamlObject::getList() const;\n> +template std::optional> YamlObject::getList() const;\n> +template std::optional> YamlObject::getList() const;\n> +\n> +#endif /* __DOXYGEN__ */\n> +\n> /**\n> * \\fn YamlObject::asDict() const\n> * \\brief Wrap a dictionary YamlObject in an adapter that exposes iterators\n> diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> index ebb654f2ef9c..bb54e0d7f76a 100644\n> --- a/test/yaml-parser.cpp\n> +++ b/test/yaml-parser.cpp\n> @@ -523,6 +523,12 @@ protected:\n> \t\t\treturn TestFail;\n> \t\t}\n>\n> +\t\tconst auto &values = firstElement.getList();\n> +\t\tif (!values || values->size() != 2 || (*values)[0] != 1 || (*values)[1] != 2) {\n> +\t\t\tcerr << \"getList() failed to return correct vector\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> \t\tauto &secondElement = level2Obj[1];\n> \t\tif (!secondElement.isDictionary() ||\n> \t\t !secondElement.contains(\"one\") ||\n> --\n> Regards,\n>\n> Laurent Pinchart\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 3729BC3275\n\tfor ;\n\tThu, 28 Jul 2022 07:33:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8609763311;\n\tThu, 28 Jul 2022 09:33:16 +0200 (CEST)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 45023603EB\n\tfor ;\n\tThu, 28 Jul 2022 09:33:14 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 517262000E;\n\tThu, 28 Jul 2022 07:33:12 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658993596;\n\tbh=LnR3CuQ8YD7cIXQBkrhcBu8+qmcd3QNqcMsrk8lcGdQ=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=l9aGktgsEdt9dbvtrw0VKAQE5yFU54fo5OnJbuDVdljsx3D3FNH5iRb0PXqGrDnb3\n\twSLNHLRb7ry0DPcM/ADe2xWURJcIypwMaosDp3OEZ190vobnDV4N49BuawA2iWDdil\n\t8/fvvFCrOtvDrxCraqp+nsjk8G8BrboPqBpPlL+YEGGsnzIQM+AciyH2HnYqjYaQTm\n\twXoia1bTf0fsqPUrAoiWN/ie1rsKW3drpb7u1Qh+C1Icnw9Sj51BBcJmnA+bl8h1Y7\n\tWZYjRTKoep4VhQbcUTs8/NG7UWPSpsuFkGx+v9iFLzfSUlw+rOGsrkLFhhOhfQCStK\n\tBF8V+awQ+kOTQ==","Date":"Thu, 28 Jul 2022 09:33:11 +0200","To":"Laurent Pinchart ","Message-ID":"<20220728073311.vherciswimeweuvm@uno.localdomain>","References":"<20220727222149.30627-1-laurent.pinchart@ideasonboard.com>\n\t<20220727222149.30627-4-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220727222149.30627-4-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 3/9] libcamera: yaml_parser: Add\n\tgetList() function","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24214,"web_url":"https://patchwork.libcamera.org/comment/24214/","msgid":"","date":"2022-07-28T10:27:37","subject":"Re: [libcamera-devel] [PATCH v5 3/9] libcamera: yaml_parser: Add\n\tgetList() function","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Thu, Jul 28, 2022 at 09:33:11AM +0200, Jacopo Mondi wrote:\n> On Thu, Jul 28, 2022 at 01:21:43AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > From: Florian Sylvestre \n> >\n> > Allow to retrieve a YAML list of any already supported types in a\n> > std::vector.\n> >\n> > Signed-off-by: Florian Sylvestre \n> > Tested-by: Naushir Patuck \n> > Reviewed-by: Laurent Pinchart \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > include/libcamera/internal/yaml_parser.h | 16 +++++++\n> > src/libcamera/yaml_parser.cpp | 53 ++++++++++++++++++++++++\n> > test/yaml-parser.cpp | 6 +++\n> > 3 files changed, 75 insertions(+)\n> >\n> > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> > index 61f2223223a7..4b3da0f03512 100644\n> > --- a/include/libcamera/internal/yaml_parser.h\n> > +++ b/include/libcamera/internal/yaml_parser.h\n> > @@ -174,6 +174,22 @@ public:\n> > \t\treturn get().value_or(defaultValue);\n> > \t}\n> >\n> > +#ifndef __DOXYGEN__\n> > +\ttemplate > +\t\t typename std::enable_if_t<\n> \n> nit: as you're using the enable_if_t helper, I guess the 'typename' keyword\n> can be dropped ? (gcc agrees)\n\nYou're right. We have many occurrences of this through the code base,\nI'll fix them all on top of this series.\n\n> > +\t\t\t std::is_same_v ||\n> > +\t\t\t std::is_same_v ||\n> > +\t\t\t std::is_same_v ||\n> > +\t\t\t std::is_same_v ||\n> > +\t\t\t std::is_same_v ||\n> > +\t\t\t std::is_same_v ||\n> > +\t\t\t std::is_same_v ||\n> > +\t\t\t std::is_same_v> * = nullptr>\n> > +#else\n> > +\ttemplate\n> > +#endif\n> \n> Why this #ifdef ? There are no overloads to confuse doxygen with,\n> right ?\n\nIt's to avoid the enable_if appearing in the generated documentation.\nOtherwise it gets rendered as\n\nā—† getList()\ntemplate||std::is_same_v< double, T >||std::is_same_v< int16_t, T >||std::is_same_v< uint16_t, T >||std::is_same_v< int32_t, T >||std::is_same_v< uint32_t, T >||std::is_same_v< std::string, T >||std::is_same_v< Size, T > > * = nullptr>\ntemplate< typename T > libcamera::YamlObject::getList< T > ( ) const\n\nwhich isn't very nice. We do the same in other places, I can reconsider\nthis, but I'd then like to do it globally.\n\n> > +\tstd::optional> getList() const;\n> > +\n> > \tDictAdapter asDict() const { return DictAdapter{ dictionary_ }; }\n> > \tListAdapter asList() const { return ListAdapter{ list_ }; }\n> >\n> > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\n> > index 4299f5abd38a..cb4f1eb29a77 100644\n> > --- a/src/libcamera/yaml_parser.cpp\n> > +++ b/src/libcamera/yaml_parser.cpp\n> > @@ -293,6 +293,59 @@ std::optional YamlObject::get() const\n> >\n> > #endif /* __DOXYGEN__ */\n> >\n> > +/**\n> > + * \\fn template YamlObject::getList() const\n> > + * \\brief Parse the YamlObject as a list of \\a T\n> > + *\n> > + * This function parses the value of the YamlObject as a list of \\a T objects,\n> > + * and returns the value as a \\a std::vector. If parsing fails, std::nullopt\n> > + * is returned.\n> > + *\n> > + * \\return The YamlObject value as a std::vector, or std::nullopt if parsing\n> > + * failed\n> > + */\n> > +\n> > +#ifndef __DOXYGEN__\n> > +\n> > +template > +\t typename std::enable_if_t<\n> > +\t\t std::is_same_v ||\n> > +\t\t std::is_same_v ||\n> > +\t\t std::is_same_v ||\n> > +\t\t std::is_same_v ||\n> > +\t\t std::is_same_v ||\n> > +\t\t std::is_same_v ||\n> > +\t\t std::is_same_v ||\n> > +\t\t std::is_same_v> *>\n> > +std::optional> YamlObject::getList() const\n> > +{\n> > +\tif (type_ != Type::List)\n> > +\t\treturn {};\n> > +\n> > +\tstd::vector values;\n> > +\tvalues.reserve(list_.size());\n> > +\n> > +\tfor (const YamlObject &entry : asList()) {\n> > +\t\tconst auto value = entry.get();\n> \n> Should we be concerned about the type of value ?\n\nI'm not sure what you mean. Would you rather write\n\n\t\tconst std::optional value = entry.get();\n\nor am I missing your point ?\n\n> If not\n> \n> Reviewed-by: Jacopo Mondi \n> \n> > +\t\tif (!value)\n> > +\t\t\treturn {};\n> > +\t\tvalues.emplace_back(*value);\n> > +\t}\n> > +\n> > +\treturn values;\n> > +}\n> > +\n> > +template std::optional> YamlObject::getList() const;\n> > +template std::optional> YamlObject::getList() const;\n> > +template std::optional> YamlObject::getList() const;\n> > +template std::optional> YamlObject::getList() const;\n> > +template std::optional> YamlObject::getList() const;\n> > +template std::optional> YamlObject::getList() const;\n> > +template std::optional> YamlObject::getList() const;\n> > +template std::optional> YamlObject::getList() const;\n> > +\n> > +#endif /* __DOXYGEN__ */\n> > +\n> > /**\n> > * \\fn YamlObject::asDict() const\n> > * \\brief Wrap a dictionary YamlObject in an adapter that exposes iterators\n> > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> > index ebb654f2ef9c..bb54e0d7f76a 100644\n> > --- a/test/yaml-parser.cpp\n> > +++ b/test/yaml-parser.cpp\n> > @@ -523,6 +523,12 @@ protected:\n> > \t\t\treturn TestFail;\n> > \t\t}\n> >\n> > +\t\tconst auto &values = firstElement.getList();\n> > +\t\tif (!values || values->size() != 2 || (*values)[0] != 1 || (*values)[1] != 2) {\n> > +\t\t\tcerr << \"getList() failed to return correct vector\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > \t\tauto &secondElement = level2Obj[1];\n> > \t\tif (!secondElement.isDictionary() ||\n> > \t\t !secondElement.contains(\"one\") ||","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 5753CBE173\n\tfor ;\n\tThu, 28 Jul 2022 10:27:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BD07D63312;\n\tThu, 28 Jul 2022 12:27:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 65AD06330D\n\tfor ;\n\tThu, 28 Jul 2022 12:27:40 +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 D4DF756D;\n\tThu, 28 Jul 2022 12:27:39 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659004061;\n\tbh=t0RCwSnRBiMIoDbiAXVBbvJtiHQZjlmfLEv1c6Qi0d0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=O9jq/NFfFYBJwyEicZvSTPc7tTPfrivjSxlzOpdx3t8jV1ifDOJmMh7ejgXjXjSZG\n\taCawxHOdVIE3GbbKs2YbHxqhs64zVBsilCU/WaL4BSAKaHKWwyX8kCfztCx9QnuFCa\n\t3P2FyCZVWAJXQzBTRy9kJNaj5K/uMH9RuNip0nR37e1kNBBjFuBbodX8N7I31y3k2S\n\tY2WYrAjNT4Y3AbQnqlXb3j1oGa24oN5v9CiRafInpio6zN1qz7oE3LW3Q2cKO4iqr+\n\t25C7gC8h6nrsndSa3b3YhJsDf0HpRetEHFU13+BK+PzaOG70NxxIihDm3YN/t59Iep\n\tc1Fz1TylEfcYg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659004060;\n\tbh=t0RCwSnRBiMIoDbiAXVBbvJtiHQZjlmfLEv1c6Qi0d0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=j9tUFdMbhwu2jMmYudTd20WU4liFa7nAovkPFM6bkundHmmk/j+thKpwn7TUTHR0t\n\tPWx6yP36zyI6bv8PX28yzY3HOV7bP5gFOFf/kYwPjPciJldKmqMDSNkrJ1zzyuEd05\n\t33dERuFv6FSNV4UcZV5DTigapGspRzrO4FPeh+hA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"j9tUFdMb\"; dkim-atps=neutral","Date":"Thu, 28 Jul 2022 13:27:37 +0300","To":"Jacopo Mondi ","Message-ID":"","References":"<20220727222149.30627-1-laurent.pinchart@ideasonboard.com>\n\t<20220727222149.30627-4-laurent.pinchart@ideasonboard.com>\n\t<20220728073311.vherciswimeweuvm@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20220728073311.vherciswimeweuvm@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v5 3/9] libcamera: yaml_parser: Add\n\tgetList() function","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24217,"web_url":"https://patchwork.libcamera.org/comment/24217/","msgid":"<20220728103902.exwigio3b2qcttpg@uno.localdomain>","date":"2022-07-28T10:39:02","subject":"Re: [libcamera-devel] [PATCH v5 3/9] libcamera: yaml_parser: Add\n\tgetList() function","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi LAurent,\n\nOn Thu, Jul 28, 2022 at 01:27:37PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Thu, Jul 28, 2022 at 09:33:11AM +0200, Jacopo Mondi wrote:\n> > On Thu, Jul 28, 2022 at 01:21:43AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > From: Florian Sylvestre \n> > >\n> > > Allow to retrieve a YAML list of any already supported types in a\n> > > std::vector.\n> > >\n> > > Signed-off-by: Florian Sylvestre \n> > > Tested-by: Naushir Patuck \n> > > Reviewed-by: Laurent Pinchart \n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > include/libcamera/internal/yaml_parser.h | 16 +++++++\n> > > src/libcamera/yaml_parser.cpp | 53 ++++++++++++++++++++++++\n> > > test/yaml-parser.cpp | 6 +++\n> > > 3 files changed, 75 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> > > index 61f2223223a7..4b3da0f03512 100644\n> > > --- a/include/libcamera/internal/yaml_parser.h\n> > > +++ b/include/libcamera/internal/yaml_parser.h\n> > > @@ -174,6 +174,22 @@ public:\n> > > \t\treturn get().value_or(defaultValue);\n> > > \t}\n> > >\n> > > +#ifndef __DOXYGEN__\n> > > +\ttemplate > > +\t\t typename std::enable_if_t<\n> >\n> > nit: as you're using the enable_if_t helper, I guess the 'typename' keyword\n> > can be dropped ? (gcc agrees)\n>\n> You're right. We have many occurrences of this through the code base,\n> I'll fix them all on top of this series.\n>\n> > > +\t\t\t std::is_same_v ||\n> > > +\t\t\t std::is_same_v ||\n> > > +\t\t\t std::is_same_v ||\n> > > +\t\t\t std::is_same_v ||\n> > > +\t\t\t std::is_same_v ||\n> > > +\t\t\t std::is_same_v ||\n> > > +\t\t\t std::is_same_v ||\n> > > +\t\t\t std::is_same_v> * = nullptr>\n> > > +#else\n> > > +\ttemplate\n> > > +#endif\n> >\n> > Why this #ifdef ? There are no overloads to confuse doxygen with,\n> > right ?\n>\n> It's to avoid the enable_if appearing in the generated documentation.\n> Otherwise it gets rendered as\n>\n> ā—† getList()\n> template||std::is_same_v< double, T >||std::is_same_v< int16_t, T >||std::is_same_v< uint16_t, T >||std::is_same_v< int32_t, T >||std::is_same_v< uint32_t, T >||std::is_same_v< std::string, T >||std::is_same_v< Size, T > > * = nullptr>\n> template< typename T > libcamera::YamlObject::getList< T > ( ) const\n>\n> which isn't very nice. We do the same in other places, I can reconsider\n> this, but I'd then like to do it globally.\n>\n> > > +\tstd::optional> getList() const;\n> > > +\n> > > \tDictAdapter asDict() const { return DictAdapter{ dictionary_ }; }\n> > > \tListAdapter asList() const { return ListAdapter{ list_ }; }\n> > >\n> > > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\n> > > index 4299f5abd38a..cb4f1eb29a77 100644\n> > > --- a/src/libcamera/yaml_parser.cpp\n> > > +++ b/src/libcamera/yaml_parser.cpp\n> > > @@ -293,6 +293,59 @@ std::optional YamlObject::get() const\n> > >\n> > > #endif /* __DOXYGEN__ */\n> > >\n> > > +/**\n> > > + * \\fn template YamlObject::getList() const\n> > > + * \\brief Parse the YamlObject as a list of \\a T\n> > > + *\n> > > + * This function parses the value of the YamlObject as a list of \\a T objects,\n> > > + * and returns the value as a \\a std::vector. If parsing fails, std::nullopt\n> > > + * is returned.\n> > > + *\n> > > + * \\return The YamlObject value as a std::vector, or std::nullopt if parsing\n> > > + * failed\n> > > + */\n> > > +\n> > > +#ifndef __DOXYGEN__\n> > > +\n> > > +template > > +\t typename std::enable_if_t<\n> > > +\t\t std::is_same_v ||\n> > > +\t\t std::is_same_v ||\n> > > +\t\t std::is_same_v ||\n> > > +\t\t std::is_same_v ||\n> > > +\t\t std::is_same_v ||\n> > > +\t\t std::is_same_v ||\n> > > +\t\t std::is_same_v ||\n> > > +\t\t std::is_same_v> *>\n> > > +std::optional> YamlObject::getList() const\n> > > +{\n> > > +\tif (type_ != Type::List)\n> > > +\t\treturn {};\n> > > +\n> > > +\tstd::vector values;\n> > > +\tvalues.reserve(list_.size());\n> > > +\n> > > +\tfor (const YamlObject &entry : asList()) {\n> > > +\t\tconst auto value = entry.get();\n> >\n> > Should we be concerned about the type of value ?\n>\n> I'm not sure what you mean. Would you rather write\n>\n> \t\tconst std::optional value = entry.get();\n>\n> or am I missing your point ?\n>\n\nI meant if we have to check if the type_ of value is of the expected\ntype :)\n\n> > If not\n> >\n> > Reviewed-by: Jacopo Mondi \n> >\n> > > +\t\tif (!value)\n> > > +\t\t\treturn {};\n> > > +\t\tvalues.emplace_back(*value);\n> > > +\t}\n> > > +\n> > > +\treturn values;\n> > > +}\n> > > +\n> > > +template std::optional> YamlObject::getList() const;\n> > > +template std::optional> YamlObject::getList() const;\n> > > +template std::optional> YamlObject::getList() const;\n> > > +template std::optional> YamlObject::getList() const;\n> > > +template std::optional> YamlObject::getList() const;\n> > > +template std::optional> YamlObject::getList() const;\n> > > +template std::optional> YamlObject::getList() const;\n> > > +template std::optional> YamlObject::getList() const;\n> > > +\n> > > +#endif /* __DOXYGEN__ */\n> > > +\n> > > /**\n> > > * \\fn YamlObject::asDict() const\n> > > * \\brief Wrap a dictionary YamlObject in an adapter that exposes iterators\n> > > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> > > index ebb654f2ef9c..bb54e0d7f76a 100644\n> > > --- a/test/yaml-parser.cpp\n> > > +++ b/test/yaml-parser.cpp\n> > > @@ -523,6 +523,12 @@ protected:\n> > > \t\t\treturn TestFail;\n> > > \t\t}\n> > >\n> > > +\t\tconst auto &values = firstElement.getList();\n> > > +\t\tif (!values || values->size() != 2 || (*values)[0] != 1 || (*values)[1] != 2) {\n> > > +\t\t\tcerr << \"getList() failed to return correct vector\" << std::endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> > > +\n> > > \t\tauto &secondElement = level2Obj[1];\n> > > \t\tif (!secondElement.isDictionary() ||\n> > > \t\t !secondElement.contains(\"one\") ||\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 54374BE173\n\tfor ;\n\tThu, 28 Jul 2022 10:39:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B9CCA63312;\n\tThu, 28 Jul 2022 12:39:06 +0200 (CEST)","from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::223])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8B6786330D\n\tfor ;\n\tThu, 28 Jul 2022 12:39:05 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id AACA460008;\n\tThu, 28 Jul 2022 10:39:04 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659004746;\n\tbh=ys1lTB6LGWOzOYOfGXGzWsQUqFRno3xf4MJqlcrPaLA=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=rUuyVclHeRlrYF9pDK2okJiijBx7ia18zOD2kNaNskISPM3As1GFq7PZlZDZRAhA7\n\t+vbhy/nyOC+x6X3L91IHMw2ZASeMT3fD1MPlDtnPCBs1FkEtr+qia8+PQmBonJq09n\n\tVxaqa8oxvmiGl2Itih0+KrVPzhLaHYmNIBTyiWHAZMKdQN1GDIH2Ds8VVQe78PtAqo\n\twBYbRReGnNMwYBfF/xQimqyO0kF+Xub1boslXLYugN5Rq7mAHcxd3Z6ka2WS2myUk1\n\t3ywvbxubDq0OQQDajOOJ3H7sWokC4UjEw4aGalZUEJxA7XHRhvizVqkZIZ69NNfjp+\n\tdyfVeGuOd464g==","Date":"Thu, 28 Jul 2022 12:39:02 +0200","To":"Laurent Pinchart ","Message-ID":"<20220728103902.exwigio3b2qcttpg@uno.localdomain>","References":"<20220727222149.30627-1-laurent.pinchart@ideasonboard.com>\n\t<20220727222149.30627-4-laurent.pinchart@ideasonboard.com>\n\t<20220728073311.vherciswimeweuvm@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v5 3/9] libcamera: yaml_parser: Add\n\tgetList() function","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24218,"web_url":"https://patchwork.libcamera.org/comment/24218/","msgid":"","date":"2022-07-28T10:40:31","subject":"Re: [libcamera-devel] [PATCH v5 3/9] libcamera: yaml_parser: Add\n\tgetList() function","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Thu, Jul 28, 2022 at 12:39:02PM +0200, Jacopo Mondi wrote:\n> On Thu, Jul 28, 2022 at 01:27:37PM +0300, Laurent Pinchart wrote:\n> > On Thu, Jul 28, 2022 at 09:33:11AM +0200, Jacopo Mondi wrote:\n> > > On Thu, Jul 28, 2022 at 01:21:43AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > > From: Florian Sylvestre \n> > > >\n> > > > Allow to retrieve a YAML list of any already supported types in a\n> > > > std::vector.\n> > > >\n> > > > Signed-off-by: Florian Sylvestre \n> > > > Tested-by: Naushir Patuck \n> > > > Reviewed-by: Laurent Pinchart \n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > include/libcamera/internal/yaml_parser.h | 16 +++++++\n> > > > src/libcamera/yaml_parser.cpp | 53 ++++++++++++++++++++++++\n> > > > test/yaml-parser.cpp | 6 +++\n> > > > 3 files changed, 75 insertions(+)\n> > > >\n> > > > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> > > > index 61f2223223a7..4b3da0f03512 100644\n> > > > --- a/include/libcamera/internal/yaml_parser.h\n> > > > +++ b/include/libcamera/internal/yaml_parser.h\n> > > > @@ -174,6 +174,22 @@ public:\n> > > > \t\treturn get().value_or(defaultValue);\n> > > > \t}\n> > > >\n> > > > +#ifndef __DOXYGEN__\n> > > > +\ttemplate > > > +\t\t typename std::enable_if_t<\n> > >\n> > > nit: as you're using the enable_if_t helper, I guess the 'typename' keyword\n> > > can be dropped ? (gcc agrees)\n> >\n> > You're right. We have many occurrences of this through the code base,\n> > I'll fix them all on top of this series.\n> >\n> > > > +\t\t\t std::is_same_v ||\n> > > > +\t\t\t std::is_same_v ||\n> > > > +\t\t\t std::is_same_v ||\n> > > > +\t\t\t std::is_same_v ||\n> > > > +\t\t\t std::is_same_v ||\n> > > > +\t\t\t std::is_same_v ||\n> > > > +\t\t\t std::is_same_v ||\n> > > > +\t\t\t std::is_same_v> * = nullptr>\n> > > > +#else\n> > > > +\ttemplate\n> > > > +#endif\n> > >\n> > > Why this #ifdef ? There are no overloads to confuse doxygen with,\n> > > right ?\n> >\n> > It's to avoid the enable_if appearing in the generated documentation.\n> > Otherwise it gets rendered as\n> >\n> > ā—† getList()\n> > template||std::is_same_v< double, T >||std::is_same_v< int16_t, T >||std::is_same_v< uint16_t, T >||std::is_same_v< int32_t, T >||std::is_same_v< uint32_t, T >||std::is_same_v< std::string, T >||std::is_same_v< Size, T > > * = nullptr>\n> > template< typename T > libcamera::YamlObject::getList< T > ( ) const\n> >\n> > which isn't very nice. We do the same in other places, I can reconsider\n> > this, but I'd then like to do it globally.\n> >\n> > > > +\tstd::optional> getList() const;\n> > > > +\n> > > > \tDictAdapter asDict() const { return DictAdapter{ dictionary_ }; }\n> > > > \tListAdapter asList() const { return ListAdapter{ list_ }; }\n> > > >\n> > > > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\n> > > > index 4299f5abd38a..cb4f1eb29a77 100644\n> > > > --- a/src/libcamera/yaml_parser.cpp\n> > > > +++ b/src/libcamera/yaml_parser.cpp\n> > > > @@ -293,6 +293,59 @@ std::optional YamlObject::get() const\n> > > >\n> > > > #endif /* __DOXYGEN__ */\n> > > >\n> > > > +/**\n> > > > + * \\fn template YamlObject::getList() const\n> > > > + * \\brief Parse the YamlObject as a list of \\a T\n> > > > + *\n> > > > + * This function parses the value of the YamlObject as a list of \\a T objects,\n> > > > + * and returns the value as a \\a std::vector. If parsing fails, std::nullopt\n> > > > + * is returned.\n> > > > + *\n> > > > + * \\return The YamlObject value as a std::vector, or std::nullopt if parsing\n> > > > + * failed\n> > > > + */\n> > > > +\n> > > > +#ifndef __DOXYGEN__\n> > > > +\n> > > > +template > > > +\t typename std::enable_if_t<\n> > > > +\t\t std::is_same_v ||\n> > > > +\t\t std::is_same_v ||\n> > > > +\t\t std::is_same_v ||\n> > > > +\t\t std::is_same_v ||\n> > > > +\t\t std::is_same_v ||\n> > > > +\t\t std::is_same_v ||\n> > > > +\t\t std::is_same_v ||\n> > > > +\t\t std::is_same_v> *>\n> > > > +std::optional> YamlObject::getList() const\n> > > > +{\n> > > > +\tif (type_ != Type::List)\n> > > > +\t\treturn {};\n> > > > +\n> > > > +\tstd::vector values;\n> > > > +\tvalues.reserve(list_.size());\n> > > > +\n> > > > +\tfor (const YamlObject &entry : asList()) {\n> > > > +\t\tconst auto value = entry.get();\n> > >\n> > > Should we be concerned about the type of value ?\n> >\n> > I'm not sure what you mean. Would you rather write\n> >\n> > \t\tconst std::optional value = entry.get();\n> >\n> > or am I missing your point ?\n> \n> I meant if we have to check if the type_ of value is of the expected\n> type :)\n\nIf it's not of type T, get() will return std::nullopt, so the !value\ncheck below handles that.\n\n> > > If not\n> > >\n> > > Reviewed-by: Jacopo Mondi \n> > >\n> > > > +\t\tif (!value)\n> > > > +\t\t\treturn {};\n> > > > +\t\tvalues.emplace_back(*value);\n> > > > +\t}\n> > > > +\n> > > > +\treturn values;\n> > > > +}\n> > > > +\n> > > > +template std::optional> YamlObject::getList() const;\n> > > > +template std::optional> YamlObject::getList() const;\n> > > > +template std::optional> YamlObject::getList() const;\n> > > > +template std::optional> YamlObject::getList() const;\n> > > > +template std::optional> YamlObject::getList() const;\n> > > > +template std::optional> YamlObject::getList() const;\n> > > > +template std::optional> YamlObject::getList() const;\n> > > > +template std::optional> YamlObject::getList() const;\n> > > > +\n> > > > +#endif /* __DOXYGEN__ */\n> > > > +\n> > > > /**\n> > > > * \\fn YamlObject::asDict() const\n> > > > * \\brief Wrap a dictionary YamlObject in an adapter that exposes iterators\n> > > > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> > > > index ebb654f2ef9c..bb54e0d7f76a 100644\n> > > > --- a/test/yaml-parser.cpp\n> > > > +++ b/test/yaml-parser.cpp\n> > > > @@ -523,6 +523,12 @@ protected:\n> > > > \t\t\treturn TestFail;\n> > > > \t\t}\n> > > >\n> > > > +\t\tconst auto &values = firstElement.getList();\n> > > > +\t\tif (!values || values->size() != 2 || (*values)[0] != 1 || (*values)[1] != 2) {\n> > > > +\t\t\tcerr << \"getList() failed to return correct vector\" << std::endl;\n> > > > +\t\t\treturn TestFail;\n> > > > +\t\t}\n> > > > +\n> > > > \t\tauto &secondElement = level2Obj[1];\n> > > > \t\tif (!secondElement.isDictionary() ||\n> > > > \t\t !secondElement.contains(\"one\") ||","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 04F40BE173\n\tfor ;\n\tThu, 28 Jul 2022 10:40:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C210A63312;\n\tThu, 28 Jul 2022 12:40:35 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9BCDA6330D\n\tfor ;\n\tThu, 28 Jul 2022 12:40:34 +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 1A25656D;\n\tThu, 28 Jul 2022 12:40:34 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659004835;\n\tbh=Xh0R4H8yNNChhUUDKRF6m9XIoZgdRcaQ7h2D6R1lJqk=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=4eZmfMYHctUROrkI6IspSjKchtRE04ltX22LyRVekAAA1L2r+THcKn91YL/liAyZz\n\teYvsXjTF4jlh87BIH4R74OU4zL0u7FyZ6pomWXhP3gIQo2htPrWkFiO+RBJ1gyRyHz\n\tmZbShvfHvGgGb5Hu63eutxM8BPxf6EKjI6/1fykdVkZObpgw678tmSGvwdESKewtrn\n\t4eUWj0NGg7XxFeAcCL+24m4GsIb1kpdt9E033N0PWFDbQbeeo3ZZgzoS8w2db7G3sH\n\t0Eko+XjariIyq+LvfjtLeXBEjuOJTNdy1XuHGEAlW3lOY5c3ga4f2tMkUVYmwg555T\n\tnIPVuhWu2Z0pg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659004834;\n\tbh=Xh0R4H8yNNChhUUDKRF6m9XIoZgdRcaQ7h2D6R1lJqk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IB571WoQxjdRL5EeowVIWGKfhL0cVeD5DmYeGB1JQUceLWxG/Scc4iylAS8LaUnWK\n\tUbA9/yx7aTEBmp6fS8oikmMACC4q8RD0T8MGPGfP496nVeR84XVkfVVxbPYdyH+RTw\n\t/kgKqGVJAdT6L5CVlApJkjbqTvrS7ttvI4DIXUNE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"IB571WoQ\"; dkim-atps=neutral","Date":"Thu, 28 Jul 2022 13:40:31 +0300","To":"Jacopo Mondi ","Message-ID":"","References":"<20220727222149.30627-1-laurent.pinchart@ideasonboard.com>\n\t<20220727222149.30627-4-laurent.pinchart@ideasonboard.com>\n\t<20220728073311.vherciswimeweuvm@uno.localdomain>\n\t\n\t<20220728103902.exwigio3b2qcttpg@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20220728103902.exwigio3b2qcttpg@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v5 3/9] libcamera: yaml_parser: Add\n\tgetList() function","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]