Conversation
mvo5
left a comment
mvo5
left a comment
There was a problem hiding this comment.
Thanks! This looks very nice, a few quick comments inline, I thnk the -o is the only critical one though.
| RGUserDialog userDialog; | ||
| userDialog.showErrors(); | ||
| exit(1); | ||
| GtkApplication *app = gtk_application_new( |
There was a problem hiding this comment.
AIUI with this we can remove the if (!gtk_init_check(&argc, &argv)) (from a few lines above) ?
There was a problem hiding this comment.
Sure. I just wanted to have as less intrusive change as possible.
There was a problem hiding this comment.
Actually removing it.
gtk/gsynaptic.cc
Outdated
| { "filter-file", 'f', G_OPTION_FLAG_NONE, G_OPTION_ARG_FILENAME, NULL, N_("Give an alternative filter file"), N_("<filter-file>") }, | ||
| { "title", 't', G_OPTION_FLAG_NONE, G_OPTION_ARG_STRING, NULL, N_("Give an alternative main window title (e.g. hostname with `uname -n`)"), N_("title") }, | ||
| { "initial-filter", 'i', G_OPTION_FLAG_NONE, G_OPTION_ARG_STRING, NULL, N_("Start with the initial filter with given name"), N_("<filter-name>") }, | ||
| { "option", 'o', G_OPTION_FLAG_NONE, G_OPTION_ARG_STRING, NULL, N_("Set an arbitrary configuration option, eg -o dir::cache=/tmp"), N_("<option>") }, |
There was a problem hiding this comment.
We probably need a G_OPTION_ARG_STRING_ARRAY or something here, I'm pretty sure the old code allowed -o foo=bar -o baz=quax etc and here I think we only support a single flag anymore.
| if (pipe (sigterm_unix_signal_pipe_fds) != 0) { | ||
| g_warning ("Could not setup pipe, errno=%d", errno); | ||
| return 1; | ||
| exit(1); |
There was a problem hiding this comment.
This feels slightly not-nice (to exit here hard instead of returning), but I guess its the only way(?) if applicationActivate is void :( oh well :)
There was a problem hiding this comment.
How about removing this code and just rely on Gtk?
|
|
||
| RPackageLister *packageLister = new RPackageLister(); | ||
| RGMainWindow *mainWindow = new RGMainWindow(packageLister, "main"); | ||
| RGMainWindow *mainWindow = new RGMainWindow(GTK_APPLICATION(app), packageLister, "main"); |
There was a problem hiding this comment.
This is pre-existing so feel free to ignore but I wonder if construcuting the packageLister and mainWindow should happen after the trick around putting an existing synaptic into the foreground (feel free to ignore)
gtk/gsynaptic.cc
Outdated
|
|
||
| if (g_variant_dict_lookup(options, "repositories", "b", &flag)) | ||
| _config->Set("Volatile::startInRepositories", flag ? "true" : "false"); | ||
| if (g_variant_dict_lookup(options, "filter-file", "s", &arg)) |
There was a problem hiding this comment.
We need to free arg here (and in the other places), no? AIUI we could use &s here to get the raw pointer which we then don't need to free (and _config->Set() clones it into a string.
gtk/gsynaptic.cc
Outdated
| g_signal_connect(app, "activate", | ||
| G_CALLBACK(applicationActivate), nullptr); | ||
|
|
||
| return g_application_run(G_APPLICATION(app), argc, argv); |
There was a problem hiding this comment.
(super nitpick) technically we should g_object_unref(app); after it was run, so something like:
int status = g_application_run(G_APPLICATION(app), argc, argv);
g_object_unref(app);
return status;but I guess it does not really matter much as we are exiting anyway.
andy128k
force-pushed
the
gtk-application
branch
from
3ee22c9 to
c987002
Compare
2 participants
No description provided.