Add ability to load raw data from a string [paintutils] #378
Conversation
|
I feel it would be nicer if it were a separate method (like |
While the check (most likely) wouldn't break anything, it's better to fix it and not worry then to not fix it and find out it breaks image loading. Also fixes how files are loaded.
| sLine = file:read() | ||
| end | ||
| file:close() | ||
| return tImage | ||
| return parseImage( sContent:sub(2) ) -- remove first newline and delegate parsing of images to parseImage function as suggested by @SquidDev in discussion of PR #378 |
There was a problem hiding this comment.
You don't need that much context in the comment :p. Though seeing as you've got the parseLine function, you could just use that.
| return result | ||
| end | ||
| local tImage = {} | ||
| for _, sLine in pairs( split( sPath, "\n" ) ) do -- read each line like original file handling did |
There was a problem hiding this comment.
I wonder if it would be cleaner to iterate using :gmatch instead of splitting the string and then looping over it. You also don't need to reassign tImage on the next line.
There was a problem hiding this comment.
issue is, :gmatch won't give you the last item (i.e; for string "0123\n4567", :gmatch with a newline as the delimiter only gives "0123" and not "4567".)
There was a problem hiding this comment.
"[^\n]+"
Edit:
Actually that's no good, paint images may contain empty lines at any point and this won't capture those. Hmm.
This works:
for sLine in (sRawData.."\n"):gmatch("(.-)\n") do
... but I'd like to believe there's a cleaner solution.
Also I'm guessing you didn't bother to test your code, as you're trying to split "sPath" instead of "sRawData".
Also fixes a typo.
I agree with SquidDev, I went a bit too far with the whole context thing.
| local sLine = file:read() | ||
| local sContent = "" | ||
| while sLine do |
There was a problem hiding this comment.
This loop could be replaced with a readAll
There was a problem hiding this comment.
Personally I'd run an io.lines() loop and call parseLines() directly.
There was a problem hiding this comment.
I believe that passing the file contents to parseImage makes more sense in the context of what it does. I will, however, replace the while loop with a :readAll call.
There was a problem hiding this comment.
In that case, why even have parseLine? Or have it return anything, for that matter... remember tImageArg doesn't hold a table, but a pointer leading to one.
loadImage is additionally creating a redundant tImage table that doesn't need to be there.
This removes the need to create a temporary file just to load an image from a string. (see hence)