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

feat: add char-based API #13

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

Conversation

mkroening
Copy link
Contributor

Hi Tatsunori,

It's been a while since I have worked on this. I prepared this code more than a year ago, but never got around to submitting it properly.
This API introduces newtypes around u8s for working with these code points.

Thanks for this crate! :)

Closes #10

Signed-off-by: Martin Kröning <martin.kroening@eonerc.rwth-aachen.de>
@tats-u
Copy link
Owner

tats-u commented Jun 6, 2024

@mkroening Do we have to improve build.rs?
I don't think we should continue to expose the raw tables & hashmaps IMHO anymore. (#12)
You can hide them from users if you want to.

Comment on lines +174 to +191
Cp437(Complete, ENCODING_TABLE_CP437, DECODING_TABLE_CP437),
Cp720(Complete, ENCODING_TABLE_CP720, DECODING_TABLE_CP720),
Cp737(Complete, ENCODING_TABLE_CP737, DECODING_TABLE_CP737),
Cp775(Complete, ENCODING_TABLE_CP775, DECODING_TABLE_CP775),
Cp850(Complete, ENCODING_TABLE_CP850, DECODING_TABLE_CP850),
Cp852(Complete, ENCODING_TABLE_CP852, DECODING_TABLE_CP852),
Cp855(Complete, ENCODING_TABLE_CP855, DECODING_TABLE_CP855),
Cp857(Incomplete, ENCODING_TABLE_CP857, DECODING_TABLE_CP857),
Cp858(Complete, ENCODING_TABLE_CP858, DECODING_TABLE_CP858),
Cp860(Complete, ENCODING_TABLE_CP860, DECODING_TABLE_CP860),
Cp861(Complete, ENCODING_TABLE_CP861, DECODING_TABLE_CP861),
Cp862(Complete, ENCODING_TABLE_CP862, DECODING_TABLE_CP862),
Cp863(Complete, ENCODING_TABLE_CP863, DECODING_TABLE_CP863),
Cp864(Incomplete, ENCODING_TABLE_CP864, DECODING_TABLE_CP864),
Cp865(Complete, ENCODING_TABLE_CP865, DECODING_TABLE_CP865),
Cp866(Complete, ENCODING_TABLE_CP866, DECODING_TABLE_CP866),
Cp869(Complete, ENCODING_TABLE_CP869, DECODING_TABLE_CP869),
Cp874(Incomplete, ENCODING_TABLE_CP874, DECODING_TABLE_CP874),
Copy link
Owner

@tats-u tats-u Jun 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't like to maintain these Complete/Incomplete and C[Pp]* written by hand if possible.
build.rs automatically generates ENCODING_TABLE_* and DECODING_TABLE_*.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so we might be able to move this macro call into build.rs. Is that what you had in mind?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that what you had in mind?

Right. write_* functions are for code generation and you have only to modify (some of) them and maybe generate_tables.
I wonder if it's possible.

@tats-u
Copy link
Owner

tats-u commented Jun 6, 2024

Our main dishes are (*DECODING_TABLE_CP_MAP).get and (*ENCODING_TABLE_CP_MAP).get.
The use of an individual code page is just a bonus.
The StringExt should be activated only by those who want to use individual code pages.

@mkroening
Copy link
Contributor Author

Our main dishes are (*DECODING_TABLE_CP_MAP).get and (*ENCODING_TABLE_CP_MAP).get.

Okay, we can make them private in favor of this API or what did you have in mind?

The StringExt should be activated only by those who want to use individual code pages.

What do you mean? A user can choose not to use StringExt. Or do you want to feature-gate it?

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.

Provide char-based API
2 participants