Socker:receive("l") without asterisk, L option

[mbartlett21]

This pull request allows users to call socket:receive() without the parameter having to start with an asterisk. It also adds the L option, which includes the end of line marker.

[Tieske]

Is this safe to use on different systems? unix vs windows, with single charcter lineends versus windows CrLf ?

[Tieske]

btw the "L" option is useful to be in line with newer Lua versions

[Tieske]

test and documentation updates are missing, mind adding those?

[mbartlett21]

Is this safe to use on different systems? unix vs windows, with single charcter lineends versus windows CrLf ?

@Tieske

Yes, it is. It currently just drops any CRs.

[mbartlett21]

test and documentation updates are missing, mind adding those?

@Tieske
For the documentation, do I remove mentions of it with the asterisk, seeing as that seems to be what happened with file:read()?

[Tieske]

Is this safe to use on different systems? unix vs windows, with single charcter lineends versus windows CrLf ?

@Tieske

Yes, it is. It currently just drops any CRs.

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.

[alerque]

I haven't reviewed this in detail, just noting my impression that it should be merged after the next (safe-harbor) release tag.

[mbartlett21]

Is this safe to use on different systems? unix vs windows, with single charcter lineends versus windows CrLf ?

@Tieske
Yes, it is. It currently just drops any CRs.

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.

Lua, for file:read() seems to just let CRs through. Is that what you want me to do here?

[Tieske]

Lua, for file:read() seems to just let CRs through. Is that what you want me to do here?

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:

Lua 5.3

Testing mode: *l
10	1234567890
10	1234567890
1	CR
11	1234567890+CR
11	1234567890+CR
0	
10	1234567890

Testing mode: *L
11	1234567890+LF
11	1234567890+LF
2	CR+LF
12	1234567890+CR+LF
12	1234567890+CR+LF
1	LF
10	1234567890
Program completed in 0.16 seconds (pid: 86623).

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

[Tieske]

on Windows I get;

Lua 5.3

Testing mode: *l
10	1234567890
10	1234567890
0	
10	1234567890
10	1234567890
0	
10	1234567890

Testing mode: *L
11	1234567890+LF
11	1234567890+LF
1	LF
11	1234567890+LF
11	1234567890+LF
1	LF
10	1234567890
Program completed in 0.25 seconds (pid: 18632).

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

[Tieske]

Updated the test code and test results above, after a bug in the test code.

[Tieske]

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.

[alerque]

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.

[alerque]

(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...)

[mbartlett21]

I've updated the newline handling so that it only does the following:

l or *l: Receives a line including a \n. Trims the \r\n or \n off the end.
L or *L: Receives a line including a \n with all characters intact.

---

I've manually tested this with 5.4 locally on my Windows machine. I've also updated the empty_connect test to reconnect if it got the local computer location. (Otherwise it just hangs on the next test that uses the data connection).

[mbartlett21]

@Tieske @alerque
Are you happy to review this at some point?
I've now rebased on the latest changes and added some tests for the newline handling. (particularly around it now allowing CRs in the middle of lines).

[mbartlett21]

I've now rebased this on top of the master branch and it fails the tests with CRs.

This is due to the sock:receive("*l") call only discarding \rs immediately before the line terminator rather than dropping every one. Is that ok to change behaviour or should it only happen for sock:receive("L")?