-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[path_provider] now supports getDownloadsDirectory() on macOS. #2436
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
I just created flutter/flutter#47851 (couldn't tag you for some reason). We need to federate this plugin as there are many platform specific APIs. If you are interested in this, please feel free to drive it :) For this PR, it might be OK to just add this one API for now since it already most of the current APIs are platform specific. @amirh to make the call here. |
…lugins into path_provider_downloads
This is a very simple change set. When can we expect this to get reviewed and hopefully merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to bump the path_provider version and add a changelog entry as well, other than that LGTM.
Side note: I'm wondering whether it would be useful for this plugin to have a way to dynamically check which directories are supported (vs. trying and seeing if an exception was thrown). My thinking is that it's probably more convenient to write app code to branch on whether a given directory is supported vs. which platform we're running on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry marked approved instead of request changes - we need to update the version for path_provider before landing
Thanks for merging! When is going to be published on https://pub.dev/packages/path_provider ? |
I see it has been published now. Thanks again! |
…er#2436) Adds support for retrieving a user's downloads directory on macOS using the path_provider plugin.
let me know if I am doing anything wrong |
…er#2436) Adds support for retrieving a user's downloads directory on macOS using the path_provider plugin.
If you are on |
Yes, try to
Then it should work. |
@technolion I think you misunderstood my comment. @Faiizii is seeing an error message, and is thus running something. But it's on |
You are right @stuartmorgan. I got confused about by the error message talking about macOS. |
Unless @Faiizii is using a third-party method-channel-based unendorsed implementation of the plugin, that PR will just replace one error method with a different one. There are no implementations in flutter/plugins that are currently gated behind that check.
As a starting point, it would need to have the test failures addressed. Then it needs a review; there's a large backlog and that PR is low priority since it doesn't fix any current issue (although it could avoid issues in the future, and is the right change to make). |
can we get a similar feature for |
Description
This PR adds support for retrieving a user's downloads directory on macOS using the path_provider plugin. The native code is done in the sub project
path_provider_macos
.Related Issues
Fixes flutter/flutter#47676
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.///
).flutter analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?