After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 628855 - Bad strings in folder merge dialog
Bad strings in folder merge dialog
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: File and Folder Operations
2.31.x
Other All
: Normal normal
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-09-06 07:43 UTC by Luca Ferretti
Modified: 2010-09-11 22:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Untested patch suggesting a more i18n friendly approach (5.15 KB, patch)
2010-09-09 20:21 UTC, Wouter Bolsterlee (uws)
needs-work Details | Review
patch (5.24 KB, patch)
2010-09-10 16:36 UTC, Cosimo Cecchi
committed Details | Review

Description Luca Ferretti 2010-09-06 07:43:01 UTC
The way to build the secondary message for merge folder dialog[1] is
really really bad from a localization point of view.


/* Translators: the first string in this printf
 * is the "An older/A newer/Another" string defined some lines before.
 */
secondary_text = g_strdup_printf (
        _("%s folder with the same name already exists in \"%s\".\n"
          "Merging will ask for confirmation before "
          "replacing any files in the folder that "
          "conflict with the files being copied."),
        time_str,
        dest_dir_name);

Could we change it ASAP, providind 4 different translatable strings:

1.  "An older older with the same name already exists in \"%s\".\n"
2.  "An newer folder with the same name already exists in \"%s\".\n"
3.  "Another folder with the same name already exists in \"%s\".\n"
4.  "Merging will ask for confirmation before replacing any files in the
folder that conflict with the files being copied."

Same, of course, for the similar message related to files:

        secondary_text = g_strdup_printf (
                        _("%s file with the same name already exists in \"%s\".\n"
                          "Replacing it will overwrite its content."),
                        time_str,
                        dest_dir_name);

Plus, 

 * dialog title "File conflict"[1] -- it should be "File Conflict", HIG
 * mnemonics for buttons -- "Merge"[2] "Reset" [3] and "Replace"[4] should have a _
 * mnemonic for checkbox  -- "Apply this ..." [5]

About dialog title, isn't better something like "Confirm File
Replacement" ?

See also http://mail.gnome.org/archives/nautilus-list/2010-September/msg00000.html and http://mail.gnome.org/archives/gnome-i18n/2010-September/msg00044.html
Comment 1 André Klapper 2010-09-06 08:47:21 UTC
(Please set appropraite keywords - Thanks!)
Comment 2 Wouter Bolsterlee (uws) 2010-09-09 19:56:16 UTC
For completeness, I also include a follow-up mail by Luca (the remainder of this report is a quote):



If we are going to change those strings, could we please consider also:
 
 * dialog title "File conflict"[1] -- it should be "File Conflict", HIG
 * mnemonics for buttons -- "Merge"[2] "Reset" [3] and "Replace"[4]
should have a _
 * mnemonic for checkbox  -- "Apply this ..." [5]

About dialog title, isn't better something like "Confirm File
Replacement" ?

[1]
http://git.gnome.org/browse/nautilus/tree/libnautilus-private/nautilus-file-conflict-dialog.c?h=gnome-2-32#n629

[2]
http://git.gnome.org/browse/nautilus/tree/libnautilus-private/nautilus-file-conflict-dialog.c?h=gnome-2-32#n311

[3]
http://git.gnome.org/browse/nautilus/tree/libnautilus-private/nautilus-file-conflict-dialog.c?h=gnome-2-32#n553

[4] 
http://git.gnome.org/browse/nautilus/tree/libnautilus-private/nautilus-file-conflict-dialog.c?h=gnome-2-32#n521

[5]
http://git.gnome.org/browse/nautilus/tree/libnautilus-private/nautilus-file-conflict-dialog.c?h=gnome-2-32#n533
Comment 3 Wouter Bolsterlee (uws) 2010-09-09 20:01:59 UTC
Fwiw, the get_str_for_mtimes() function seems to be used only once, so while this approach is not correct i18n-wise, translating "a newer" and "an older" as if they were followed by the word "folder" will result in correct strings in many languages.

Yet, this should be solved properly.
Comment 4 Wouter Bolsterlee (uws) 2010-09-09 20:07:37 UTC
(In reply to comment #3)
> Fwiw, the get_str_for_mtimes() function seems to be used only once, so while
> this approach is not correct i18n-wise, translating "a newer" and "an older" as
> if they were followed by the word "folder" will result in correct strings in
> many languages.

I take that back. It is also used for with "file" after it, which makes it impossible to translate correctly in many languages.
Comment 5 Wouter Bolsterlee (uws) 2010-09-09 20:21:27 UTC
Created attachment 169893 [details] [review]
Untested patch suggesting a more i18n friendly approach
Comment 6 André Klapper 2010-09-09 20:37:45 UTC
"by the explanation about merging above.": I'm not sure that the order of the appearance of strings in a po file can be determined.
Comment 7 Wouter Bolsterlee (uws) 2010-09-09 20:54:20 UTC
In practice they do, unless the strings are used more than once in the application, which is not the case here.
Comment 8 Luca Ferretti 2010-09-09 21:36:01 UTC
Wouter, proposed patch seems i18n-friendly and good for me.

Speaking just for Italian language, please note that you have

 An older folder --> Una cartella più vecchia
 A newer folder  --> Una cartella più nuova

So, starting from 
   "An older"        "A newer"       "%s folder"
you could translate them as
   "più vecchia"     "più nuova"     "Una cartella %s"
and you'll have correct strings (while not correct translations, strictly speaking).

But including
    Another folder --> Un'altra cartella
breaks anything.

So, original code was really elegant, but i18n prefers copy and paste :D
Comment 9 Cosimo Cecchi 2010-09-09 23:54:28 UTC
Review of attachment 169893 [details] [review]:

Wouter, thanks for the patch!
It looks good except for these two minor comments below.

I need to ask gnome-i18n list for an approval request to merge this, right?

::: libnautilus-private/nautilus-file-conflict-dialog.c
@@ +103,3 @@
 	gboolean source_is_dir,	dest_is_dir, should_show_type;
 	NautilusFileConflictDialogDetails *details;
+	char *primary_text, *secondary_text, *secondary_text_extra;

secondary_text_extra should be const gchar *

@@ +148,3 @@
+				  "the folder that conflict with the files being copied.");
+
+			if (src_mtime > dest_mtime)

