Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cast start number of ordered list item to integer in Lua 5.3 #30

Merged
merged 1 commit into from Mar 20, 2019

Conversation

Witiko
Copy link
Collaborator

@Witiko Witiko commented Mar 19, 2019

In Lua 5.2, both print(4) and print(4.0) writes out 4, since both numbers are internally represented as floats. Since version 5.3, Lua differentiates between floats and integers, which affects how casting numbers to strings behaves. Now, print(4) writes out 4, whereas print(4.0) writes out 4.0.

The code seems to be mostly immune to this change, since we rarely work with numbers, but I discovered one minor regression. Since larsers.enumerator captures both the number and the period, the tonumber function casts the string to a float rather than an integer, affecting the conversion output.

@@ -1126,6 +1126,9 @@ function M.new(writer, options)
local function ordered_list(s,tight,startnum)
if options.startnum then
startnum = tonumber(startnum) or 1 -- fallback for '#'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be startNumber = ?

Copy link
Collaborator Author

@Witiko Witiko Mar 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry for not double-checking, startnum is startNumber in my fork.

@jgm jgm merged commit 0838351 into jgm:master Mar 20, 2019
@Witiko Witiko deleted the fix-lua-5.3 branch April 18, 2019 21:22
@Witiko Witiko mentioned this pull request May 11, 2019
@Witiko
Copy link
Collaborator Author

Witiko commented May 27, 2019

Please note that complete Lua 5.3 support will also require dropping the slnunicode dependency, among other things. I have not yet sunk my teeth into this, but it seems that we can support Lua 5.3 and earlier from one code base using two rockspecs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants