[path_provider] Android implementation of getDownloadsDirectory
#4559
Conversation
There was a problem hiding this comment.
Making the change this way adds complexity, and a breaking change we don't want. The better option is to:
- Wait until 2.8 or later is on
stable - Switch Android to Dart+internal-method-channel implementation (see [path_provider] Switch macOS to an internal method channel #4547)
- Make getDownloadsDirectory redirect at the Dart level to
getExternalStorageDirectories(which despite the legacy name no longer calls that Android method) with the downloads directory constant.
| @@ -242,6 +256,10 @@ private String getPathProviderStorageDirectory() { | |||
| return dir.getAbsolutePath(); | |||
| } | |||
|
|
|||
| private String getPathProviderDownloadsDirectory() { | |||
| return Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS).getAbsolutePath(); | |||
There was a problem hiding this comment.
This is deprecated; we deliberately do not use it for that reason.
| @@ -84,8 +84,8 @@ class MethodChannelPathProvider extends PathProviderPlatform { | |||
|
|
|||
| @override | |||
| Future<String?> getDownloadsPath() { | |||
| if (!_platform.isMacOS) { | |||
| throw UnsupportedError('Functionality only available on macOS'); | |||
| if (_platform.isIOS) { | |||
There was a problem hiding this comment.
This is actually a breaking change. These packages aren't atomic, and the layering here is problematic (these checks should really never have been in the platform interface), so it would be possible to have a version of the Android package that doesn't support it, but the new version of the platform interface that removed the check, and as a result get a different error type.
|
2.8 has been released to stable, so this could proceed as described above. Also, you'll need to fix your commits to use the same email address as the one you used to sign the CLA (which appears to be the one associated with your GitHub account). |
I created a new PR (from a new GitHub account) focusing only on switching Android to an internal method channel: #4617 I just copied the same steps you did on your MacOS PR (except for the directory creation part). What else do you think it would need? Perhaps @gaaclarke could review the TODO that he left in for when flutter/engine#29147 was in stable (which I believe it is). Just wanted you to know I'm on it and if anybody is interested in participating they can do so through #4617 instead of creating a new PR. |
Probably nothing; I'll take a look.
That's unrelated to the Dart side of the method channel, so would be best handled in a different PR. |
|
Are you planning on updating this to do step 3 of #4559 (review)? |
|
Thank you for your contribution. I'm going to close this PR for now since there are outstanding comments, just to get this off our PR review queue. Please don't hesitate to submit a new PR if you have the time to address the review comments. Thanks! |
This PR adds an Android implementation for the getDownloadsDirectory method in the path_provider interface, returning the public Android downloads directory path.
Resolves issue flutter/flutter#93198: [path_provider] getDownloadsDirectory() doesn't work for iOS and Android.
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.