Conversation
Makes it easy to write if mudletOlderThan("3.2") then return end
src/mudlet-lua/lua/Other.lua
Outdated
|
|
||
| -- strip trailing zeros from input version | ||
| local input = rex.gsub(version, [[(\.0+)+$]], ''):split('%.') | ||
| local mudlets = {getMudletVersion("table")} |
There was a problem hiding this comment.
Why not use local major, minor, patch = getMudletVersion() to get just the numbers you want...?
|
It's easier that way for the rest of the algorithm :)
…On Sat, 3 Jun 2017 5:04 pm Stephen Lyons, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/mudlet-lua/lua/Other.lua
<#1064 (comment)>:
> @@ -636,3 +636,29 @@ function shms(seconds, bool)
return hh, mm, ss
end
end
+
+-- returns true if your Mudlet is older than the given version
+-- for example, it'll return true if you're on 2.1 and you do mudletOlderThan("3.1")
+-- it'll return false if you're on 4.0 and you do mudletOlderThan("4.0.0")
+-- credit to https://stackoverflow.com/a/16187766/72944 for the algorithm
+function mudletOlderThan(version)
+ local tonumber = tonumber
+
+ -- strip trailing zeros from input version
+ local input = rex.gsub(version, [[(\.0+)+$]], ''):split('%.')
+ local mudlets = {getMudletVersion("table")}
Why not use local major, minor, patch = getMudletVersion() to get just
the *numbers* you want...?
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#1064 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAGxjDDgBOkUQOkUf-vzSNp731SvhFyBks5sAXXngaJpZM4NvFRL>
.
|
keneanung
left a comment
There was a problem hiding this comment.
In general the indenting is funny.
src/mudlet-lua/lua/Other.lua
Outdated
|
|
||
| local diff | ||
| for i = 1, minlength do | ||
| diff = tonumber(input[i]) - mudlets[i] |
There was a problem hiding this comment.
Can we do a null check on the tonumber result first, in case we don't get a numbered version? Then we can return a better error than throwing an arithmic with nil error.
There was a problem hiding this comment.
The tonumber() is there because string.split returns a list of strings. I don't think we should error on strings being passed as part of the version - it can happen - how about treating them as 0?
That'll also then remove the need to strip out -dev in the above lines.
|
I've had a fresh look and rewritten the function, thanks @SlySven for the idea. I was looking at it from a more general version comparison perspective but as we only need this for Mudlet, we can afford to get a lot of efficiency gains. |
SlySven
left a comment
There was a problem hiding this comment.
Needs a typo fixing but otherwise OK.
src/mudlet-lua/lua/Other.lua
Outdated
|
|
||
| assert(type(inputmajor) == "number", sformat("bad argument #1 type (major version as number expected, got %s!)", type(inputmajor))) | ||
| assert(inputminor == nil or type(inputminor) == "number", sformat("bad argument #2 type (optional minor version as number expected, got %s!)", type(inputminor))) | ||
| assert(inputpatch == nil or type(inputpatch) == "number", sformat("bad argument #2 type (optional patch version as number expected, got %s!)", type(inputpatch))) |
There was a problem hiding this comment.
Should that not be "bad argument #3..." in this one?
| -- for example, it'll return true if you're on 2.1 and you do mudletOlderThan(3,1) | ||
| -- it'll return false if you're on 4.0 and you do mudletOlderThan(4,0,0) | ||
| function mudletOlderThan(inputmajor, inputminor, inputpatch) | ||
| local mudletmajor, mudletminor, mudletpatch = getMudletVersion("table") |
There was a problem hiding this comment.
😊 It is embarrassing but I really did cock-up the optional arguments for this command - no arguments produces a lua associative table containing four string-keys "major", "minor", "patch" and "build" whereas the return for a supplied argument of "table" is a multi-return set of four return values with those items - "build" being a nil for "release" versions in either case (so it disappears anyway)...!
keneanung
left a comment
There was a problem hiding this comment.
This looks much better. The tests don't work anymore now, but since they are broken anyways, I'm ok with it.
We all know the pain of not having a proper function to do version comparison, so we shouldn't inflict it on Mudlet users.
Makes it easy to write
if mudletOlderThan("3.2") then return endin case some features cannot be checked in the preferred way ofif desiredMudletFunction thensuch as coroutine support - coroutines were always present and would instantly crash the application if you tried to use them. There's no way to check for that sans checking the major+minor version.I've also dumped some tests but as busted doesn't work, they're incomplete. Somehow busted's tests are passing so we'll have to see how do they make it work.
Tagging @Mudlet/lua-interface for review.