Skip to content

feat: Add utility to convert OsStr to CString #136

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

Merged
merged 7 commits into from
Jun 8, 2019

Conversation

ralphtheninja
Copy link
Collaborator

Flubs awesome string conversion + fmt.

Floris Bruynooghe and others added 7 commits June 8, 2019 00:43
This is approach seems acceptable in the context of deltachat, it
should work correctly on unix and on Windows requires paths to be
valid utf-8.
- Adds an OsStrExt impl for Path directly, a little more convenience.

- Fix the windows code to actually use the right function name.  Test
  the impl function on unix too since that was the point of having it
  implemented in a separate function to begin with.

- Improve the docs, do hyperlinks a bit better.
Having your compiler in the cloud is just painful.
When PRs are made from forks the passwords are unavailable.  We don't
want CI to fail because of this.
This means any type with implements this trait will get this
implementation, thus covering both OsStr and Path instead of having
duplicate implementations for those like before.
@ralphtheninja ralphtheninja changed the title Flub os str to c string feat: Add utility to convert OsStr to CString Jun 7, 2019
@ralphtheninja
Copy link
Collaborator Author

@flub Please feel free to push directly to the main repo when making PRs. Makes it a little bit easier for others to tweak in case you're not around. I really want this change in 😉

@flub
Copy link
Contributor

flub commented Jun 8, 2019

Fair enough, I always feel weird using privileges others don't have though. And it did surface the fact that CI had a bug for PRs from other repos :)

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