Skip to content

Explicitly set C default library locale#1283

Merged
dictoon merged 2 commits intoappleseedhq:masterfrom
glebmish:master
Mar 4, 2017
Merged

Explicitly set C default library locale#1283
dictoon merged 2 commits intoappleseedhq:masterfrom
glebmish:master

Conversation

@glebmish
Copy link
Copy Markdown
Contributor

@glebmish glebmish commented Mar 4, 2017

QApplication sets application locale to user's locale when created.
It can lead to erros with decimal point when parsing .obj files.
To prevent this C locale must be explicitly set right after QApplication
is built.

QApplication sets application locale to user's locale when created.
It can lead to erros with decimal point when parsing .obj files.
To prevent this C locale must be explicitly set right after QApplication
is built.
Copy link
Copy Markdown
Member

@dictoon dictoon left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! A few easy changes are required before it's good to merge. You can just push a new commit, it'll automatically get appended to this PR.

#include <cstdio>
#include <cstdlib>
#include <cstring>
#include <clocale>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We require that #include clauses are always sorted alphabetically. This helps preventing duplicate includes.

g_previous_message_handler = qInstallMsgHandler(message_handler);

QApplication application(argc, argv);
// QApplication sets C locale to user's locale, we need to fix this
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

May I suggest to move this after the setup of QApplication.

QApplication::setWindowIcon(QIcon(make_app_path("icons/appleseed.png")));

// The locale must be set after the construction of QApplication.
QLocale::setDefault(QLocale::C);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oops, you removed this, but we do need it: the Qt locale must be also set. Please keep this and just add your call after it.

@dictoon dictoon merged commit 617129f into appleseedhq:master Mar 4, 2017
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