[Github-comments] [geany/geany] Provide utf-8 command line arguments on Windows (#1258)

[Colomban Wendling]

b4n requested changes on this pull request.

The idea seems good, and without having actually tested the PR I can confirm from other tests I did yetserday that the combination of `GetCommandLineW()` and `CommandLineToArgvW()` seems to work just fine.

> +{
+	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)

should actually use `g_strfreev()` as the `argv[]` array ends with a `NULL` element.

> @@ -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'd prefer a `for()` loop like
```C
for (int i = 0; i < num_arg; i++)
	utf8argv[i] = g_utf16_to_utf8((gunichar2 *)szarglist[i], -1, NULL, NULL, NULL);
```

> @@ -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);

Checking for error might be needed, as [Windows seems to allows invalid UTF-16 in filenames](https://en.wikipedia.org/wiki/UTF-8#WTF-8), and I'm afraid `g_utf16_to_utf8()` doesn't allow this.
Another solution might be a custom "broken UTF-16 to WTF-8" converter, like I played with in https://github.com/b4n/wtf8tools.  It seems most places in GLib don't have problem with WTF8, as it's structurally fine and "just" encodes weird stuff.

Anyway, something should be done because only the last element of `argv[]` should ever be `NULL`, and I wouldn't be surprised it'd crash somewhere if it wasn't respected.

OK, this is a low-risk issue I guess (invalid UTF-16 filenames), but still a theoretically possible one.

> @@ -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)

Agreed

> @@ -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);

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 :)

> @@ -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);

Alternatively the conversion and freeing could be done in `main()` itself (from *main.c*), so `main_lib()` simply receives a nice argument list directly:

```C
int main(int argc, char **argv)
{
	int ret;
#ifdef G_OS_WIN32
	/* on Windows, get the Unicode argulment list and convert it to UTF-8 into argv[] */
	LPWSTR *szarglist = CommandLineToArgvW(GetCommandLineW(), &argc);

	argv = g_new(char *, argc + 1);
	for (int i = 0; i < argc; i++)
		argv[i] = g_utf16_to_utf8((gunichar2 *)szarglist[i], -1, NULL, NULL, NULL);
	argv[argc] = NULL;
	LocalFree(szarglist);
#endif

	ret = main_lib(argc, argv);

#ifdef G_OS_WIN32
	g_strfreev(argv);
#endif

	return ret;
}
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/1258#pullrequestreview-7773217
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20161109/37f43408/attachment.html>