Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use PosixPath #37

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Use PosixPath #37

wants to merge 1 commit into from

Conversation

georgefst
Copy link
Owner

I'm not quite certain whether we should be using OsPath or PosixPath. The filepath library could perhaps be clearer about the distinction. I think the latter is preferable since this is a Linux-specific library. And I kind of have to use it anyway due to unix (see openFd).

But then doesFileExist and listDirectory want OsPath, hence the conversions here via internal constructors. I think, looking at the file-io library, the intention is that utilities are duplicated for OsPath and PlatformPath, and the problem is maybe just that directory doesn't have the latter for some reason.

We also still have fsnotify, which still uses plain old strings, but will presumably eventually switch to using OsPath not PosixPath since, unlike us, it's cross-platform...

@rmgaray
Copy link

rmgaray commented Aug 19, 2024

There is this blog post by Hasufell that explains the intended use for each type.

I think you got it right: evdev should use PosixString/PosixPath (since it is platform specific) and the problem lies in the libraries that evdev depends on that 1. are platform-agnostic but 2. don't provide a platform-specific interface (based on PlatformString/PlatformPath) to avoid conversions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants