-
Notifications
You must be signed in to change notification settings - Fork 282
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 user callback for handling invalid characters. #283
base: master
Are you sure you want to change the base?
Conversation
HI,bro @ashtuchkin |
Hey, I think it's a good intention, but it's also a pretty complex topic (that's why I didn't implement it back in the days) and I don't have enough time these days to actively review or discuss design. Maybe in a couple months when the load on my day job decreases.. I don't think your current approach would work because '�' character can be in the input and we don't want to call the callback in that case. |
Hi bro, sincerely speaking, since Github is an open source community, i'd love to contribute, no matter it's easy or hard. I'm just back from the holiday and got the email. But I'm a little confused. If we're just trying to solve #210 , this would work, Now that I'm back, I'm thinking about this PR(or a related set of questions).
May I just throw an error instead of calling the callback when '�' character be inputed? |
I understand that, and I think it's a great intention and a great way to learn. I appreciate that you're doing this. The problem is that it would require time commitment from my side to take this PR to a level at which I would feel comfortable merging it. This issue is more complex than your previous pull request. I don't have a good design in my head that would resolve it in a good way, so to even start code reviewing I would need to spend time thinking about it / designing it. I don't think an easy solution exists here that would not harm baseline performance and would avoid adding unexpected behavior.
No, that would be unexpected behavior for the library users and would potentially break existing use cases. |
More than a month ago, I analyzed almost 40 issues and sorted them into an excel.
what do u think? For the second point, about #253, I think this issue is of great significance. Initially, I'll expose two interfaces (or methods) that allow iconv calls and adds encoding,
I'm not particularly good at reporting this, but I don't think testing is a particularly important work in the early stage. Recently, I have plenty of free time, |
Hope we have this callback to handle invalid characters |
hi buddy,this is work-in progress.
It might not be easy to get the user to write a callback function, but I think we can at least do it first.
I have noticed that there are many issues about callback(#53) at present,
It may be difficult to solve all the problems, but we can take them apart step by step,
So now I have taken the first step (at least we can solve #210 for now),
and the first step now seems easy(or too easy🤣).
First, we declare and tell the user what custom rules can be processed:
Then, the options argument can be passed in when the user calls public API, like:
Finally, we pick it up at decode:
and the encoding is written similarly.
In short, we accept some special rules that user-written callback functions.