Hi Immi, Yes, this is hideous, I agree. The issue at this point is backwards compatibility -- existing xournal users have .config files with locale-dependent numeric data in it and we'd like to be able to read them even if they upgrade to a future version that's better about storing data in the C locale only.
As far as I know there aren't locales with number formats other than 1.5 or 1,5 but I might be wrong. Anyway, as long as we rely on cleanup_numeric() rather than properly going around locales, it does NOT matter whether we do it when writing the file (in update_keyval() but only for numerical keys as you suggest in your option (a)) or when reading it (in parse_keyval_...() as we currently do) -- because cleanup_numeric() is not locale-aware, it will not treat properly a locale in which 1.5 would be 1'5 and using it at write-time rather than at read-time doesn't fix the issue. Any discrepancy between the locale used at save time and that used at load time is irrelevant since we clean up keys using a fixed algorithm rather than using a locale conversion, and which locale the file is written in only matters for aesthetics, not for real life. (Of course if cleanup_numeric() is replaced by something that does genuine locale conversion rather than the naive algorithm, then it becomes very important to be in the right locale and we can't assume that file written in one locale will be read back in the same locale). But you are right that it would be much cleaner to use (b) or (c) -- print the numbers in the C locale when writing the config file. And any locale-switching needs to be done independently when printing the number, not all around the config file writing code, because there are translated strings in there for the descriptions of the parameters. (By the way, there's similar issues with xoj files -- right now we write it all out in the C locale, but there used to be issues with floating point numbers e.g. stroke coordinates etc. in the xoj file. There's some cleanup_numeric() there too since past versions of xournal were careless about it, and it will have to stay.) Honestly, in my opinion this is not worth fixing for now, until someone actually encounters a problem caused by it. (And I take the view that the config file should be user-editable, since there are many parameters with no UI to modify them; to that effect the comments explaining what each key does is localized and so on. For that reason having numbers be in the user's favorite format is not a stupid idea. Of course I didn't extend the same courtesy to booleans or keywords, e.g. "true"/"false" are not translated.) Best, Denis On 09/13/2016 05:30 AM, Immi Halupczok wrote: > Hi, > > I just noticed that the latest version of xournal (the master branch in > github) stores values in the config file using the current locale, which > means writing "1,5" instead of "1.5" on a German system. I saw that this > behaviour has been intentionally introduced in > > "2b74a01... Fix issues with commas in config file (bug #161)" > > To me, it feels quite dangerous to have the config file format depending > on the locale. Some more remarks: > > - I saw that, when reading the config file, (the new version of) xournal > accepts both, "1.5" and "1,5", independently of the locale. This makes > it far less bad than I thought at the first moment. Nevertheless, I just > run into trouble: I had saved the config with the latest version of > xournal (so numbers got saved as "1,5") and then my old version of > xournal couldn't read it anymore. > > - I think I understood why 2b74a01 has been made: cleanup_numeric() > should not be called for other things than numbers (otherwise one has > bug #161), so since update_keyval() doesn't know whether its argument is > a number, it's much easier to change parse_keyval_**() instead. > > > > One solution would be (a) an additional argument to update_keyval(), > saying whether cleanup_numeric() should be called or not. However, to > me, creating a locale-dependent number and then manually "repairing" it > sounds quite dangerous anyway (who knows whether even stranger locales > exist), so I'd rather suggest to directly create a locale-independent > number. > > I'm not yet sure about the best way to do that: > > (b) Temprorarily switch the locale, while calling g_strdup_printf()? > Using g_strdup_vprintf() instead, we could create our own version of > g_strdup_printf() which does that locale-switching automatically. Right > now this seems like the best solution to me, but maybe I'm overlooking > something. > > (c) Use g_ascii_formatd() instead? I don't see a way to find out how big > the buffer should be we need to allocate; therefore, for the moment, I'd > tend to (a). > > > Any thoughts/opinions about this? I'm willing to implement any of those > solutions if you agree. > > > Best, > Immi > > > > > > ------------------------------------------------------------------------------ > _______________________________________________ > Xournal-devel mailing list > Xournal-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/xournal-devel > -- Denis Auroux UC Berkeley, Department of Mathematics 817 Evans Hall, Berkeley CA 94720-3840, USA aur...@math.berkeley.edu ------------------------------------------------------------------------------ _______________________________________________ Xournal-devel mailing list Xournal-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/xournal-devel
