Before it silently failed if there was any error. Now at least a console warning is logged.
The TODO is meant for post-1.27 to add a GUI message box or a log entry in the Status messages window to tell the user about the error. But since we are in string freeze, add at least a console warning.
I noticed the `plugin_help` functionality of some plugins is broken on Windows because the affected plugins use DOCDIR to construct the URL to the local help file. But on Windows we need some more advanced logic, probably based on `app->docdir` or so.
This log message helps identifying those problems. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/937
-- Commit Summary --
* Add error reporting for opening URIs on Windows
-- File Changes --
M src/win32.c (10)
-- Patch Links --
https://github.com/geany/geany/pull/937.patch https://github.com/geany/geany/pull/937.diff
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/937
Please scratch the part about plugins' help function being broken because they use DOCDIR. Maybe we can fix this for Windows in the build system. On Linux, everything is fine.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/937#issuecomment-192880565
@@ -794,7 +795,14 @@ void win32_open_browser(const gchar *uri) uri++; } }
- ShellExecute(NULL, "open", uri, NULL, NULL, SW_SHOWNORMAL);
- ret = (gint) ShellExecute(NULL, "open", uri, NULL, NULL, SW_SHOWNORMAL);
- if (ret <= 32)
I love Windows :D
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/937/files#r55140222
@@ -794,7 +795,14 @@ void win32_open_browser(const gchar *uri) uri++; } }
- ShellExecute(NULL, "open", uri, NULL, NULL, SW_SHOWNORMAL);
- ret = (gint) ShellExecute(NULL, "open", uri, NULL, NULL, SW_SHOWNORMAL);
- if (ret <= 32)
- {
gchar *err = g_win32_error_message(GetLastError());/* TODO add a GUI warning that opening an URI failed */g_warning("ShellExecute failed opening \"%s\" (code %d): %s", uri, ret, err);
You could add a translatable string if you want, it wouldn't get translated for next release but that wouldn't be worse than not translatable at all. But well, if it's meant to be changed right after the release (would have to remember :)), I guess it doesn't really matter.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/937/files#r55140237
LGBI
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/937#issuecomment-192895789
Please scratch the part about plugins' help function being broken because they use DOCDIR. Maybe we can fix this for Windows in the build system.
Can we? using relative paths won't help much, as we won't know for sure where we are. We'd need something like `utils_resource_dir()` in plugins too I guess. And on non-Windows we get the same problem when it's a relocatable executable (something probably never tests on Linux though).
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/937#issuecomment-192896129
@@ -794,7 +795,14 @@ void win32_open_browser(const gchar *uri) uri++; } }
- ShellExecute(NULL, "open", uri, NULL, NULL, SW_SHOWNORMAL);
- ret = (gint) ShellExecute(NULL, "open", uri, NULL, NULL, SW_SHOWNORMAL);
- if (ret <= 32)
- {
gchar *err = g_win32_error_message(GetLastError());/* TODO add a GUI warning that opening an URI failed */g_warning("ShellExecute failed opening \"%s\" (code %d): %s", uri, ret, err);
I would add it later, we could live without any error message until now, so it's not a blocker or so :). Maybe I'll add some GUI stuff before 1.27 but only if time permits and after all the rest is sorted out. If not, I'll add it after 1.27 but more than two weeks before 1.28 :).
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/937/files#r55140293
You are right. I was just too optimistic :(. Anyway, the docdir problems on Windows and relocatable executables is another issue.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/937#issuecomment-192917096
Ready to merge?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/937#issuecomment-195752775
sure
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/937#issuecomment-195754004
Merged #937.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/937#event-587436824
github-comments@lists.geany.org