@techee
> In my opinion, the whole prefix of the message that should be removed [...]
I can remove the "FunctionName() failed:" from the spawn module. The other prefixes, if any, are added by the callers.
> ui_set_statusbar(TRUE, _("Cannot execute grep tool '%s';"
> " check the path setting in Preferences.")
This is indeed user friendly, but tells us absolutely nothing about why grep failed. My suggestion above was to maybe display a user friendly message, but also put the full text in Status or Debug -> Messages.
If we are going to display something like this, with no information from the actual GError message, there would be no reason to touch the spawn module...
(BTW, this particular message was not correct, unless you read "cannot execute" very widely: it was produced if g_find_program_in_path() failed, and the execution itself was about 60 lines below, with a completely different message.)
> _("Could not parse terminal command \"%s\" "
> "(check Terminal tool setting in Preferences)")
spawn_async() parses the command line and reports any errors in more detail. It also rejects insecure program names under Windows, which is not really a parsing error.
TL;DR:
Now, if I also remove the parsed command text from the spawn GError, leaving only the error description text (my 2nd suggestion), these may be unified and become:
Cannot execute grep tool "%s": <error>. Check the path setting in Preferences.
Cannot execute terminal command "%s": <error>. Check the path setting in Preferences.
For example:
Cannot execute grep tool "c:\msys32\usr\bin\grep.exe": The system cannot find the file specified. Check the path setting in Preferences.
Cannot execute terminal command "": Text was empty (or contained only whitespace). Check the path setting in Preferences.
Seems pretty reasonable to me.
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/issues/541#issuecomment-147111111
Filebrowser plugin works incorrectly, if in project settings has relative path. Problem in plugins/filebrowser.c file. The problem appears when use (pseudocode):
GeanyProject *project = geany->app->project;
dir = project->base_path ;
and not checked the path type - absolute or relative base_path.
The correct realization in the file geany-plugins/projectorganizer/src/prjorg-utils.c (function gchar *get_project_base_path(void)).
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/issues/686
Treebrowser plugin works incorrectly, if in project settings has relative path. Problem in treebrowser/src/treebrowser.c file. The problem appears when use (pseudocode):
GeanyProject *project = geany->app->project;
dir = project->base_path ;
and not checked the path type - absolute or relative base_path.
The correct realization in the file geany-plugins/projectorganizer/src/prjorg-utils.c (function gchar *get_project_base_path(void)).
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/issues/288
Just went through the code quickly and one more useful message that got removed in
690cb922be902f023881d455ae0c0a87d1c62170
is
ui_set_statusbar(TRUE,
_("Could not parse terminal command \"%s\" "
"(check Terminal tool setting in Preferences)"), tool_prefs.term_cmd);
(The other message I was referring to previously was removed in 74171cca5026e915e68b55f420b986b266ddcf7f)
Apart from these messages it should be OK (together with removal of the debugging messages from GError).
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/issues/541#issuecomment-147087496
> /** Function pointer type used for keybinding callbacks. */
> typedef void (*GeanyKeyCallback) (guint key_id);
>
> +/** Function pointer type used for keybinding callbacks, with userdata for passing context
> + *
> + * You should return @c TRUE to indicate handling the callback. (Occasionally, if the keybinding
> + * cannot apply in the current situation, it is useful to return @c FALSE to allow a later keybinding
> + * with the same key combination to handle it).
> + *
> + * @since 1.26 (API 226) */
> +typedef gboolean (*GeanyKeyBindingFunc)(GeanyKeyBinding *key, guint key_id, gpointer pdata);
I was trying to port some plugins to use this API and realized `key_id` is kinda redundant here with `key->id`. What about instead making `GeanyKeyBinding::id` public (add a doc comment) and dropping the `key_id` parameter here?
Or do we think it's good because it's more consistent with the group callback?
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/376/files#r41696921
@zhekov In my opinion, the whole prefix of the message that should be removed, i.e.:
search_find_in_files: spawn_with_callbacks() failed: CreateProcess() failed:
This is something the users shouldn't see when GError message is displayed - it's just something that might be useful for debugging (might be fine to put it to the debugging messages).
Regarding the grep message, I think it wasn't a good idea to replace the original error message:
ui_set_statusbar(TRUE, _("Cannot execute grep tool '%s';"
" check the path setting in Preferences."), tool_prefs.grep_cmd);
with
if (command_grep == NULL)
command_line = g_strdup_printf("%s %s --", tool_prefs.grep_cmd, opts);
else
{
command_line = g_strdup_printf("\"%s\" %s --", command_grep, opts);
The original message didn't use the GError at all and I think in this case it's the right thing to do - we know that the grep command failed to execute so we tell the user plus the info what needs to be changed.
I didn't go through all the uses of the new spawn module but I can imagine such a thing happens on other places too - from the special case where the spawn() is called we can probably create a more useful error message than what the spawn() itself can generate.
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/issues/541#issuecomment-147085213
The new filename for VTE 2.91 on linux is libvte-2.91.so.0 and vte_terminal_fork_command needs to be replaced with vte_terminal_spawn_sync because the former was deprecated for some time and has been removed.
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/issues/685
I drew a line with virtual spaces down between columns 2 and 3 and then pressed end. Should it select all the text of the longest line enclosed or just to the length of the last line?
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/issues/683
The spawn calls display nothing, of course, they only return a GError (and even know nothing about gtk+, only glib is used).
Since I haven't seen any internally used win~1 functions except CreateProcess (== exec) to produce any errors, removing the function name, except when compiling the test program, seems acceptable to me.
One unpleasant problem is that, if the command line can not be parsed properly, glib often includes it's text in the GError message, along with the translated error description. If the command line is in locale, it won't be displayed properly (Help -> Debug Messages will even terminate Geany with an assertion, as in PR #658). But the (entire) message can not be converted to utf-8 by the caller, because the translated text will break.
Maybe I should refrain from mimicking this glib behavior under win~1, and even replace the offending glib messages under *nix? It's possible, based on their GError codes.
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/issues/541#issuecomment-146928217