Nautilus' code style uses braces for if statements also when it's only one line.
Comment 10 Cosimo Cecchi 2010-09-09 23:54:29 UTC
Review of attachment 169893 [details] [review]:

Wouter, thanks for the patch!
It looks good except for these two minor comments below.

I need to ask gnome-i18n list for an approval request to merge this, right?

::: libnautilus-private/nautilus-file-conflict-dialog.c
@@ +103,3 @@
 	gboolean source_is_dir,	dest_is_dir, should_show_type;
 	NautilusFileConflictDialogDetails *details;
+	char *primary_text, *secondary_text, *secondary_text_extra;

secondary_text_extra should be const gchar *

@@ +148,3 @@
+				  "the folder that conflict with the files being copied.");
+
+			if (src_mtime > dest_mtime)

Nautilus' code style uses braces for if statements also when it's only one line.
Comment 11 Cosimo Cecchi 2010-09-09 23:54:29 UTC
Review of attachment 169893 [details] [review]:

Wouter, thanks for the patch!
It looks good except for these two minor comments below.

I need to ask gnome-i18n list for an approval request to merge this, right?

::: libnautilus-private/nautilus-file-conflict-dialog.c
@@ +103,3 @@
 	gboolean source_is_dir,	dest_is_dir, should_show_type;
 	NautilusFileConflictDialogDetails *details;
+	char *primary_text, *secondary_text, *secondary_text_extra;

secondary_text_extra should be const gchar *

@@ +148,3 @@
+				  "the folder that conflict with the files being copied.");
+
+			if (src_mtime > dest_mtime)

Nautilus' code style uses braces for if statements also when it's only one line.
Comment 12 Wouter Bolsterlee (uws) 2010-09-10 16:01:59 UTC
Cosimo, thanks for the feedback. I hardly speak any C though, so my completely untested patch was merely intended as a suggestion. (I also stated this in the patch description.) Happy to see that it's almost ready as-is. :)

That said, I currently don't have any Gnome development environment set up at all, so it would take me many hours to actually test my changes, given that I need to compile half of Gnome from scratch in that case.

If you could take it from here, that would be really appreciated. Please let me know if I can be of any help.
Comment 13 Wouter Bolsterlee (uws) 2010-09-10 16:06:00 UTC
(In reply to comment #8)
>  An older folder --> Una cartella più vecchia
>  A newer folder  --> Una cartella più nuova

In Dutch these strings are impossible to translate, since "a newer folder" would become "een nieuwere map", and "a newer file" would become "een nieuwer bestand", i.e. "nieuwere" versus "nieuwer" depending on the noun it refers to.
Comment 14 Wouter Bolsterlee (uws) 2010-09-10 16:08:16 UTC
(In reply to comment #9)
> I need to ask gnome-i18n list for an approval request to merge this, right?

This issue has already come up on the gnome-i18n list:
http://mail.gnome.org/archives/gnome-i18n/2010-September/msg00044.html

As a coordination team member, I hereby give you my +1 for fixing this.
Comment 15 Cosimo Cecchi 2010-09-10 16:36:19 UTC
Created attachment 169972 [details] [review]
patch

This is an updated patch.
I also made it a bit smarter than yours, so that things as '\n%s' do not get included in the translatable strings.
I also formally asked i18n for a string freeze break for this patch.
Comment 16 Wouter Bolsterlee (uws) 2010-09-10 16:47:43 UTC
Thanks, Cosimo!

> +	secondary_text = g_strdup_printf ("%s\n%s", message, message_extra);

I intentionally added two newlines, i.e. "%s\n\n%s", because the result would look ugly otherwise (line break at a strange place, depending on the language).
Comment 17 Cosimo Cecchi 2010-09-11 12:58:30 UTC
(In reply to comment #16)

> I intentionally added two newlines, i.e. "%s\n\n%s", because the result would
> look ugly otherwise (line break at a strange place, depending on the language).

Hmm, that could be included in the translation for the languages for which it's problematic, no?
I committed my patch to master and gnome-2-32. Closing this as FIXED.
Comment 18 Wouter Bolsterlee (uws) 2010-09-11 22:39:11 UTC
It's either a) no newlines at all two create a single paragraph, or b) two newlines to create a new paragraph. Right now there is a newline "somewhere randoml in the middle", which doesn't make sense in any language.

Apart from that, thanks for looking into this.