[virt-tools-list] [PATCH virt-viewer] fix character encoding on Windows platform

[Qiu Wenbo]

That's ok.

On 2018/10/15 21:13, Christophe Fergeau wrote:
> Hey, patch looks good to me, if that's ok with you, I'll change the
> commit log to:
>
>      win32: fix command line encoding on windows platform
>
>      On Windows, the arguments we get in GApplication::ocal_command_line
>      come from g_win32_get_command_line(), and g_option_context_parse_strv()
>      documentation says:
>      « On Windows, the strings are expected to be in UTF-8. This is in
>      contrast to g_option_context_parse() which expects them to be in the
>      system codepage, which is how they are passed as argv to main(). See
>      g_win32_get_command_line() for a solution. »
>      This was causing issues on Windows when running:
>      remote-viewer -t "你好" spice://<target-host>:5900
>
> Christophe
>
> On Mon, Oct 15, 2018 at 08:35:34PM +0800, Qiu Wenbo wrote:
>> Sorry, I don't read the code very carefully. You are right, since the
>> default command line is parsed by g_option_context_parse_strv (https://gitlab.gnome.org/GNOME/glib/blob/7c1f38b1c9d97519bf087a37a7d8c0a6e6bfdf18/gio/gapplication.c#L561)
>> we can use the same code path. I have generated a new patch.
>>
>> On 2018/10/15 19:54, Christophe Fergeau wrote:
>>> Hey,
>>>
>>> On Mon, Oct 15, 2018 at 07:26:12PM +0800, Qiu Wenbo wrote:
>>>> Thank you. We can't use the same code path here. The code path of
>>>> GApplication on Windows is different compared to Linux. To handle different
>>>> file name encodings on Windows, GApplication will get its command line
>>>> arguments through g_win32_get_command_line() which will return an arrary of
>>>> UTF-8 encoded strings. But on Linux, GApplication simply get its command
>>>> line  arguments through argv of the program's main() which is in encoded by
>>>> current system locale. The only difference between g_option_context_parse()
>>>> and g_option_context_parse_strv() is how they interpret their arguments. We
>>>> need use g_option_context_parse_strv on Windows since it accept UTF-8
>>>> encoded strings. And we should use the original code path to handle system
>>>> locale encoded strings on Linux.
>>> Looking at glib's code, on linux, I think we'd be getting locale encoded
>>> strings with both methods on linux
>>> ( https://gitlab.gnome.org/GNOME/glib/blob/7c1f38b1c9d97519bf087a37a7d8c0a6e6bfdf18/gio/gapplication.c#L2385
>>> and
>>> https://gitlab.gnome.org/GNOME/glib/blob/fba8d7a48ad0f4455461162130c7f2eec9c4e363/glib/goption.c#L2688
>>> - strv_mode is only used on Windows to convert between locale encoding
>>>     and utf8).
>>>
>>> Christophe
>>  From f96c6c4bdc9e7d997fb38aeebc74108076d0017c Mon Sep 17 00:00:00 2001
>> From: Qiu Wenbo <qiuwenbo at kylinos.com.cn>
>> Date: Mon, 15 Oct 2018 20:23:45 +0800
>> Subject: [PATCH] win32: fix command line encoding on windows platform
>>
>> ---
>>   configure.ac          | 4 ++--
>>   src/virt-viewer-app.c | 3 +--
>>   2 files changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 9b52eb4..bfbf27d 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -13,8 +13,8 @@ m4_ifndef([AM_SILENT_RULES], [m4_define([AM_SILENT_RULES],[])])
>>   AM_SILENT_RULES([yes])
>>   
>>   # Keep these two definitions in agreement.
>> -GLIB2_REQUIRED="2.38"
>> -GLIB2_ENCODED_VERSION="GLIB_VERSION_2_38"
>> +GLIB2_REQUIRED="2.40"
>> +GLIB2_ENCODED_VERSION="GLIB_VERSION_2_40"
>>   
>>   # Keep these two definitions in agreement.
>>   GTK_REQUIRED="3.12"
>> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
>> index 889cc6b..256eea7 100644
>> --- a/src/virt-viewer-app.c
>> +++ b/src/virt-viewer-app.c
>> @@ -1809,7 +1809,6 @@ virt_viewer_app_local_command_line (GApplication   *gapp,
>>   {
>>       VirtViewerApp *self = VIRT_VIEWER_APP(gapp);
>>       gboolean ret = FALSE;
>> -    gint argc = g_strv_length(*args);
>>       GError *error = NULL;
>>       GOptionContext *context = g_option_context_new(NULL);
>>       GOptionGroup *group = g_option_group_new("virt-viewer", NULL, NULL, gapp, NULL);
>> @@ -1828,7 +1827,7 @@ virt_viewer_app_local_command_line (GApplication   *gapp,
>>       g_option_context_add_group(context, spice_get_option_group());
>>   #endif
>>   
>> -    if (!g_option_context_parse(context, &argc, args, &error)) {
>> +    if (!g_option_context_parse_strv(context, args, &error)) {
>>           if (error != NULL) {
>>               g_printerr(_("%s\n"), error->message);
>>               g_error_free(error);
>> -- 
>> 2.19.0
>>