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

Refactor #20

Merged
merged 5 commits into from
Jun 5, 2020
Merged

Refactor #20

merged 5 commits into from
Jun 5, 2020

Conversation

ccouzens
Copy link
Collaborator

This addresses safety (f203642 #16)

And ergonomics (#17, #18, 6a1948f, ce8303c)

Please see the individual commits for more detail.

This has given me the opportunity to review the safety of the unsafe
code - and it was found lacking.

But it's fixed in the plumbing module.

#16

I've tried to not put naming opinions within the plumbing module.
That is, things within it are named to match the c or c++ libraries of
leptonica and tesseract as much as possible.

This addresses #17 at
least within the plumbing module.
I'm not sure what purpose it served
This gives users a chance to deal with errors, rather than having
panics.
As pointed out in #18
it makes the API nicer.

And I'm breaking the API anyway, in order to return result types, so I may
as well make this further change.
@hdevalence
Copy link

I didn't look carefully at the FFI parts but the Rust API looks nice to use! The only question I have is, if it's mandatory to set a language, should the language string be passed into Tesseract::new rather than by a separate set_lang call?

This was done by initializing on the call to new.

This was suggested by the pull request
#20 by @hdevalence.

This may address #6,
as we now expose datapath.
@ccouzens ccouzens merged commit 1ca5f26 into master Jun 5, 2020
@ccouzens ccouzens deleted the refactor branch January 31, 2021 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants