<div id="geary-body" dir="auto"><div>Hi all,</div></div><div id="geary-quote" dir="auto">On Thu, Apr 30, 2020 at 18:18, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:<br><blockquote type="cite"><div class="plaintext" style="white-space: pre-wrap;">On Thu, Apr 30, 2020 at 04:05:56PM +0100, Kieran Bingham wrote:
<blockquote> On 30/04/2020 15:15, Umang Jain wrote:
> This commit introduces no functional changes.
> This is required so that the combo-box list can be managed
> conveniently from various private functions in subsequent
> commit.
>
> Signed-off-by: Umang Jain <<a href="mailto:email@uajain.com">email@uajain.com</a>>
> ---
> src/qcam/main_window.cpp | 9 ++++-----
> src/qcam/main_window.h | 2 ++
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index d021fa9..344b7ec 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -11,7 +11,6 @@
> #include <string>
> #include <sys/mman.h>
>
> -#include <QComboBox>
Shouldn't we keep this here ... and...
> #include <QCoreApplication>
> #include <QFileDialog>
> #include <QImage>
> @@ -114,14 +113,14 @@ int MainWindow::createToolbars()
> connect(action, &QAction::triggered, this, &MainWindow::quit);
>
> /* Camera selector. */
> - QComboBox *cameraCombo = new QComboBox();
> - connect(cameraCombo, QOverload<int>::of(&QComboBox::activated),
> + cameraCombo_ = new QComboBox();
> + connect(cameraCombo_, QOverload<int>::of(&QComboBox::activated),
> this, &MainWindow::switchCamera);
>
> for (const std::shared_ptr<Camera> &cam : cm_->cameras())
> - cameraCombo->addItem(QString::fromStdString(cam->name()));
> + cameraCombo_->addItem(QString::fromStdString(cam->name()));
>
> - toolbar_->addWidget(cameraCombo);
> + toolbar_->addWidget(cameraCombo_);
>
> toolbar_->addSeparator();
>
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 5d6251c..28c325f 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -9,6 +9,7 @@
>
> #include <memory>
>
> +#include <QComboBox>
instead just forward declare
class QComboBox;
But as this hasn't been done for other parts of this component, I don't
think it makes much difference.
Either way...
Reviewed-by: Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>>
</blockquote>
I was going to mention the same. We already forward-declare QAction that
way. QToolBar should do the same, and so should QComboBox here. Apart
from that,
Reviewed-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>></div></blockquote><span style="white-space: pre-wrap;"><div><span style="white-space: pre-wrap;"><br></span></div>Well, the QToolBar is already forward-declared via #include <QMainWindow></span><div><span style="white-space: pre-wrap;">header, so it's not should not be necessary to foward-declare it here.</span></div><div><br></div><div><span style="white-space: pre-wrap;">Thanks for the comments. I will submit v2 of the patches in a few minutes.</span></div><div><blockquote type="cite"><div class="plaintext" style="white-space: pre-wrap;">
<blockquote> > #include <QElapsedTimer>
> #include <QIcon>
> #include <QMainWindow>
> @@ -72,6 +73,7 @@ private:
> /* UI elements */
> QToolBar *toolbar_;
> QAction *startStopAction_;
> + QComboBox *cameraCombo_;
> ViewFinder *viewfinder_;
>
> QIcon iconPlay_;
</blockquote>
<div>--
</div>Regards,
Laurent Pinchart
</div></blockquote></div></div><img src="https://u15657259.ct.sendgrid.net/wf/open?upn=GCEip0g28ftA9O9fsCR2M7x08El53O4YVYtHuSI-2FrLwtytoSlmO-2FnSq-2B0q807R-2FE4lsF14i-2FFr8b0GYTbZ-2Fx0ron7uoi3m14JFHobCybhMIyFZcnYk6P2LIJqqDFdwlkOBrCoph8qasWnPkpPcaVqi2ZF6UDskQVxPND01yVVzFyXSfAThtYpPERmOP9CHqzy9deYWC08ZETxPf5q37M3c3Tm6L0Xu222EzCc2eb9C4wxt-2BjHtc5a1e0cSnukfFP" alt="" width="1" height="1" border="0" style="height:1px !important;width:1px !important;border-width:0 !important;margin-top:0 !important;margin-bottom:0 !important;margin-right:0 !important;margin-left:0 !important;padding-top:0 !important;padding-bottom:0 !important;padding-right:0 !important;padding-left:0 !important;"/>