Skip to content
This repository has been archived by the owner on Oct 10, 2019. It is now read-only.

Check for iconv() TRANSLIT support #365

Merged
merged 2 commits into from
Oct 2, 2016

Conversation

kobezda
Copy link
Contributor

@kobezda kobezda commented Sep 29, 2016

Some iconv() implementations don't support transliteration.
When iconv with //TRANSLIT is first attempted and fails,
try again without it, and remember the result.

Fixes #364
Fixes #174

Some iconv() implementations don't support transliteration.
When iconv with //TRANSLIT is first attempted and fails,
try again without it, and remember the result.

Fixes akrennmair#364
@@ -16,7 +17,7 @@ namespace newsbeuter {
*/

stfl::form::form(const std::string& text) : f(0) {
ipool = stfl_ipool_create((std::string(nl_langinfo(CODESET)) + "//TRANSLIT").c_str());
ipool = stfl_ipool_create(utils::translit(std::string(nl_langinfo(CODESET)), "WCHAR_T").c_str());
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can encapsulate some string conversion if we add std::string translit(const char* tocode, const std::string& fromcode).

(There's also final c_str() call, but I suspect this is more tricky due to time of life details. I'll deal with this myself later.)

@coveralls
Copy link

coveralls commented Sep 29, 2016

Coverage Status

Coverage increased (+0.04%) to 28.201% when pulling d41345f on kobezda:check-translit-support into fcbe4fd on akrennmair:master.

@coveralls
Copy link

coveralls commented Sep 29, 2016

Coverage Status

Coverage increased (+0.04%) to 28.204% when pulling d0cea1b on kobezda:check-translit-support into fcbe4fd on akrennmair:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 28.204% when pulling d0cea1b on kobezda:check-translit-support into fcbe4fd on akrennmair:master.

@Minoru
Copy link
Collaborator

Minoru commented Sep 29, 2016

We don't have any tests for that part of Newsbeuter, so it's a bit early to call it a day.

Checklist of things I want to do before merging this:

  • build on FreeBSD, check if anything breaks
  • install on my machine, see if anything changes (I have UTF-8 locale so nothing should change for me)

If both of these checks fail to find any problems, I'm going to merge this PR on Sunday, October 1st.

@kobezda, thanks for your work!

@Minoru Minoru merged commit 8a68790 into akrennmair:master Oct 2, 2016
@Minoru
Copy link
Collaborator

Minoru commented Oct 2, 2016

Well, my testing didn't find any problems, so hopefully our users won't run into any either. Thanks, @kobezda!

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

Successfully merging this pull request may close these issues.

3 participants