Thunar and xfce4-terminal both ship a dialog which askes for confirmation to close the window if multiple tabs are open. In order to have the same look & feel for both, the code from xfce4-terminal was reused in thunar (See https://git.xfce.org/xfce/thunar/commit?id=33f6570e3c828f5867244bed3a3f6d9403feaa9d) It would be nice to have the code only once, in libxfce4ui, so that further changes on the dialog will always have effect on both applications.
Created attachment 9065 Patch to add confirmation dialog for use when closing multiple tabs Here is my go at implementing this. The patch adds a single function, xfce_dialog_confirm_close_tabs, which takes 3 arguments: the parent window, the number of open tabs (this is displayed to the user; if the argument is negative then the message just says "multiple open tabs"), and a pointer to a boolean which is used to set/get the user's preference for seeing the confirmation dialog (if this is NULL then the user is not given this option). See my next comment for a small demonstrator program. Criticisms and suggestions for improvement are most welcome! Would it be a good idea to open bugs for thunar and xfce4-terminal pointing to this bug as a way of inviting comments from the people whose programs would be using this feature? I'm rather new at bugzilla etiquette so I'm not sure if this is sensible.
Created attachment 9066 demonstration program for above patch
Thanks for the patch ! Here the command I used to build the demo, after the modified libxfce4ui got installed: gcc `pkg-config --cflags gtk+-3.0` -o demo -I/usr/local/include/xfce4 -I/usr/local/include/xfce4/libxfce4ui-2 demo.c -L/usr/local/lib -lxfce4ui-2 `pkg-config --libs gtk+-3.0` Some remarks: Works fine for me ! Just a minor thing: I would use a additional, separate argument "dont_ask_again_box_checked" for the return value, and rename "do_confirm" to "show_dont_ask_again_box", to be more clear. I added Igor to the CC List, to have a look on it. Let's first ask if he is ok to have the dialog in libxfce4ui and if so, push it. Than we could open bugs on thunar/xfce4-terminal to actually make use of the dialog. Before using the dialog in thunar/terminal, I guess we need to wait for a libxfce4ui dev release, since we would need to update our minimal xfce4ui version in configure.ac.in.
Thanks for including me! A few comments on the patch. The function description is saying: + * xfce_dialog_confirm_close_tabs: + * @parent : (allow-none): transient parent of the dialog, or %NULL. However, the very first check suggests that the function will return immediately if parent==NULL: + g_return_val_if_fail (parent == NULL || GTK_IS_WINDOW (parent), 0); +gint +xfce_dialog_confirm_close_tabs (GtkWindow *parent, + const gint num_tabs, + gboolean* const do_confirm) The "const gint num_tabs" declaration is redundant since num_tabs is passed as value anyway (i.e. you cannot modify the variable value from within the function). Both primary_text and warning_icon can be declared as "const gchar *" since you don't have to dynamically allocate memory for them.
Thanks for the comments, Igor and alexxcons! I will implement your suggestions in the next version of the patch. Igor, with the description/handling of the "parent parameter", I just copied what the existing function xfce_dialog_confirm does. Do you think I should remove the description of "parent" as allowing a null value?
(In reply to Reuben Green from comment #5) > Thanks for the comments, Igor and alexxcons! I will implement your > suggestions in the next version of the patch. > > Igor, with the description/handling of the "parent parameter", I just copied > what the existing function xfce_dialog_confirm does. Do you think I should > remove the description of "parent" as allowing a null value? Yes, I think "allow-none" and "or %NULL" should be removed from the description. Thanks!
Igor, I've just had another look at the code, and in fact the function in its current form does not return immediately if parent is NULL. The check g_return_val_if_fail (parent == NULL || GTK_IS_WINDOW (parent), 0); returns 0 if parent fails to be either NULL or a valid GtkWindow, so NULL is allowed. Indeed, NULL indicates the lack of a parent window, and this is used in the demonstrator program. Sorry, I wasn't concentrating when I replied before! So I suppose the question is, should the dialog be allowed to have no parent window? Since the dialog is about tabs in a main window, it probably won't normally be used without a parent, but maybe it's a good idea to give people the choice in case they want to organise things differently in their program? I'm not sure, what you think? Thanks!
Hi Ruben, my bad, I overlooked that the check actually looks for a failed condition. :( Since xfce_message_dialog_new() does allow NULL parent, I think your function should allow it, too. Another question is what value should be returned in the failure scenario: g_return_val_if_fail (parent == NULL || GTK_IS_WINDOW (parent), 0); Currently, 0 is returned; however, GtkResponseType enum doesn't define any response as 0 - "GTK+ leaves values of 0 or greater for application-defined response ids." (see https://developer.gnome.org/gtk3/stable/GtkDialog.html#GtkResponseType)
Hi Igor, no worries! :) I got the behaviour of returning 0 when a failure is detected from the existing function xfce_message_dialog, which does the same check and also returns a response id. Looking at the possible values in the GtkResponseType enum, there's no value predefined to represent an error. I suppose if we need to use a response from that list, GTK_RESPONSE_NONE or GTK_RESPONSE_CANCEL might be the most appropriate?
I'm leaning towards GTK_RESPONSE_NONE, to differentiate it from the case when the user actually clicked "Cancel" (GTK_RESPONSE_CANCEL).
Created attachment 9106 Patch to add confirmation dialog for use when closing multiple tabs Thanks for the comments. Here is a revised patch.
Created attachment 9107 demonstration program for above patch and here is a revised demonstrator program for the revised patch
The patch looks good to me, thanks!
Nice ! Just one minor thing: (sorry, I missed that on the first patch ) > +#if GTK_CHECK_VERSION (3, 0, 0) Afaik no ifdef is needed. Should be ok to assume that gtk 3.18.0 is used (Minimal requirement defined in configure.ac.in) Igor, possibly you have push access to xfce4ui ? (If not, we will have to ask on #xfce-dev )
(In reply to alexxcons from comment #14) > Nice ! Just one minor thing: (sorry, I missed that on the first patch ) > > > +#if GTK_CHECK_VERSION (3, 0, 0) > Afaik no ifdef is needed. Should be ok to assume that gtk 3.18.0 is used > (Minimal requirement defined in configure.ac.in) Are we still building libxfce4ui for both gtk2 and gtk3? If so, the #if is needed. Reuben, have you checked that your change builds fine with gtk2? > Igor, possibly you have push access to xfce4ui ? (If not, we will have to > ask on #xfce-dev ) No, I don't think I have push access to the repo.
I think libxfce4ui still supports gtk2, as there is an --enable-gtk2 option in the libxfce4ui configure script. I forgot to check it builds with this option on, but I have now checked and it builds fine, and the demo.c program still compiles and runs fine when compiled against the gtk2 version of the patched libxfce4ui. Thanks for the advice, I'm still fairly inexperienced at dealing with these kinds of issues!
> I think libxfce4ui still supports gtk2, as there is an --enable-gtk2 option in the libxfce4ui configure script Ops, sorry, I completely overlooked that one :F I just asked for push & release permission on #xfce-dev
Reuben Green referenced this bugreport in commit 68994e2d4e0e27ab853527d8e3e978eaab224ffa Add a dialog to confirm closure of multiple tabs (bug #15873) https://git.xfce.org/xfce/libxfce4ui/commit?id=68994e2d4e0e27ab853527d8e3e978eaab224ffa
Ok, pushed to master. Thanks alot for the patch Reuben ! ochosi stated vie IRC we should wait a bit (and drop gtk2 support :F ) before doing a dev release of xfce4ui: > <ochosi> alexxcons: i would suggest two things before doing a 4.15 dev release: drop gtk2 first (so that there are > no misunderstandings later) and wait patiently for the dev phase to start :) https://wiki.xfce.org/releng/4.16/roadmap > <ochosi> also, there are a few other things that could go into the first dev release, like the improved about dialog (which is ready and > usable, even if maybe not final), so a little coordination wouldn't hurt In any case, if you like, it will not hurt to already foresee patches for thunar and xfce4terminal In case one of you are interested in dropping gtk2 from xfce4ui, please let me know (or tell so on #xfce-dev) .. just to prevent that the work is done twice.
Thanks Alex! So to drop gtk2 support, is it mostly a case of removing #ifdef etc blocks that provide gtk2 code? Eg replacing blocks of the form #if GTK_CHECK_VERSION (3, 0, 0) XXX #else YYY #endif with just XXX. If so I could have a go at that. Of course there would also need to be changes in configure.in.ac and related files, but I'd probably need help with that since anything autotools-related is incomprehensible witchcraft as far as I'm concerned :).
That would be great ! Yes, I guess it would mostly be dropping of the #if /#else / #endifs and the lines in configure.ac.in you mentioned earlier. I dont think more autotools magic is involved. Lets just drop a link to a patch into #xfce-dev when finished, so somebody with better autohell knowledge can check. Do you idle in #xfce-dev ? Or should I drop a comment there, so they know that you are working on it ?
Hi Alex! OK, I'll have a look through the source code and see if it looks like something I can manage. If I think I can, I'll comment in #xfce-dev and open a bug "drop gtk2 support" in libxfce4ui saying I'm planning on doing this, and then assign it to myself and start work if no-one objects after a few days. Thanks!
ochosi and Skunnky did it, libxfce4ui 4.15.0 was just released \o/ So finally we can now start to use the new dialog. I opened two bugs for that purpose: Bug #16254 Bug #16255
Hi Alex, that's great news. I'll get working on a patch to use this in thunar to close bug #16254. Sorry I didn't manage to do the dropping of gtk2 from libxfce4ui like I said - I had plans to get to doing it but then my first kid unexpectedly arrived a few weeks early (unfortunately gdb doesn't yet have support for babies, so it takes rather longer to figure out what to do when he starts complaining!)
Best wishes from me ! I learned that the very small ones you best debug by checking if the system resources are fine (hungry?, poo in pants?, sleep needed? ) If they grew older, it will get more complicated. Debugging is hard, though luckily over time they will start to give more better feedback ;)