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

Port string_char_to_byte and string_byte_to_char #1534

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

agraven
Copy link
Collaborator

@agraven agraven commented Aug 6, 2019

This is a more or less direct port of the C code. I experimented with using if expressions directly in the assignments of best_above and friends, but the two conditionals created enough visual noise to reduce readability.

I moved the statics for caching over to rust, but I wasn't sure if I should use lazy_static or not since they don't need to be initialized at runtime/first use.

I also added some convenience functions for using char_head_p, multibyte_length_by_head and multibyte_char_at on strings, and I'm not entirely sure how the multibyte_char_at one should look with regards to (absence of) error handling.

The PR is based on the lisp_string_width PR as it's the only place in the rust code the index conversion functions are used, and additionally I want to remove all invocations of C code from lisp_string_width to enable wring rust tests.

On the subject of tests, none are in this PR yet; I intend to add some ASAP.

agraven and others added 8 commits April 20, 2019 17:59
Co-Authored-By: Nick Drozd <nicholasdrozd@gmail.com>
I forgot github suggestions only change one line, whoops.
Additional changes:
* Port clear_string_char_byte_cache
* Add head_len, is_head_at and char_and_len to LispStringRef
rust_src/src/multibyte.rs Outdated Show resolved Hide resolved
@agraven
Copy link
Collaborator Author

agraven commented Jan 10, 2020

As the lisp_string_width PR has been merged, I plan on finishing this PR sometime soon

src/fns.c Show resolved Hide resolved
@agraven
Copy link
Collaborator Author

agraven commented Jan 14, 2020

Unless anyone has any additional feedback, I think this PR is ready for merge

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

Successfully merging this pull request may close these issues.

None yet

2 participants