The condition is wrong and for version 2.0 it will constantly report there's an update available when upgrading from 1.x (where x > 0).
I had a "visionary" moment regarding possible 2.0 problems and had a look at the updatechecker plugin if it handles major versions - id does, but wrongly (in addition, there are two identical conditions for the "minor" version but the "mini" version check is missing).
I haven't tested this at all (not sure how to test when there's no actual release).
@b4n @eht16 I think this or some other variant of this patch should go to 2.0. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany-plugins/pull/1284
-- Commit Summary --
* updatechecker: Fix logic comparing version numbers
-- File Changes --
M updatechecker/src/updatechecker.c (22)
-- Patch Links --
https://github.com/geany/geany-plugins/pull/1284.patch https://github.com/geany/geany-plugins/pull/1284.diff
@b4n approved this pull request.
LGTM (not tested)
Indeed the check is wrong in many aspects -- event 0.0.2 would be < then 1.38.1 Also, it uses `GEANY_VERSION`, which is a compile-time thing, so better update your update checker! It could benefit from Geany exposing `main_get_version_string()` or similar (but probably not under that name).
Indeed the check is wrong in many aspects -- event 0.0.2 would be < then 1.38.1
Here you'll be saved by the copy-paste error of the current code where the last digit is ignored ;-).
The current code won't behave as bad as I thought originally because for releases the value on the server is the highest number. It would be just in the meantime, like now, where we already have 2.0 and server says 1.38 and for the code 1.38 > 2.0. Anyway, better to fix and also handle the last digit of the version.
Also, it uses GEANY_VERSION, which is a compile-time thing, so better update your update checker! It could benefit from Geany exposing main_get_version_string() or similar (but probably not under that name).
Yeah, but hopefully most of the time users update geany+plugins at the same time.
Merged #1284 into master.
Here you'll be saved by the copy-paste error of the current code where the last digit is ignored ;-).
Ah indeed, missed that typo :)
The current code won't behave as bad as I thought originally because for releases the value on the server is the highest number. It would be just in the meantime, like now, where we already have 2.0 and server says 1.38 and for the code 1.38 > 2.0.
Well, one issue is that people using the plugin won't be notified about 2.0 anyway, as it'll make it older than basically whatever their version is… but we can't do anything about that.
Well, one issue is that people using the plugin won't be notified about 2.0 anyway, as it'll make it older than basically whatever their version is… but we can't do anything about that.
They should get notified because of ``` geany_running.major < geany_current.major ``` The problem was rather the potential "false positive" update notifications of the plugin. (I think the biggest problem of the code was to reason about what actually it would do.)
Ah right, it was notifying when *either* `major` or `minor` was higher. So yeah, only false positives if the local version is newer than the release.
github-comments@lists.geany.org