cordova-plugin-file icon indicating copy to clipboard operation
cordova-plugin-file copied to clipboard

Published 20 hours ago • verified publisher icon apache

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

Open salbahra opened this issue 8 years ago • 9 comments
trafficstars

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

  • [X] Reported an issue in the JIRA database
  • [X] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • [X] Added automated test coverage as appropriate for this change.

salbahra avatar Aug 23 '17 18:08 salbahra

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?

janpio avatar Oct 10 '17 13:10 janpio

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

The merge conflicts should be resolved now.

Thanks!

salbahra avatar Oct 12 '17 01:10 salbahra

Any status update on this PR?

salbahra avatar Nov 14 '17 09:11 salbahra

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 avatar Oct 01 '18 13:10 janpio

Rerun tests with current config.

janpio avatar Jul 04 '19 10:07 janpio

CI: Once more please.

janpio avatar Jul 04 '19 17:07 janpio

@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!

salbahra avatar Jul 04 '19 17:07 salbahra

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) 🦀

janpio avatar Jul 04 '19 18:07 janpio

Is this PR abandoned or fixed in another PR?

salbahra avatar Aug 09 '20 04:08 salbahra

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.

salbahra avatar Jul 23 '23 18:07 salbahra