os: add temporary notice about changing XDG user dirs related fns behavior#26301
os: add temporary notice about changing XDG user dirs related fns behavior#26301gechandesu wants to merge 3 commits into
Conversation
|
Why? This is V. Add a trailing struct param with a bool that tells the code whether or not to create the dir for you. You could add that right away, with the default That is the least breaking way to handle it, while still allowing for the other behavior for those that want it. |
|
If we want to create a directory, even if it's optional, we need to change the function signature. It would be better to return Result to avoid panicking (which can't be handled now) and also to avoid hiding the error. Therefore, I prefer to remove the directory creation entirely, leaving only the path return, as is common. Another way is to add new functions, but it can be difficult to find good names for them and this will also clutter the public API. |
|
I do not see the value of not creating the folders. Having a panic at the end, is not enough for me, and I would like to see a concrete example, where that was a problem for you. |
|
I don't have a real-world example where this would cause problems. If you're looking for these paths, you probably want to write or read something there, so it all seems logical. I also looked closely at the possible errors that would trigger a panic. The list isn't too long, and they're either strange (for example, not having read or write permissions to your home directory), unrealistic (a path longer than 4 KB), or extreme cases where normal application operation is unlikely. These cases are probably truly worthy of a panic, but that doesn't change the fact that the panic is occurring in an unexpected place. I believe an API should provide the ability to handle any errors instead of panicking unconditionally where the library author deems it appropriate. My motivation for this PR is to rid the API of actions that might be unexpected. As I wrote in the issue, it's usually expected to simply retrieve a path, not immediately create it. If you're not convinced, please close the PR; I'll add documentation for these functions in a new PR. |
|
@gechandesu thank you 🙇🏻 . The intended use of the affected APIs, is to return paths, that are ready to be used, without further preparations/decisions, saving each app from having to re-implement some non trivial logic at each callsite. I think documenting it (and that they can panic, if the corresponding folders do not exist, and could not be created) is a better path forward, from an ergonomic point of view (as well as not breaking backwards compatibility), so I will close this PR. That said, I do think, that we can add a new set of APIs, that just return strings, formed by adding a Those new APIs, can then work based on just string manipulation, without attempting to create or do any filesystem manipulations (that can panic), similar to how the python library you linked in the issue (when it is used without the I also noticed, that the rust library I am not worried about API clutter, since the new ones will use a common suffix, and because both the existing ones and the new ones, would be useful in different situations (brevity, vs wanting more explicit control over what is created/when), and because the compiler will just trim the ones, that are not used in any given application. The overhead would be just the normal parsing/checking time for a few more lines of code. |
Part 1 of #26300
Added a notice about the behavior change for now. I plan to change the behavior in another PR in a month.