On Thu, 2017-10-26 at 22:28 +0200, Colin Leroy wrote:
> From 2120613fa5bd68ebbdd0e9428357030736fce013 Mon Sep 17 00:00:00 2001
> From: Colin Leroy <colin at colino.net>
> Date: Thu, 26 Oct 2017 22:20:36 +0200
> Subject: [PATCH] Implement single-launch with Gtk::Application
>
> This introduces a new file for clarity. Options
> handling changes so that --tab changes the tab
> if the window is already opened. Other options
> are only used at start time.
The division of responsibilities isn't always very clean, but I don't
have ideas for simple fixes for that. I think PavuApplication should
create the UI and an object for managing the libpulse stuff, and the
libpulse stuff shouldn't directly depend on the UI parts. Currently the
libpulse stuff is mostly handled in pavucontrol.cc, but that is very
much dependent on the UI stuff, so fixing this would be a big project.
> ---
> po/POTFILES.in | 1 +
> src/Makefile.am | 1 +
> src/mainwindow.cc | 18 ++++++
> src/mainwindow.h | 2 +
> src/pavuapplication.cc | 151 +++++++++++++++++++++++++++++++++++++++++++++++++
> src/pavuapplication.h | 51 +++++++++++++++++
> src/pavucontrol.cc | 98 ++++----------------------------
> src/pavucontrol.h | 2 +
> 8 files changed, 237 insertions(+), 87 deletions(-)
> create mode 100644 src/pavuapplication.cc
> create mode 100644 src/pavuapplication.h
>
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index c952a83..e9d0f55 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -12,3 +12,4 @@ src/sinkwidget.cc
> src/sourceoutputwidget.cc
> src/sourcewidget.cc
> src/streamwidget.cc
> +src/pavuapplication.cc
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 7257260..b5c0314 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -37,6 +37,7 @@ pavucontrol_SOURCES= \
> rolewidget.h rolewidget.cc \
> mainwindow.h mainwindow.cc \
> pavucontrol.h pavucontrol.cc \
> + pavuapplication.cc pavuapplication.h \
> i18n.h
>
> pavucontrol_LDADD=$(AM_LDADD) $(GUILIBS_LIBS) $(PULSE_LIBS)
> diff --git a/src/mainwindow.cc b/src/mainwindow.cc
> index 31d5695..b73108f 100644
> --- a/src/mainwindow.cc
> +++ b/src/mainwindow.cc
> @@ -340,6 +340,24 @@ static void updatePorts(DeviceWidget *w, std::map<Glib::ustring, PortInfo> &port
> }
> }
>
> +void MainWindow::selectBestTab(int default_tab) {
> + if (default_tab != -1) {
> + if (default_tab < 1 || default_tab > notebook->get_n_pages()) {
> + if (sinkInputWidgets.size() > 0)
> + notebook->set_current_page(0);
> + else if (sourceOutputWidgets.size() > 0)
> + notebook->set_current_page(1);
> + else if (sourceWidgets.size() > 0 && sinkWidgets.size() == 0)
> + notebook->set_current_page(3);
> + else
> + notebook->set_current_page(2);
> + } else {
> + notebook->set_current_page(default_tab - 1);
> + }
> + default_tab = -1;
> + }
> +}
This is a weird function. The name implies that it will select the best
tab on some heuristics, but it only does that if default_tab is 0. If
default_tab is -1, then the function does absolutely nothing. The
"default_tab = -1" assignment at the end doesn't make any sense.
I think it would be better if this function was only called when the
caller really wants to apply the heuristics.
> +
> void MainWindow::updateCard(const pa_card_info &info) {
> CardWidget *w;
> bool is_new = false;
> diff --git a/src/mainwindow.h b/src/mainwindow.h
> index 30e1ad0..a163069 100644
> --- a/src/mainwindow.h
> +++ b/src/mainwindow.h
> @@ -60,6 +60,8 @@ public:
> void removeSourceOutput(uint32_t index);
> void removeClient(uint32_t index);
>
> + void selectBestTab(int default_tab);
> +
> void removeAllWidgets();
>
> void setConnectingMessage(const char *string = NULL);
> diff --git a/src/pavuapplication.cc b/src/pavuapplication.cc
> new file mode 100644
> index 0000000..18f8d0b
> --- /dev/null
> +++ b/src/pavuapplication.cc
> @@ -0,0 +1,151 @@
> +/***
> + This file is part of pavucontrol.
> +
> + Copyright 2006-2008 Lennart Poettering
> + Copyright 2008 Sjoerd Simons <sjoerd at luon.net>
> +
> + pavucontrol is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation, either version 2 of the License, or
> + (at your option) any later version.
> +
> + pavucontrol is distributed in the hope that it will be useful, but
> + WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with pavucontrol. If not, see <http://www.gnu.org/licenses/>.
> +***/
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include "i18n.h"
> +
> +#include <canberra-gtk.h>
> +
> +#include "pavuapplication.h"
> +#include "pavucontrol.h"
> +#include "mainwindow.h"
> +
> +PavuApplication::PavuApplication() : Gtk::Application("org.pulseaudio.pavucontrol", Gio::ApplicationFlags::APPLICATION_HANDLES_COMMAND_LINE) {
> +}
> +
> +Glib::RefPtr<PavuApplication> PavuApplication::create() {
> + return Glib::RefPtr<PavuApplication>(new PavuApplication());
> +
> +}
Why does the PavuApplication need to be wrapped in a RefPtr?
> +
> +Gtk::Window* PavuApplication::create_window()
> +{
> + m = pa_glib_mainloop_new(g_main_context_default());
> + g_assert(m);
> +
> + Gtk::Window* pavucontrol_window = pavucontrol_get_window(m, maximize, retry, tab);
> + add_window(*pavucontrol_window);
> +
> + pavucontrol_window->signal_hide().connect(
> + sigc::bind<Gtk::Window*>(sigc::mem_fun(*this,
> + &PavuApplication::on_hide_window), pavucontrol_window));
> +
> + return pavucontrol_window;
> +}
> +
> +void PavuApplication::on_activate()
> +{
> + Gtk::Window* pavucontrol_window = NULL;
> +
> + auto windows = get_windows();
> +
> + if (windows.size() > 0) {
> + pavucontrol_window = dynamic_cast<Gtk::Window*>(windows[0]);
> + }
This seems a bit ugly. Couldn't the main window be a member variable of
PavuApplication?
> +
> + if (pavucontrol_window == NULL) {
> + pavucontrol_window = create_window();
> + } else if (tab != -1) {
> + ((MainWindow *)pavucontrol_window)->selectBestTab(tab);
> + }
> +
> + pavucontrol_window->present();
> +}
This is probably part of the "switch tab if already running" logic, but
I don't understand how that works in practice. If there's already one
instance running, how does the tab index get passed from the new
process to the old one? I'm sure I could figure it out from the GLib
documentation, but there's apparently a lot of magic happening behind
the scenes, and it would be good to have a comment that explains how
the magic works.
--
Tanu
https://www.patreon.com/tanuk
