Re: [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops

[Namhyung Kim <namhyung.kim@lge.com>]

Hi,

On Mon, 30 Apr 2012 09:31:28 +0300 (EEST), Pekka Enberg wrote:
> On Mon, 30 Apr 2012, Namhyung Kim wrote:
>> Define and use perf_gtk_eops to provide a GTK2 message
>> dialog for error reporting. To do that, we need global
>> main_window variable for tracking UI state.
>> 
>> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
>> diff --git a/tools/perf/ui/gtk/util.c b/tools/perf/ui/gtk/util.c
>> index a727fe394e91..0d0ed2ed3937 100644
>> --- a/tools/perf/ui/gtk/util.c
>> +++ b/tools/perf/ui/gtk/util.c
>> @@ -2,6 +2,53 @@
>>  #include "../../util/debug.h"
>>  #include "gtk.h"
>>  
>> +#include <stdio.h>
>> +#include <string.h>
>> +#include <stdlib.h>
>> +
>> +
>> +GtkWidget *main_window;
>> +
>> +static int message_dialog(const char *title, const char *format, va_list args)
>> +{
>> +	char *msg;
>> +	GtkWidget *dialog;
>> +	GtkMessageType message_type = GTK_MESSAGE_WARNING;
>> +
>> +	if (!main_window || vasprintf(&msg, format, args) < 0) {
>> +		fprintf(stderr, "%s:\n", title);
>> +		vfprintf(stderr, format, args);
>> +		return -1;
>> +	}
>> +
>> +	if (strcmp(title, "Error") == 0)
>> +		message_type = GTK_MESSAGE_ERROR;
>> +
>> +	dialog = gtk_message_dialog_new_with_markup(GTK_WINDOW(main_window),
>> +					GTK_DIALOG_DESTROY_WITH_PARENT,
>> +					message_type,
>> +					GTK_BUTTONS_CLOSE,
>> +					"<b>%s</b>\n\n%s", title, msg);
>> +	gtk_dialog_run(GTK_DIALOG(dialog));
>> +	gtk_widget_destroy(dialog);
>> +	free(msg);
>> +	return 0;
>> +}
>
> I think this is an usability glitch waiting to happen - especially so if 
> you use it for warnings. There's no reason to require the user to react to 
> warning messages in the GUI because there's absolutely nothing they can do 
> about them.
>
> I guess we could do something like the "ui helpline" thing used by the 
> newt front-end if we really wanted to.
>
> 			Pekka

I did quick grep on ui__warning and found that most of its users are
trying to show the messages before exiting. I think some (at least) of
them can be converted to ui__error(). And as existing implementation
(TUI) already requires user input for this, I thought it's ok.

But I agreed with you that ui__warning should not be used for showing
non-critical messages and converted to helpline-style ones.

Thanks,
Namhyung