Update sci_get_selected_text_length() so it returns the same value like Scintilla 5.1.4 and earlier versions. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3099
-- Commit Summary --
* Update sci_get_selected_text_length() after change to Scintilla 5.1.5 * Fix warning
-- File Changes --
M src/sciwrappers.c (6) M src/ui_utils.c (4)
-- Patch Links --
https://github.com/geany/geany/pull/3099.patch https://github.com/geany/geany/pull/3099.diff
@techee pushed 1 commit.
88f802260fb12c8b713c44867116aa28be1fa542 Bump API version for the Scintilla 5.1.5 change
Not a huge fan of this but I can understand the compatibility concern. I think we do need a plan to eventually match scintilla (i.e. have a consistent API too).
E.g. 1) Introduce `sci_get_selected_text_length2()` with the new behavior and deprecate `sci_get_selected_text_length()` now 2) Remove `sci_get_selected_text_length()` at some later point 3) Re-introduce `sci_get_selected_text_length()` at some even more later point
We could also introduce some mechanism where plugins declare a API TARGET. E.g. if they target 242 (or none, because it's new) they get the old behavior, if they target 243 they get the new behavior. Obviously this works only for cases where we can handle the behavior change at compile time and in header-only. At some point we could reject plugins that target too old APIs (always with good reason, not "just because").
@kugel- sounds like a plan.
Which one of the two?
Oh, it was multiple choice :grin:
Steps 1,2,3 are fine for this function, step 1 as soon as anybody does it, and as we will need bump the ABI at step 2 it forces a recompile which will fail for any plugins still using the old call, so its all safe.
I read it as intended to be applied to this function only, but obviously it can be applied to any other individual function we want to change the behaviour of.
IIUC what the compile time part does is shrinks the period between steps 2 and 3, a plugin that was only re-compiled at steps 1 and 3 would then be using the new behaviour by accident, so some time is needed, but if a compile time check is also implemented it would allow it to be only one release cycle.
@kugel- (1) sounds good to me and I think it could actually stay this way forever (despite the not-so-nice name).
@techee pushed 1 commit.
acacbe30574a42a698d49385628e84d9cbdc1241 Add sci_get_selected_text_length2() and deprecate sci_get_selected_text_length()
I added the `sci_get_selected_text_length2()` function and made `sci_get_selected_text_length()` deprecated in the last commit. I'm just not getting the depreciation warnings when using `GEANY_DEPRECATED_FOR`. Am I doing something wrong?
Perhaps you have disabled it somehow? We normally get a lot of these warnings because Geany uses deprecated GTK3 stuff.
I just run plain `autogen.sh` without specifying anything extra. If I'm not mistaken, the GTK3 stuff is suppressed by
https://github.com/geany/geany/blob/142b8ffdd4b7372b77966abbc99435644210515a...
But it should be only for GTK.
I'm just not getting the depreciation warnings when using `GEANY_DEPRECATED_FOR`. Am I doing something wrong?
This is probably intended. I understand https://github.com/geany/geany/blob/master/src/geany.h#L58 so thst those deprecation warnings are only generated if `GEANY_PRIVATE` is *not* defined. When building Geany itself, `GEANY_PRIVATE` *is* defined. So, not seeing these messages is ok when building Geany. I guess in plugins they would appear.
BTW on the topic: I like the idea of `sci_get_selected_text_length2()` and to deprecate `sci_get_selected_text_length`. I'm thinking about a better name for the new function, so far without a good result unfortunately :(.
thinking about a better name for the new function, so far without a good result unfortunately :(.
Not alone, sqlite couldn't either, [for example](https://www.sqlite.org/c3ref/create_module.html) :smile:
Should we merge this?
I would agree with @techee that step 1 from @kugel-'s three step road would be enough in the long term. Even if not, we can follow steps 2 and 3 at any time later.
Fine for me to merge
@kugel- approved this pull request.
Merged #3099 into master.
github-comments@lists.geany.org