Created attachment 22779 [details] [review]
higgify revert dialog

The patch looks good.

I'm not sure removing "Do you want to " is a good idea. 

Please, ask to an english speaking guy on IRC and commit.

Thanks,
Paolo

According to various people on #gnome removing the "Do you want to" is
fine, since "Do you ..." leads to "Yes" or "No" response buttons,
while omitting it leads to "Revert" response.

Anyway since #usability was silent at the moment, so I'm cc'ing
usability for a final response.

You can omit the item if your users don't need it.

If you keep it, let the user have some measure of how much will change
by indicating when to file was last saved, how much time has passed,
or something like "%d lines will be lost and %d lines will be changed."
An alternative button could allow the user to see a diff instead.

Removing the limits from Undo/Redo, so that the user can either redo
all the changes or undo the reversion, would allow you to eliminate
this alert entirely.

(copying some of the comments I made on IRC)

I'm not sure about other peoples, but I never used/noticed the Revert
item myself: the few times I needed to restart from scratch with a
file I just closed the file and reopened it from the list of recently
opened docs.


If it stays:

- maybe the secondary message could become "Changes made to the file
will be lost" instead of "You will not be able to undo this operation"
(of course adding "... changes made in the last x minutes... would be
even better)

- providing a diff button: not sure... it's a nice idea but probably
it's overkill, beside the "diff" plugin is in a separate package
(gedit-plugins)

- undo: beside being (probably) hard to implement (you are reloading
the file from disk, not making changes to the current loaded doc), I
don't think it's really necessary; IMHO the user is really saying:
"Hey I really mean to throw this crap away and restart from scratch!".
Beside even if the action could be undoed, I'm not sure if the alert
dialog telling the user that he is reverting all the changes he made
could be removed.

I think it should stay.

About the other comments, I agree with Paolo B.

- I think the secondary message should become "Changes made to the
document in the last X minutes will be definitively lost"

- about the "Diff" button: I think it is overkill. Would be a normal
user able to read a diff file?

- undo: it cannot be easily implemented with the current undo manager
in a memory safe way

Giving a look at conglomerate code I saw the following function:

GtkDialog *cong_dialog_revert_confirmation_alert_new(GtkWindow *parent, 
						     const gchar *document_name,
						     glong seconds_since_last_save_or_load)
{
	GtkWidget *dialog, *content;
	gchar *primary_text, *secondary_text;
	glong minutes;

	g_return_val_if_fail(document_name, NULL);

	dialog = gtk_dialog_new_with_buttons(NULL, /* empty title string */
					     parent,
					     GTK_DIALOG_MODAL,

					     GTK_STOCK_CANCEL,
					     CONG_REVERT_CONFIRMATION_RESULT_CANCEL,

					     GTK_STOCK_REVERT_TO_SAVED,
					     CONG_REVERT_CONFIRMATION_RESULT_REVERT,

					     NULL);
	gtk_dialog_set_default_response(GTK_DIALOG(dialog),
					CONG_REVERT_CONFIRMATION_RESULT_REVERT);
					
	gtk_container_set_border_width(GTK_CONTAINER(dialog), 6);

	primary_text = g_strdup_printf(_("Revert unsaved changes to document
\"%s\"?"), document_name);

	minutes = seconds_since_last_save_or_load/60;

	if (minutes>120) {
		secondary_text = g_strdup_printf(_("Changes from the past %li hours
will be discarded."), (minutes/60));
	} else if (minutes>1) {
		secondary_text = g_strdup_printf(_("Changes from the past %li
minutes will be discarded."), minutes);
	} else {
		secondary_text = g_strdup_printf(_("Changes from the past minute
will be discarded."));
	}

	content = cong_alert_content_new(GTK_STOCK_DIALOG_WARNING,
					 primary_text, 
					 secondary_text, 
					 NULL);


	g_free(primary_text);
	g_free(secondary_text);

	gtk_container_add(GTK_CONTAINER(GTK_DIALOG(dialog)->vbox), content);
	gtk_widget_show_all(dialog);

	return GTK_DIALOG(dialog);
}

They use "Revert unsaved changes to document \"%s\"?" as primary text.
I like it. I still prefer the text I suggested in my previous comment
as secondary text.
I don't like very much the way they have implemented the function.
First of all ngettext should be used. Second, I'd prefer to manage
hours and seconds too.

If seconds < 60 use seconds only (i.e. in the last xx seconds)
If seconds (60, 90) use "in the last minute"
if seconds [90, 120] use "in the last two minutes"
if seconds (120, 3600) use "in the last xx minutes"
if seconds [3600, 7200) use "in the last hour and xx minutes"
if seconds >= 7200 use in the last xx hours

I have just added to gedit-document.[ch] the following function:

glong gedit_document_get_seconds_since_last_save_or_load
(GeditDocument *doc);

Adding the HIG keyword.

Here is an updated patch. As discussed on irc it only handles seconds
and minutes.

Some doubts I have about it:

+	if (seconds < 1)
+		seconds = 1;
+
paranoid? 



+		secondary_msg = g_strdup_printf (ngettext ("Changes made in the
last second will be definitively lost",
+							   "Changes made in the last %li seconds will be definitively
lost",
+	
Is it ok for ngettext/translators that the singular case doesn't use
%li seconds at all?



+		secondary_msg = g_strdup_printf (_("Changes made in the last %li
minutes will be definitively lost"),
+						 seconds/60 + 1);
+
Should use ngettext anyway even if it doesn't have a singular case? If
yes how?



Last note that I used (seconds/60 + 1) so that 1 min 26 secs is
reported as 2 minutes. Ok?

Created attachment 22948 [details] [review]
updated

(oops of course \n\n must be removed from primary_msg)

Fixed it in my local copy.

I have only a doubt:

Using the Revert command you can also Reload an unmodified file from
the disk. Think to this scenario:
1. cvs -z3 up -Pd (on terminal)
2. File->Revert (on gedit)

In this way you can reload the current document and eventually get a
newer version.
Are the messages "Revert unsaved changes to document \"%s\"?" and
"Changes made to the document in the last X minutes will be
definitively lost" correct in this case too? Also in the case the
document in gedit in "unmodified" but different from the on-disk file.

Fixed in CVS HEAD.