-
Notifications
You must be signed in to change notification settings - Fork 633
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
Socker:receive("l") without asterisk, L
option
#294
base: master
Are you sure you want to change the base?
Conversation
The TCP Receive code now uses a switch statement and allows dropping the * at the start of the string, like Lua's io library
Is this safe to use on different systems? unix vs windows, with single charcter lineends versus windows CrLf ? |
btw the "L" option is useful to be in line with newer Lua versions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test and documentation updates are missing, mind adding those?
Yes, it is. It currently just drops any CRs. |
@Tieske |
don't windows users then end up with a lingering LF? I'm not sure what the Lua 5.3 behaviour is, but I think it should mimic that. As it would be the "least surprising" option. |
I haven't reviewed this in detail, just noting my impression that it should be merged after the next (safe-harbor) release tag. |
Lua, for |
What did you test? I tested "*L" (on OSX) and it seems to cut on LF (10) (a single CR is ignored) my test code: print(_VERSION)
local CR = string.char(13)
local LF = string.char(10)
local function writefile(name, content)
local f = io.open(name, "wb")
f:write(content)
f:close()
end
local function readfile(name, mode)
local f = io.open(name, "r")
while f do
local l = f:read(mode)
if l then
local le = l:gsub(CR, "+CR"):gsub(LF, "+LF")
if le:sub(1,1) == "+" then
le = le:sub(2,-1)
end
print(#l, le)
else
f:close()
f = nil
end
end
end
local lines = { -- "*L" result:
"1234567890"..LF, -- 1 line, 11 bytes
"1234567890"..LF..CR..LF, -- 2 lines, 11 bytes + 2 bytes
"1234567890"..CR..LF, -- 1 line, 12 bytes
"1234567890"..CR..LF..LF, -- 2 lines, 12 bytes + 1 byte
"1234567890" -- 1 line, 10 bytes
}
lines = table.concat(lines)
writefile("test.txt", lines)
for _, mode in ipairs { "*l", "*L" } do
print("\nTesting mode: "..mode)
readfile("test.txt", mode)
end Output:
So looks to me like "*l" is platform dependent, whereas "*L" just cust right after the "LF", and is platform independent. I'll try and test on Windows as well. EDIT: writing the file; added the "B" for binary mode |
on Windows I get;
The "*l" option converts the line endings, which differs from the Unix one. At the same time the "*L" option, also differs because it retains the line endings, but only AFTER conversion. EDIT: updated this post after adding the "B" to the file being written in the test code |
Updated the test code and test results above, after a bug in the test code. |
The culprit seems to be the difference between "text" and "binary" mode on Windows systems, because if the file is opened for reading "binary" mode, then the Windows output is identical to the Mac output. So I think we should assume a socket to have a "binary" format when reading, and as such should get the same results. That would be cutting the line on each "LF" character, and not dropping any preceding "CR" character. Which is what the current PR seems to be doing. So a lot of fuss to conclude the PR looks good. But it would need some tests to prove the behavior. |
On the subject of tests, I would really prefer if all new tests were written in busted instead of the current ad-hock framework. Even if it's one test at a time lets start migrating that direction when we need a new test for something or to do some substantial fix to an existing test. I'll try to get a couple basic ones started so there is a pattern to follow. |
(This case of course is exacerbated by the fact that we're not current doing any CI testing on Windows at all; I'd be glad to facilitate if somebody with expertise knows how to make it go...) |
This pull request allows users to call
socket:receive()
without the parameter having to start with an asterisk. It also adds theL
option, which includes the end of line marker.