-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[RFC] Unify and simplify Iconv implementation (LibC vs LibIconv) #11882
Comments
I object to @HertzDevil's comment #11876 (comment)
Renaming the lib type in these files changes nothing about the target of these bindings. They still refer to the same system library symbols. Maybe a minor argument could be made about defining system library bindings on a lib type named |
Not if
|
The purpose of This would be the require code for iconv with #11876 and this issue resolved: {% if flag?(:use_libiconv) || flag?(:win32) %}
require "./lib_iconv"
{% else %}
require "c/iconv"
{% end %} |
What would this look like if we reuse the |
This is a recap of a side conversation started in #11876:
@asterite commented:
Followed by this comment:
And confirmation from @HertzDevil in this comment:
I think what @asterite was suggesting is really remove all
lib_c/*/c/iconv.cr
and get everything unified inlib_iconv.cr
. Except for certain platforms (mentioned before), they all share the same Lib interface definition.Then the only difference if is that is exposed by the libc of that platform or not (Eg musl iconv vs having libiconv explicit linked).
Still not sure the best approach on this (not fully grasped all the needed changes), so decided to move this out of the PR so we can keep track of all the comments.
Thank you!
❤️ ❤️ ❤️
The text was updated successfully, but these errors were encountered: