<p><b>@b4n</b> requested changes on this pull request.</p>
<p>The idea seems good, and without having actually tested the PR I can confirm from other tests I did yetserday that the combination of <code>GetCommandLineW()</code> and <code>CommandLineToArgvW()</code> seems to work just fine.</p><hr>
<p>In <a href="https://github.com/geany/geany/pull/1258#pullrequestreview-7773217">src/win32.c</a>:</p>
<pre style='color:#555'>> +{
+ int num_arg;
+ LPWSTR *szarglist = CommandLineToArgvW(GetCommandLineW(), &num_arg);
+ char **utf8argv = g_new0(char *, num_arg + 1);
+ int i = num_arg;
+ while(i)
+ {
+ i--;
+ utf8argv[i] = g_utf16_to_utf8((gunichar2 *)szarglist[i], -1, NULL, NULL, NULL);
+ }
+ *pargc = num_arg;
+ *pargv = utf8argv;
+ LocalFree(szarglist);
+}
+
+void win32_free_argv_made_in_utf8(gint argc, gchar **argv)
</pre>
<p>should actually use <code>g_strfreev()</code> as the <code>argv[]</code> array ends with a <code>NULL</code> element.</p>
<hr>
<p>In <a href="https://github.com/geany/geany/pull/1258#pullrequestreview-7773217">src/win32.c</a>:</p>
<pre style='color:#555'>> @@ -1032,4 +1032,31 @@ gchar *win32_get_user_config_dir(void)
return g_build_filename(g_get_user_config_dir(), "geany", NULL);
}
+
+void win32_make_argc_and_argv_in_utf8(gint *pargc, gchar ***pargv)
+{
+ int num_arg;
+ LPWSTR *szarglist = CommandLineToArgvW(GetCommandLineW(), &num_arg);
+ char **utf8argv = g_new0(char *, num_arg + 1);
+ int i = num_arg;
+ while(i)
</pre>
<p>I'd prefer a <code>for()</code> loop like</p>
<div class="highlight highlight-source-c"><pre><span class="pl-k">for</span> (<span class="pl-k">int</span> i = <span class="pl-c1">0</span>; i < num_arg; i++)
utf8argv[i] = g_utf16_to_utf8((gunichar2 *)szarglist[i], -<span class="pl-c1">1</span>, <span class="pl-c1">NULL</span>, <span class="pl-c1">NULL</span>, <span class="pl-c1">NULL</span>);</pre></div>
<hr>
<p>In <a href="https://github.com/geany/geany/pull/1258#pullrequestreview-7773217">src/win32.c</a>:</p>
<pre style='color:#555'>> @@ -1032,4 +1032,31 @@ gchar *win32_get_user_config_dir(void)
return g_build_filename(g_get_user_config_dir(), "geany", NULL);
}
+
+void win32_make_argc_and_argv_in_utf8(gint *pargc, gchar ***pargv)
+{
+ int num_arg;
+ LPWSTR *szarglist = CommandLineToArgvW(GetCommandLineW(), &num_arg);
+ char **utf8argv = g_new0(char *, num_arg + 1);
+ int i = num_arg;
+ while(i)
+ {
+ i--;
+ utf8argv[i] = g_utf16_to_utf8((gunichar2 *)szarglist[i], -1, NULL, NULL, NULL);
</pre>
<p>Checking for error might be needed, as <a href="https://en.wikipedia.org/wiki/UTF-8#WTF-8">Windows seems to allows invalid UTF-16 in filenames</a>, and I'm afraid <code>g_utf16_to_utf8()</code> doesn't allow this.<br>
Another solution might be a custom "broken UTF-16 to WTF-8" converter, like I played with in <a href="https://github.com/b4n/wtf8tools">https://github.com/b4n/wtf8tools</a>. It seems most places in GLib don't have problem with WTF8, as it's structurally fine and "just" encodes weird stuff.</p>
<p>Anyway, something should be done because only the last element of <code>argv[]</code> should ever be <code>NULL</code>, and I wouldn't be surprised it'd crash somewhere if it wasn't respected.</p>
<p>OK, this is a low-risk issue I guess (invalid UTF-16 filenames), but still a theoretically possible one.</p>
<hr>
<p>In <a href="https://github.com/geany/geany/pull/1258#pullrequestreview-7773217">src/win32.c</a>:</p>
<pre style='color:#555'>> @@ -1032,4 +1032,31 @@ gchar *win32_get_user_config_dir(void)
return g_build_filename(g_get_user_config_dir(), "geany", NULL);
}
+
+void win32_make_argc_and_argv_in_utf8(gint *pargc, gchar ***pargv)
</pre>
<p>Agreed</p>
<hr>
<p>In <a href="https://github.com/geany/geany/pull/1258#pullrequestreview-7773217">src/libmain.c</a>:</p>
<pre style='color:#555'>> @@ -1231,6 +1230,9 @@ gint main_lib(gint argc, gchar **argv)
#endif
gtk_main();
+#ifdef G_OS_WIN32
+ win32_free_argv_made_in_utf8(argc, argv);
</pre>
<p>I'm not sure it's worth having a function which is a no-op on non-Windows just for not duplicating one guarding of a free; if we really don't want duplication here we find a way to only have one exit path :)</p>
<hr>
<p>In <a href="https://github.com/geany/geany/pull/1258#pullrequestreview-7773217">src/libmain.c</a>:</p>
<pre style='color:#555'>> @@ -1231,6 +1230,9 @@ gint main_lib(gint argc, gchar **argv)
#endif
gtk_main();
+#ifdef G_OS_WIN32
+ win32_free_argv_made_in_utf8(argc, argv);
</pre>
<p>Alternatively the conversion and freeing could be done in <code>main()</code> itself (from <em>main.c</em>), so <code>main_lib()</code> simply receives a nice argument list directly:</p>
<div class="highlight highlight-source-c"><pre><span class="pl-k">int</span> <span class="pl-en">main</span>(<span class="pl-k">int</span> argc, <span class="pl-k">char</span> **argv)
{
<span class="pl-k">int</span> ret;
#<span class="pl-k">ifdef</span> G_OS_WIN32
<span class="pl-c">/* on Windows, get the Unicode argulment list and convert it to UTF-8 into argv[] */</span>
LPWSTR *szarglist = <span class="pl-c1">CommandLineToArgvW</span>(<span class="pl-c1">GetCommandLineW</span>(), &argc);
argv = <span class="pl-c1">g_new</span>(<span class="pl-k">char</span> *, argc + <span class="pl-c1">1</span>);
<span class="pl-k">for</span> (<span class="pl-k">int</span> i = <span class="pl-c1">0</span>; i < argc; i++)
argv[i] = <span class="pl-c1">g_utf16_to_utf8</span>((gunichar2 *)szarglist[i], -<span class="pl-c1">1</span>, <span class="pl-c1">NULL</span>, <span class="pl-c1">NULL</span>, <span class="pl-c1">NULL</span>);
argv[argc] = <span class="pl-c1">NULL</span>;
<span class="pl-c1">LocalFree</span>(szarglist);
#<span class="pl-k">endif</span>
ret = <span class="pl-c1">main_lib</span>(argc, argv);
#<span class="pl-k">ifdef</span> G_OS_WIN32
<span class="pl-c1">g_strfreev</span>(argv);
#<span class="pl-k">endif</span>
<span class="pl-k">return</span> ret;
}</pre></div>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/geany/geany/pull/1258#pullrequestreview-7773217">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABDrJ3uxCo9j06CyzJBcySkPNRh-HACfks5q8YyBgaJpZM4KPpLl">mute the thread</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/ABDrJ86K9oPDYe9lyYqSmC4GNoOKEoeMks5q8YyBgaJpZM4KPpLl.gif" width="1" /></p>
<div itemscope itemtype="http://schema.org/EmailMessage">
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
<link itemprop="url" href="https://github.com/geany/geany/pull/1258#pullrequestreview-7773217"></link>
<meta itemprop="name" content="View Pull Request"></meta>
</div>
<meta itemprop="description" content="View this Pull Request on GitHub"></meta>
</div>
<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/geany/geany","title":"geany/geany","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/geany/geany"}},"updates":{"snippets":[{"icon":"PERSON","message":"@b4n requested changes on #1258"}],"action":{"name":"View Pull Request","url":"https://github.com/geany/geany/pull/1258#pullrequestreview-7773217"}}}</script>