| Message ID | 20210906081908.3083134-1-paul.elder@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi, On Mon, Sep 06, 2021 at 05:19:08PM +0900, Paul Elder wrote: > The indentation of the deserialization call on the proxy worker side > inside the case statement was one level too shallow. Fix it. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > .../libcamera_templates/module_ipa_proxy_worker.cpp.tmpl | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl > index c54ecdb9..c306d8df 100644 > --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl > +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl > @@ -79,7 +79,7 @@ public: > > {% for method in interface_main.methods %} > case {{cmd_enum_name}}::{{method.mojom_name|cap}}: { > - {{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(8, true)}} > + {{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(16, true)}} Note that this will mix spaces with tabs. But we do so anyway elsewhere in the template code. jinja's built-in indent filter (that we use here and elsewhere) in 2.x uses spaces only, though in 3.x apparently you can specify the string to indent with. iirc we support both? Paul > {% for param in method|method_param_outputs %} > {{param|name}} {{param.mojom_name}}; > {% endfor %} > -- > 2.27.0 >
On 06/09/2021 09:24, paul.elder@ideasonboard.com wrote: > Hi, > > On Mon, Sep 06, 2021 at 05:19:08PM +0900, Paul Elder wrote: >> The indentation of the deserialization call on the proxy worker side >> inside the case statement was one level too shallow. Fix it. >> >> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> >> --- >> .../libcamera_templates/module_ipa_proxy_worker.cpp.tmpl | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl >> index c54ecdb9..c306d8df 100644 >> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl >> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl >> @@ -79,7 +79,7 @@ public: >> >> {% for method in interface_main.methods %} >> case {{cmd_enum_name}}::{{method.mojom_name|cap}}: { >> - {{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(8, true)}} >> + {{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(16, true)}} > > Note that this will mix spaces with tabs. But we do so anyway elsewhere > in the template code. jinja's built-in indent filter (that we use here > and elsewhere) in 2.x uses spaces only, though in 3.x apparently you can > specify the string to indent with. iirc we support both? I was going to ask if the indent could be specified in quantified 'levels' rather than spaces ... I've seen the mix of spaces and tabs ... it's autogenerated, so I don't think we need to dwell too long over perfection, but any little we can do to help readability is beneficial in these templates. Certainly reading the output is much easier than reading the templates. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Paul > >> {% for param in method|method_param_outputs %} >> {{param|name}} {{param.mojom_name}}; >> {% endfor %} >> -- >> 2.27.0 >>
On Mon, Sep 06, 2021 at 11:46:34AM +0100, Kieran Bingham wrote: > On 06/09/2021 09:24, paul.elder@ideasonboard.com wrote: > > On Mon, Sep 06, 2021 at 05:19:08PM +0900, Paul Elder wrote: > >> The indentation of the deserialization call on the proxy worker side > >> inside the case statement was one level too shallow. Fix it. > >> > >> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > >> --- > >> .../libcamera_templates/module_ipa_proxy_worker.cpp.tmpl | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl > >> index c54ecdb9..c306d8df 100644 > >> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl > >> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl > >> @@ -79,7 +79,7 @@ public: > >> > >> {% for method in interface_main.methods %} > >> case {{cmd_enum_name}}::{{method.mojom_name|cap}}: { > >> - {{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(8, true)}} > >> + {{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(16, true)}} > > > > Note that this will mix spaces with tabs. But we do so anyway elsewhere > > in the template code. jinja's built-in indent filter (that we use here > > and elsewhere) in 2.x uses spaces only, though in 3.x apparently you can > > specify the string to indent with. iirc we support both? > > I was going to ask if the indent could be specified in quantified > 'levels' rather than spaces ... > > I've seen the mix of spaces and tabs ... it's autogenerated, so I don't > think we need to dwell too long over perfection, but any little we can > do to help readability is beneficial in these templates. > > Certainly reading the output is much easier than reading the templates. > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> My OCD says tabs would be nice, the rest of me says it doesn't matter here :-) Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> {% for param in method|method_param_outputs %} > >> {{param|name}} {{param.mojom_name}}; > >> {% endfor %}
diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl index c54ecdb9..c306d8df 100644 --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl @@ -79,7 +79,7 @@ public: {% for method in interface_main.methods %} case {{cmd_enum_name}}::{{method.mojom_name|cap}}: { - {{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(8, true)}} + {{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(16, true)}} {% for param in method|method_param_outputs %} {{param|name}} {{param.mojom_name}}; {% endfor %}
The indentation of the deserialization call on the proxy worker side inside the case statement was one level too shallow. Fix it. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- .../libcamera_templates/module_ipa_proxy_worker.cpp.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)