Skip to content

Unicode support on Linux #247

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

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

Conversation

aafemt
Copy link

@aafemt aafemt commented Jun 7, 2025

No description provided.

@irodushka
Copy link
Contributor

Hi @aafemt

I see this's really a huge job.
But...

I don't like so many changes in so many source files. Looks like over-complicated and resembles a patchwork quilt. Sorry.
And this is just an initial work...)

I prefer not to increase the FB ODBC driver code entropy, since it's already very very high.
And the lack of testing tools, as well as near-to-zero test coverage, make such a changes vastly dangerous.
And all this only to support a case that has never been seen since the FB ODBC driver foundation)) i.e. "for ages")

What options can I suggest:

  1. You make a fork of the ODBC driver (concerning all this unicode stuff) and maintain it by your own, occasionally merging a master branch from here to your fork. And I can mention your fork somewhere here.
  2. OR Try to make your changes more systemic. For example, you can create some wrapper/helper class(es) that encapsulate all the charset conversion logic. As the first step, you implement these helpers into the existing codebase (i.e. pure refactoring, without any new functionality). We merge it and test it and it's okay. We can then override these helper layer to implement some new functionality, that will resolve a ticket Unicode support on Linux #247. If we do this carefully, we can switch between legacy and new unicode conversion logic simply with a #define or some feature-flag.
  3. OR Cover all the code you touch with unit tests, for all possible scenarios & platforms, and consider merging your patches as is. Suppose that's a kind of non-science fiction)

Frankly speaking, I prefer option (2).

Regards
Yuri

@aafemt
Copy link
Author

aafemt commented Jul 4, 2025

I will do (3) when have enough time. So far you can consider this branch as (1) to attract testers.

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.

2 participants