Re: [Github-comments] [geany/geany] Gracefully handle proxies registering invalid extensions (#1212)
> I kind of did ignore it, because as I understood what he meant was to use > `g_error()` or alike and crash Geany, which sounded kind of harsh/insane. > This was possibly a misunderstanding on my part though. Fair point. Maybe a more clear comment on why you thought it was a bad idea, or why @kugel- thought it was could have made things clearer. But well, it's harder to see how other will understand us, that's for sure. > Since it was such a trivial patch and the number of people who will encounter > this code path so small, I figured I'd just make an "executive decision" so > to speak. Which does make sense, otherwise if nobody "feels strongly" nothing ever happens. -- 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/1212#issuecomment-247567583
Re: [Github-comments] [geany/geany] Gracefully handle proxies registering invalid extensions (#1212)
> As the discussion reads from here, it looks like you ignored it. I kind of did ignore it, because as I understood what he meant was to use `g_error()` or alike and crash Geany, which sounded kind of harsh/insane. This was possibly a misunderstanding on my part though. Since it was such a trivial patch and the number of people who will encounter this code path so small, I figured I'd just make an "executive decision" so to speak. > Same with showing the plugin name in the warning (#1212 (comment)) -- > probably a mere forgetfulness, though. That was indeed forgetfulness (or rather misreading), I missed your comment when reviewing the comments before merging. -- 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/1212#issuecomment-247566056
Re: [Github-comments] [geany/geany] Gracefully handle proxies registering invalid extensions (#1212)
> So do you want me to change it so it does a hard crash of Geany when a > string-based plugin typo is detected? Hum, I didn't think so no, rather just warn and leave the plugin not working as expected, like in #1233, which see. -- 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/1212#issuecomment-247564334
Re: [Github-comments] [geany/geany] Gracefully handle proxies registering invalid extensions (#1212)
> I kinda have to agree with @kugel- that his remark should have been addressed > on way or another before merging, especially as it looks like a fairly easy > change. He said he didn't feel strongly, and obviously I implemented it this way so I at least feel more strongly. But point taken. So do you want me to change it so it does a hard crash of Geany when a string-based plugin typo is detected? -- 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/1212#issuecomment-247561036
Re: [Github-comments] [geany/geany] Gracefully handle proxies registering invalid extensions (#1212)
Meta point: I kinda have to agree with @kugel- that his remark should have been addressed on way or another before merging, especially as it looks like a fairly easy change. Sure one can further improve things later, but it kind of misses the point of reviewing then; and requires a fairly higher involvement so is likely to lead to not happen even for a very simple fix, leading to the potentially worth remark to just be thrown away. -- 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/1212#issuecomment-247559392
Re: [Github-comments] [geany/geany] Gracefully handle proxies registering invalid extensions (#1212)
> @codebrainz Can you make the change if you don't feel strong either? The > documentation is clear so we don't have to be forgiving. If you feel strongly, go ahead and add a commit to change it, I will support it. -- 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/1212#issuecomment-246157680
Re: [Github-comments] [geany/geany] Gracefully handle proxies registering invalid extensions (#1212)
Merged #1212. -- 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/1212#event-785082909
Re: [Github-comments] [geany/geany] Gracefully handle proxies registering invalid extensions (#1212)
@codebrainz Can you make the change if you don't feel strong either? The documentation is clear so we don't have to be forgiving. -- 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/1212#issuecomment-245685550
Re: [Github-comments] [geany/geany] Gracefully handle proxies registering invalid extensions (#1212)
Ok, I read "that one place" to be in the HOWTO :) -- 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/1212#issuecomment-245153099
Re: [Github-comments] [geany/geany] Gracefully handle proxies registering invalid extensions (#1212)
I meant "and" like "in addition to the reference docs". It's mentioned in the `@param` doc for the `extensions` argument. -- 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/1212#issuecomment-245151908
Re: [Github-comments] [geany/geany] Gracefully handle proxies registering invalid extensions (#1212)
> (and in the Proxy Howto). Certainly if its a requirement its should be in the documentation, not the howto. If you don't want to fix that here can you make an issue pointing it out. -- 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/1212#issuecomment-245150619
Re: [Github-comments] [geany/geany] Gracefully handle proxies registering invalid extensions (#1212)
> I'd prefer to error out (perhaps we need the dot in the future?) but I don't > feel strong. I personally don't feel strong about the specifics either, just that it should give some help. Mostly I made this PR out frustration after spending an hour or so scratching my head trying to understand why my proxy plugin hooks weren't being called, sprinkling `printf`s all over my code and deep inside the bowels of Geany to solve it (yeah, I know I should've used my debugger). I simply just missed the mention in the API reference that said it doesn't include the dots. To my mind the dot is part of the extension, so I didn't even suspect that could be the problem until I really started pinning down the program flow. -- 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/1212#issuecomment-244988396
Re: [Github-comments] [geany/geany] Gracefully handle proxies registering invalid extensions (#1212)
I'd prefer to error out (perhaps we need the dot in the future?) but I don't feel strong. -- 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/1212#issuecomment-244933640
Re: [Github-comments] [geany/geany] Gracefully handle proxies registering invalid extensions (#1212)
Sounds reasonable -- 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/1212#issuecomment-244932042
Re: [Github-comments] [geany/geany] Gracefully handle proxies registering invalid extensions (#1212)
> @@ -1963,6 +1963,18 @@ static void pm_show_dialog(GtkMenuItem *menuitem,
> gpointer user_data)
> }
>
>
> +static const gchar *fix_extension(const gchar *ext)
> +{
> + if (*ext == '.')
> + {
> + g_warning(_("Proxy plugin extension '%s' starts with a dot, "
> + "stripping. Please fix your proxy plugin."), ext);
would be nice to include the plugin name somewhere
--
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/1212/files/4d0acad41751552c7b271e6d06bd64a9f9682e29#r77622584
