opened
mbartlett21
username_0
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.
Socker:receive("l") without asterisk, `L` option
issue
created
Tieske
username_1
Is this safe to use on different systems? unix vs windows, with single charcter lineends versus windows CrLf ?
comment
created
Tieske
username_1
btw the "L" option is useful to be in line with newer Lua versions
comment
created
mbartlett21
username_0
@username_1 

Yes, it is. It currently just drops any CRs.
comment
created
mbartlett21
username_0
For the documentation, do I remove mentions of it with the asterisk, seeing as that seems to be what happened with [`file:read()`](https://www.lua.org/manual/5.4/manual.html#pdf-file:read)?
comment
created
alerque
username_2
I haven't reviewed this in detail, just noting my impression that it should be merged after the next (safe-harbor) release tag.
comment
created
mbartlett21
username_0
Lua, for `file:read()` seems to just let CRs through. Is that what you want me to do here?
comment
created
Tieske
username_1
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:
```lua
print(_VERSION)
local CR = string.char(13)
local LF = string.char(10)

local function writefile(name, content)
  local f = io.open(name, "w")
  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.
comment
created
Tieske
username_1
interestingly, I get the exact same result on Windows;
```
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.23 seconds (pid: 5044).
```

where I would have expected the "*l" option to also drop the CR's.

But this does not matter for this implementation, since the "*L" behaviour is the same.
comment
created
Tieske
username_1
Updated the test code and test results above, after a bug in the test code.
comment
created
Tieske
username_1
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.
comment
created
alerque
username_2
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.
comment
created
alerque
username_2
(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...)
comment