Discussion:
[LyX/master] Set the default locale at startup.
Georg Baum
2014-07-10 20:33:14 UTC
Permalink
commit 82faa6619239c2e57fba9128899bafe29d728e51
Date: Wed Jun 11 18:23:44 2014 +0200
Set the default locale at startup.
On startup, the default locale is "C", meaning that all system
functions assume an ascii codeset. The environment's locale
settings should be selected by calling setlocale(LC_ALL,"").
This is done by Qt during the QCoreApplication initialization
but this inizialization is never performed for batch processing
and, as a result, LyX is not able to process files whose names
contain non-ascii characters. This is not an issue on Windows,
where the file names are always stored as UTF-16, so the call is
only performed for unix-like platforms (this also includes cygwin,
due to its own filenames management that allows using characters
which are forbidden to native programs).
diff --git a/src/support/os_cygwin.cpp b/src/support/os_cygwin.cpp
index 92cbf15..4179d49 100644
--- a/src/support/os_cygwin.cpp
+++ b/src/support/os_cygwin.cpp
@@ -215,6 +215,9 @@ void init(int argc, char * argv[])
argc_ = argc;
argv_ = argv;
+ // Set environment's default locale
+ setlocale(LC_ALL, "");
+
// Make sure that the TEMP variable is set
// and sync the Windows environment.
setenv("TEMP", "/tmp", false);
diff --git a/src/support/os_unix.cpp b/src/support/os_unix.cpp
index b85bdb2..03dfb38 100644
--- a/src/support/os_unix.cpp
+++ b/src/support/os_unix.cpp
@@ -46,6 +46,9 @@ void init(int argc, char * argv[])
{
argc_ = argc;
argv_ = argv;
+
+ // Set environment's default locale
+ setlocale(LC_ALL, "");
}
This breaks the tex2lyx tests with a german locale, probably since
support::convert<double>(std::string const s) wants now a comma instead of
a period as decimal point, so the string "0.3359375" gets now converted to
0 instead of 0.3359375. Other conversions are probably broken as well, and
unfortunately this is also in 2.1.1.

The question is: Do we need two versions of support::convert() (one taking
the current locale into account for user inout/output, and one always using
the "C" locale for internal conversions), or was the old behaviour correct?


Georg
Jean-Marc Lasgouttes
2014-07-10 21:25:13 UTC
Permalink
Post by Georg Baum
This breaks the tex2lyx tests with a german locale, probably since
support::convert<double>(std::string const s) wants now a comma instead of
a period as decimal point, so the string "0.3359375" gets now converted to
0 instead of 0.3359375. Other conversions are probably broken as well, and
unfortunately this is also in 2.1.1.
The question is: Do we need two versions of support::convert() (one taking
the current locale into account for user inout/output, and one always using
the "C" locale for internal conversions), or was the old behaviour correct?
Yes, we are again in locale hell... What about adding a
setlocale(LC_NUMERIC, "C")
? Would that be enough?

JMarc
Enrico Forestieri
2014-07-11 09:07:44 UTC
Permalink
Post by Jean-Marc Lasgouttes
Post by Georg Baum
This breaks the tex2lyx tests with a german locale, probably since
support::convert<double>(std::string const s) wants now a comma instead of
a period as decimal point, so the string "0.3359375" gets now converted to
0 instead of 0.3359375. Other conversions are probably broken as well, and
unfortunately this is also in 2.1.1.
The question is: Do we need two versions of support::convert() (one taking
the current locale into account for user inout/output, and one always using
the "C" locale for internal conversions), or was the old behaviour correct?
Yes, we are again in locale hell... What about adding a
setlocale(LC_NUMERIC, "C")
? Would that be enough?
I bet on it.
--
Enrico
Enrico Forestieri
2014-07-11 09:04:36 UTC
Permalink
Post by Georg Baum
This breaks the tex2lyx tests with a german locale, probably since
support::convert<double>(std::string const s) wants now a comma instead of
a period as decimal point, so the string "0.3359375" gets now converted to
0 instead of 0.3359375. Other conversions are probably broken as well, and
unfortunately this is also in 2.1.1.
I don't know how to run those tests and, most probably, also packagers.
So, this is not a big deal.

