[path_provider] Document null vs UnsupportedError return semantics#11793
[path_provider] Document null vs UnsupportedError return semantics#11793theprantadutta wants to merge 2 commits into
Conversation
The dartdoc on getDownloadsDirectory previously only mentioned the UnsupportedError case, but the function also returns null when a platform supports the concept of a downloads directory but no such directory is currently available (for example, Linux without xdg-user-dir installed, or when xdg-user-dir fails when called). Document both return-value semantics on the public API method in path_provider, and mirror the implementation contract on PathProviderPlatform.getDownloadsPath in path_provider_platform_interface. Wording follows the explanations @stuartmorgan-g and @Hixie gave in the issue thread (flutter/flutter#143238). Fixes flutter/flutter#143238
There was a problem hiding this comment.
Code Review
This pull request documents the difference between a null return and an UnsupportedError for getDownloadsDirectory and getDownloadsPath across the path_provider and path_provider_platform_interface packages. The review feedback points out a formatting issue in the CHANGELOG.md of path_provider, where inserting a new bullet point accidentally changes the nesting hierarchy of an existing sub-bullet point.
| * Documents the difference between a `null` return and an `UnsupportedError` | ||
| from `getDownloadsDirectory`. |
There was a problem hiding this comment.
Inserting this bullet point here accidentally changes the nesting of the subsequent bullet point. The sub-bullet * Applications built with older versions of Flutter... (on lines 8-9) was meant to be nested under * Updates README..., but now it is nested under this new bullet point.
To preserve the correct hierarchy, please move this new bullet point to the end of the ## NEXT section (after the nested bullet point) or place it before the * Updates README... bullet point.
|
Thanks for the contribution! In the future, please do not delete the checklist that is in the PR template; it is there for a reason. This PR is missing required elements described in the checklist, which need to be addressed before it moves forward with review. I am marking the PR as a Draft. Please re-add and complete the checklist, updating the PR as appropriate, and when that’s complete please feel free to mark the PR as ready for review. Also, in the future please do not ping people in PR descriptions; it's extremely disruptive because it not only pings them on the PR (which is not constructive), it pings them again when the PR lands, and when it's mirrored into forks.
This is part of a different repository's template, which suggests that you used AI to create this entire PR description. Please don't do that. We have a template. |
- Fix CHANGELOG.md nesting so the existing 'Applications built with older versions...' sub-bullet stays correctly nested under the README bullet (gemini-code-assist bot finding). - Bump path_provider 2.1.5 -> 2.1.6 and path_provider_platform_interface 2.1.2 -> 2.1.3, replacing the NEXT heading with the new version per the flutter/packages CHANGELOG conventions for continuous releases.
|
Thanks for the careful review — all addressed:
Marking ready for review. |
There was a problem hiding this comment.
Code Review
This pull request updates the documentation for getDownloadsDirectory in path_provider and getDownloadsPath in path_provider_platform_interface to clarify the behavior and implementation contracts for returning null versus throwing an UnsupportedError. It also bumps the package versions to 2.1.6 and 2.1.3 respectively, and updates their changelogs. There are no review comments, and I have no feedback to provide.
Updates the dartdoc for
getDownloadsDirectoryinpath_providerand the matchinggetDownloadsPathcontract onPathProviderPlatformto explain both return-value cases:null— the platform supports the concept of a downloads directory but no such directory is currently available (concrete example: Linux withoutxdg-user-dirinstalled, or wherexdg-user-dirfails when called).UnsupportedError— the current platform has no concept of a downloads directory at all.Previously the dartdoc only mentioned the
UnsupportedErrorcase, which left users unsure when they might see anullresult.The wording matches the explanations already given by maintainers in the linked issue thread.
Fixes flutter/flutter#143238
Pre-Review Checklist
[shared_preferences]///).This is a documentation-only change to public dartdoc comments. No code behavior changes; no new tests are required per the auto-exempt rules for changes that only affect comments/documentation.