> {
> ui_set_statusbar(TRUE, "%s", err->message);
> g_error_free(err);
> return FALSE;
> }
>
> - filedata->len = (gsize) st.st_size;
@b4n Maybe I miss something but right now (without my patch):
1. The stat() length value is passed to encodings_convert_to_utf8_auto() here
https://github.com/geany/geany/blob/master/src/document.c#L963
2. encodings_convert_to_utf8_auto() stores this value to buffer.size and strlen() to buffer.len parameter passed to handle_buffer() here:
https://github.com/geany/geany/blob/master/src/encodings.c#L1082
3. Finally, handle_buffer() compares these two and does some magic based on this:
https://github.com/geany/geany/blob/master/src/encodings.c#L1029
The thing is we need both the stat() value and strlen() value in order to do what's done in (3). Right now, my patch discards the stat() value which causes that the buffer->len != buffer->size check is equivalent to FALSE.
In the commit I mentioned the size was just moved into encodings.c but the two values are still there.
So I think this is something that should be fixed in my patch - personally I'd move the stat call inside handle_buffer() because passing the size value from somewhere else is a bit misleading (I at least assumed it was the size of the string which isn't the case).
What do you think?
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/621/files#r40608720
> {
> ui_set_statusbar(TRUE, "%s", err->message);
> g_error_free(err);
> return FALSE;
> }
>
> - filedata->len = (gsize) st.st_size;
> Yeah agreed it looked odd. FTR I just traced it back to some encoding-related stuff, no explanation why not using the size returned by the load function
Maybe we shouldn't store the length here at all if encoding stores `strlen()` as part of its NUL checks.
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/621/files#r40511290
Given that the next release has some big changes in it (sure most is in plugins not normal usage but still) maybe it would be better to have a stable release we can direct people back to in case of problems in the new release. So I can see benefits of a point release even if its close to the next release.
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/issues/605#issuecomment-143599712
> {
> ui_set_statusbar(TRUE, "%s", err->message);
> g_error_free(err);
> return FALSE;
> }
>
> - filedata->len = (gsize) st.st_size;
nah, before that commit `len` was `strlen(data)`, and there was a separate `size`. IIUC the only change there was that on this side of the code there was only one length, the true length of the data, and the post-encoding-conversion length was handled by the encoding part.
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/621/files#r40509129
> {
> ui_set_statusbar(TRUE, "%s", err->message);
> g_error_free(err);
> return FALSE;
> }
>
> - filedata->len = (gsize) st.st_size;
Hmm, just checked too and it's also related to
f60ead793d4435b8bd126519f093aef66b73a236
where size was moved to encodings.c and inside handle_buffer() (called from encodings_convert_to_utf8_auto()) it appears it distinguishes between len and size. So at least for encodings_convert_to_utf8_auto() we should actually use the value from stat(). Will try to fix.
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/621/files#r40507696
> + {
> + match = utils_str_equal(linebuf, "#!!PROXY_MAGIC!!\n");
> + fclose(f);
> + }
> + return match ? PROXY_MATCHED : PROXY_IGNORED;
> +}
> +@endcode
> +
> +GeanyProxyFuncs::load is a bit more complex. It reads the file, fills the inferior's PluginInfo
> +fields and calls GEANY_PLUGIN_REGISTER_FULL(). Additionally, it creates a per-plugin context that
> +holds GKeyFile instance (a poor man's interpeter context). You can also see that it does not call
> +GEANY_PLUGIN_REGISTER_FULL() if g_key_file_load_from_file() found an error (probably a syntax
> +problem) which means the sub-plugin cannot be enabled.
> +
> +It also installs wrapper functions for the inferior's GeanyPluginFuncs as Ini files aren't code.
> +It's very likely that your proxy needs something similar because you can only install function
agreed. at first "inferior" bugged me, but a dictionary lookup made me shut up and leave it to @elextr, which has now stricken :]
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/629/files#r40504011
I would vote to wait for the next release even though it seems to affect users.
Now we just have two more months until the next release.
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/issues/605#issuecomment-143547806
> @b4n I think it's actually good that the open documents don't change - users may change the sorting for individual open files manually and may not want them changed when the settings changes.
Sure, but one might have not touched anything but want to change all, and then need to reopen everything. Not so bad, but maybe not optimal (esp. for a first-class setting with UI and everything).
This said, I don't have a strong opinion either.
---
Otherwise LGTM.
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/581#issuecomment-143547598
@elextr @codebrainz @eht16 @frlan @ntrel What do you think about point release? OK, it's been 2 months so we're not so far from next release, and a bit late for a hot fixup (sorry, my fault basically), but if it's that annoying for many people it might make sense. Opinions?
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/issues/605#issuecomment-143547337