As regards other possible brekages, I only see one instance of
convert<double> in the tex2lyx sources, namely in scale_as_percentage(),
and this is about converting a scale parameter for fonts. So, I tried
to import the attached latex document with either 2.1.0 and 2.1.1.
Both were broken for different reasons. The 2.1.0 version produced
a line "\font_sf_scale 92.5" which was later rejected as an integer
and treated as 0. The 2.1.1 version directly produced 0, thus avoiding
a warning ;-)

So, it seems there is no regression...
--
Enrico
Kornel Benko
2014-07-11 09:43:36 UTC
Permalink
Post by Enrico Forestieri
Post by Georg Baum
This breaks the tex2lyx tests with a german locale, probably since
support::convert<double>(std::string const s) wants now a comma instead of
a period as decimal point, so the string "0.3359375" gets now converted to
0 instead of 0.3359375. Other conversions are probably broken as well, and
unfortunately this is also in 2.1.1.
I don't know how to run those tests and, most probably, also packagers.
So, this is not a big deal.
As regards other possible brekages, I only see one instance of
convert<double> in the tex2lyx sources, namely in scale_as_percentage(),
and this is about converting a scale parameter for fonts. So, I tried
to import the attached latex document with either 2.1.0 and 2.1.1.
Both were broken for different reasons. The 2.1.0 version produced
a line "\font_sf_scale 92.5" which was later rejected as an integer
and treated as 0. The 2.1.1 version directly produced 0, thus avoiding
a warning ;-)
So, it seems there is no regression...
I am not so sure. Running tests under ctest, some of them fail, (and I don’t remember to fail them earlier).
Setting LC_ALL to "C" prior to the tests, they all pass.

Kornel
Georg Baum
2014-07-11 19:27:04 UTC
Permalink
Post by Enrico Forestieri
Post by Georg Baum
This breaks the tex2lyx tests with a german locale, probably since
support::convert<double>(std::string const s) wants now a comma instead
of a period as decimal point, so the string "0.3359375" gets now
converted to 0 instead of 0.3359375. Other conversions are probably
broken as well, and unfortunately this is also in 2.1.1.
I don't know how to run those tests and, most probably, also packagers.
So, this is not a big deal.
This is not the point. The point is that there was a regression in the code
which did not exist before. This regression was made visible by the tests
(and if you want to know how to run them: see section 3 of
lib/doc/Development.lyx).
Post by Enrico Forestieri
As regards other possible brekages, I only see one instance of
convert<double> in the tex2lyx sources, namely in scale_as_percentage(),
and this is about converting a scale parameter for fonts. So, I tried
to import the attached latex document with either 2.1.0 and 2.1.1.
Both were broken for different reasons. The 2.1.0 version produced
a line "\font_sf_scale 92.5" which was later rejected as an integer
and treated as 0. The 2.1.1 version directly produced 0, thus avoiding
a warning ;-)
I am not sure whether restricting scale values to integers is a good idea.
Scales are inherently floating point values, and LaTeX uses floating point
values as well. Therefore, LyX should be fixed to accept floating point
values for all font scale parameters, and the integer restriction in tex2lyx
should be reverted.
Post by Enrico Forestieri
So, it seems there is no regression...
There was a regression, see the attached diff (which I should have attached
yesterday, but forgot). It might not be caused by convert<double>, but
tex2lyx created valid LyX files before the change, and after the change they
were still valid, but the contents was not in line with the TeX file
anymore. After your fix every test is fine again.


Georg
Enrico Forestieri
2014-07-11 23:43:22 UTC
Permalink
Post by Georg Baum
After your fix every test is fine again.
Good.
--
Enrico
Loading...