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

nitcorn: fix bug with binary files #1817

Merged
merged 7 commits into from
Nov 7, 2015
Merged

Conversation

xymus
Copy link
Contributor

@xymus xymus commented Nov 7, 2015

Downloading binary files from a nitcorn server works as expected once again.

The API should be enough for most users but there is a few features lacking. Namely alternating custom text with files and writing custom data. We could merge the body and files attributes into a sequence of Bytables to send, with support for Bytes.

You can test the result on xymus.net, either by downloading the latest WBTW at http://xymus.net/pub/wbtw-v0.4-110-g668b261.tar.gz or by playing a clone of Super Hexagon (mostly made by @ablondin) at http://xymus.net/pub/hex/.

if err then
last_error = new IOError("Failed to add file at '{path}'")
file.close
end
Copy link
Member

Choose a reason for hiding this comment

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

quite convoluted, only the FileReaded.open seems necesarry, the initial file_exist is useless since the error will be caugut latter. And the file_stat sould be replaced by a FileStream::stat or something that use the POSIX fstat internally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I'm not mistaken file.file_stat already uses fstat. But l'll remove the file_exist check.

Copy link
Member

Choose a reason for hiding this comment

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

re +1 then

@privat
Copy link
Member

privat commented Nov 7, 2015

+1

Signed-off-by: Alexis Laferrière <alexis.laf@xymus.net>
Signed-off-by: Alexis Laferrière <alexis.laf@xymus.net>
Signed-off-by: Alexis Laferrière <alexis.laf@xymus.net>
Signed-off-by: Alexis Laferrière <alexis.laf@xymus.net>
Signed-off-by: Alexis Laferrière <alexis.laf@xymus.net>
Signed-off-by: Alexis Laferrière <alexis.laf@xymus.net>
Signed-off-by: Alexis Laferrière <alexis.laf@xymus.net>
@xymus
Copy link
Contributor Author

xymus commented Nov 7, 2015

Rerolled to remove the file_exist check.

privat added a commit that referenced this pull request Nov 7, 2015
Downloading binary files from a nitcorn server works as expected once again.

The API should be enough for most users but there is a few features lacking. Namely alternating custom text with files and writing custom data. We could merge the `body` and `files` attributes into a sequence of `Bytables` to send, with support for `Bytes`.

You can test the result on xymus.net, either by downloading the latest WBTW at http://xymus.net/pub/wbtw-v0.4-110-g668b261.tar.gz or by playing a clone of Super Hexagon (mostly made by @ablondin) at http://xymus.net/pub/hex/.

Pull-Request: #1817
Reviewed-by: Jean Privat <jean@pryen.org>
@privat privat merged commit a5de54e into nitlang:master Nov 7, 2015
@xymus xymus deleted the nitcorn-binary branch November 9, 2015 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants