Rebase of sd/improvements #27
Conversation
on windows it can also be \r\n
Best or worst idea ever!? lets find out... Pro: - we guarantee to read all these files, so we should test them - better coverage Con: - Very Slow - Packages should be removed if not added?! should we give an option to install all IO packages anyways!? Or just put them all in the REQUIRE?
@dreammaker, you think that's sufficient?
Does that seem like a reasonable fix? It fixes my problems on windows, but I'm not sure if there are any strings attached.
this seems even shadier...But windows does not guarantee \r\n.... So I'm not sure how to best handle this. Maybe we should just drop windows support entirely :D
I know there are also `\r` in magic bytes... but if we just ignore it alltogether we might be fine!? I'd be suprised if this won't byte us at some point....Bad pun intended.
…ta package and/or falls out of PkgEval
|
I don't really know why appveyor isn't picking up this PR, but it seems to work fine for master... |
|
I'm inclined to merge this and tag FileIO. I'm not sure if I forgot anything, but most of the problems should now be in external packages. Images will take some more time but if we tag now we can at least advance with Meshes. |
| @@ -254,7 +258,7 @@ function query(filename::AbstractString) | |||
| if haskey(ext2sym, ext) | |||
| sym = ext2sym[ext] | |||
| len = lenmagic(sym) | |||
| if length(len) == 1 && (all(x->x==0, len) || !isfile(filename)) # we only found one candidate and there is no magic bytes, or no file, trust the extension | |||
| if length(len) == 1 && (((all(x->x==0, len) || !isfile(filename))) || !isfile(filename)) # we only found one candidate and there is no magic bytes, or no file, trust the extension | |||
There was a problem hiding this comment.
Parenthesis are not needed here.
|
@timholy any objections to merging this? I think we are antsy to get MeshIO published. Semver should allow us to tag and if we need to make any API changes it can bump the minor version. Anything that gets registered can come in as a patch version bump. |
| @@ -254,7 +258,7 @@ function query(filename::AbstractString) | |||
| if haskey(ext2sym, ext) | |||
| sym = ext2sym[ext] | |||
| len = lenmagic(sym) | |||
| if length(len) == 1 && (all(x->x==0, len) || !isfile(filename)) # we only found one candidate and there is no magic bytes, or no file, trust the extension | |||
| if length(len) == 1 && ((all(x->x==0, len) || !isfile(filename)) || !isfile(filename)) # we only found one candidate and there is no magic bytes, or no file, trust the extension | |||
There was a problem hiding this comment.
I'm confused about this line: aren't you effectively doing isfile(filename) twice?
There was a problem hiding this comment.
Uhm, I thought I cleaned that up... Definitely wrong :D
|
No objections here, merge when ready. |
…hrase Rebase of sd/improvements
Also try to make CI pass so we can get closer to publishing this.