search

apache / cordova-plugin-file

Apache Cordova File Plugin
https://cordova.apache.org/
Apache License 2.0
741 stars 756 forks source link

fix(windows): `readAsBinaryString` only reads the requested range (CB-13208) #216

Closed salbahra closed 10 months ago

salbahra commented 6 years ago

Platforms affected

Windows

What does this PR do?

Change readAsBinaryString on FileReader for Windows to use a stream which seeks to the start position and only reads until the requested end instead of reading the entire file to a buffer then slicing the requested fragment.

What testing has been done on this change?

Plugin change has been tested on Windows as that's the only platform affected.

Checklist

janpio commented 6 years ago

Did I get this right that this basically adds an additional step in between where only the requested part is read into buffer?

There seems to be a conflict now, can you fix that by rebasing your changes?

salbahra commented 6 years ago

That is correct, I am using the other read methods as a template.

The merge conflicts should be resolved now.

Thanks!

salbahra commented 6 years ago

Any status update on this PR?

janpio commented 5 years ago

Hey, I just fixed the problem that caused Android tests to fail in master. Could you rebase this PR please? This should get rid of the Android failures and possibly fix all test failures for this PR.

janpio commented 4 years ago

Rerun tests with current config.

janpio commented 4 years ago

CI: Once more please.

salbahra commented 4 years ago

@janpio Sorry for the delay but just noticed your request. I merged apache/master into my fork which has updated this PR. Let me know if there is anything else I can do to help with.

Thanks!

janpio commented 4 years ago

Oh sorry, this wasn't even a request to you - I just have to close, then reopen the PR to trigger a CI rebuild 🐒 So I was talking to myself or CI (which doesn't listen of course, but rebuilds becaus of the open/close dance I was doing) 🦀

salbahra commented 3 years ago

Is this PR abandoned or fixed in another PR?

salbahra commented 10 months ago

I take this PR will never be merged. Going to close it to remove it from my list of open PRs. This is an issue but I assume the platform has been abandoned.