Created attachment 167296 [details] [review]
Use g_str_has_suffix instead of strcmp to compare extensions

Created attachment 167297 [details] [review]
Minor changes

 * Sort alphabetically constants and Makefile.am
 * Added a trailing space to the constants when needed
 * initialize have_sk before
 * fixed typo in tests
 * removed useless 'else' for default values
 * update .gitignore

Created attachment 167298 [details] [review]
Make error strings translatable

These error strings are shown in dialogs

Created attachment 167299 [details] [review]
get_args now returns nitems parameter and fixed memleaks

This patch avoids calling many times g_strv_length()

The first patch (Use g_str_has_suffix instead of strcmp to compare extensions) does not appear to be a patch; it looks like a VIM swapfile or something.  Could you re-attach with the actual patch?

The first patch (Use g_str_has_suffix instead of strcmp to compare extensions) does not appear to be a patch; it looks like a VIM swapfile or something.  Could you re-attach with the actual patch?

I've applied "Minor changes" except for the bits that remove the default args from openvpn.  The default args help protect against if/when openvpn changes defaults, and sometimes programs get their configuration information from other sources as well, and this helps ensure that openvpn only gets the configuration we want it to get.

2f7540f3d482e4fc53f5cdee2b23629eec97aca5

In the error string translation patch, could you update it to use g_set_error_literal() where you've removed the "%s" arguments?  Certain compiler options will complain if we pass a constant string to varags function in place of the format argument.  So, where the code was this:

g_set_error (error, X, X, "%s", "foobar");

instead it should be this:

g_set_error_literal (error, X, X, "foobar");


thanks!

Created attachment 167503 [details] [review]
Use g_str_has_suffix instead of strcmp to compare extensions (good)

Created attachment 167507 [details] [review]
Make error strings translatable (fixed _literal)

Here they are.

Should I commit them now?

What about the g_strv_length one?

I fixed up the g_strv_length patch for current git and did a few formatting cleanups.  Pushed as:

0cfd6b966c80de0e689badedf9d6ad50dd355e4f

The g_str_has_prefix() patch looks good, though there's some odd formatting of the if() in tls_default_filter().

For the translations thing, one more fix: make sure you add any new translatable files to po/POTFILES.in, import-export.c is missing.  After that, I think its good to push to master. Thanks!

Created attachment 173446 [details] [review]
Use g_str_has_suffix instead of strcmp to compare extensions

Good! Here they are.

Created attachment 173447 [details] [review]
Make error strings translatable (fixed _literal) v2

BTW: Is there any style guide about formatting if sentences? I have always had doubts...

Does that still apply to master?

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.

f2936bb Make error strings translatable
08517dc Use g_str_has_suffix instead of strcmp to compare extensions

(In reply to comment #17)
> BTW: Is there any style guide about formatting if sentences? I have always had
> doubts...

Not really. The best advice so far is follow whatever you see used (which works as far as there are no conflicting uses...)