I think message context ( C_() ) would be even better.

Created attachment 238571 [details] [review]
imap-smtp: Add some context for "None"

Good catch!

So you have a combobox like this:
"_Encryption": "None", "STARTTLS after connecting", "SSL on a dedicated port".

There is a similar combo box in Evolution, but with "Encryption method" and "No encryption" for the first two strings. It would be cool to have similar translations in both modules. I don't know what is the best way to send that point across.

Thanks for fixing this up.

Created attachment 238682 [details] [review]
imap-smtp: Add translation context for "None", and some comments too

Added a comment mentioning the similarity with the strings in evolution.

(In reply to comment #3)
> Thanks for fixing this up.

Since we are in string freeze, would you be willing to give one of the two approvals needed to get this in?

Well. Since this is a single string can be potentially problematic for many I do think that it should get a string freeze exception. However, I think that those approvals need to come from members of some official i18n team (the structure has always confused me a bit), so I don't think I can.

Sorry.

Review of attachment 238682 [details] [review]:

::: src/goabackend/goaimapsmtpprovider.c
@@ +736,3 @@
       gtk_combo_box_text_append (GTK_COMBO_BOX_TEXT (data->smtp_encryption),
                                  "none",
+                                 C_("Encryption", "None"));

Using the same context for the same string is absolutely pointless.

Contexts are used to differentiate between the different meanings of the same string.
If "None" here has a different meaning than in the other place [BTW, this isn't true either, so no need for contexts], normally you would end up with only one translatable string in the po files as you can see in the original report text. 

Now if you want two strings for the two different meanings, you would use contexts to differentiate between the identical strings. That makes two different entries appear in the translation file, which would have the same msgid, but different msgctxt fields. This way localizers can use different translations for the different meanings. See also: https://developer.gnome.org/glib/stable/glib-I18N.html#C-:CAPS and http://www.gnu.org/software/gettext/manual/gettext.html#Contexts

The content of the msgctxt field is going to be the first parameter of the C_() macro. Those HAVE to be unique, unless you want to end up with something like this:

#: ../src/goabackend/goaimapsmtpprovider.c:700
#: ../src/goabackend/goaimapsmtpprovider.c:738
msgctxt "Encryption"
msgid "None"

(actual excerpt from g-o-a.pot after applying this patch)

The comments can stay though, that is helpful and needs no i18n approval.

Looks good to me.

(In reply to comment #7)
> Review of attachment 238682 [details] [review]:
> 
> ::: src/goabackend/goaimapsmtpprovider.c
> @@ +736,3 @@
>        gtk_combo_box_text_append (GTK_COMBO_BOX_TEXT (data->smtp_encryption),
>                                   "none",
> +                                 C_("Encryption", "None"));
> 
> Using the same context for the same string is absolutely pointless.

Yes, but I used it to guard against a future use of "None" in a different context.

> #: ../src/goabackend/goaimapsmtpprovider.c:700
> #: ../src/goabackend/goaimapsmtpprovider.c:738
> msgctxt "Encryption"
> msgid "None"
> 
> (actual excerpt from g-o-a.pot after applying this patch)

Yes, the string "None" is used in the same context in both those places. In fact I have a subsequent patch that refactors the code so that it shows up only in one place.

So what do you suggest? Should I remove the context or keep it?

(In reply to comment #9)
> Yes, the string "None" is used in the same context in both those places. In
> fact I have a subsequent patch that refactors the code so that it shows up only
> in one place.
> 
> So what do you suggest? Should I remove the context or keep it?

Looks like you don't need to specify the context. I think adding a comment pointing out that the string can be found in evo should be ok here.

Created attachment 238693 [details] [review]
imap-smtp: Refactor the encryption combo-box code

I removed the context and left the comments.