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

Allow to set more options #1

Merged
merged 2 commits into from
Aug 5, 2023
Merged

Conversation

h3ndrk
Copy link
Contributor

@h3ndrk h3ndrk commented Aug 5, 2023

Thanks for the great bindings library!

For my project, I need more control over the options of the phoneme generation. Therefore this PR adds more options to the PhonemeGenOptions::Standard enum variant.

This repository seems to not be formatted according to standard rustfmt which might introduce some unchanged lines that were not intended. Please consider to reformat this repository to make contributing easier. I did not format the lib.rs to reduce the number of change lines in this PR. 😃

Feel free to discuss or propose alternatives for setting the options more easily from the outside. Thanks in advance!

@GnomedDev
Copy link
Owner

Right then, I've formatted the repo using rustfmt and updated some dependencies, can you rebase?

@h3ndrk
Copy link
Contributor Author

h3ndrk commented Aug 5, 2023

Thanks for formatting! 🎉

The files in tests/ also needed formatting. I did that in another commit, but I can drop it if you want. 😃

Should be rebased.

@GnomedDev
Copy link
Owner

Thanks, although I have concerns that most of these text modes are going to be unusable due to the fact that &str is always UTF8 encoded?

@h3ndrk
Copy link
Contributor Author

h3ndrk commented Aug 5, 2023

That might be true. I only added all available variants without testing.

I also thought whether we want to exclude Mbrola because there is a dedicated enum variant in PhonemeGenOptions. 🤔

Do you want to be close to upstream or only expose what is implemented and provide a single way to do things?

@GnomedDev
Copy link
Owner

I want to smooth over the weirdness in the C api, so I'd prefer to only expose what works.

@h3ndrk
Copy link
Contributor Author

h3ndrk commented Aug 5, 2023

Fine for me. Better now? Only UTF-8 is supported in the TextMode because the others might use ISO-8859 or some wide character formats.

@GnomedDev
Copy link
Owner

Looks good to me!

@GnomedDev GnomedDev merged commit 9db5670 into GnomedDev:master Aug 5, 2023
@h3ndrk h3ndrk deleted the more-options branch August 5, 2023 20:48
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