From: SourceForge.net <no...@so...> - 2012-06-29 12:01:51
|
Bugs item #3536888, was opened at 2012-06-21 05:41 Message generated for change (Comment added) made by oehhar You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=3536888&group_id=10894 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: 30. msgcat Package Group: current: 8.5.11 Status: Closed Resolution: Fixed Priority: 5 Private: No Submitted By: Harald Oehlmann (oehhar) Assigned to: Harald Oehlmann (oehhar) Summary: Locale guessing of msgcat fails on (some) Windows 7 Initial Comment: I experienced on a german-swiss windows 7 computer, that : <session on tcl 8.5.11> % package require msgcat 1.4.4 % msgcat::mcpreferences en_us en {} </session> (the last result should be "de_ch de {}") The reason for that is, that the language is detected observing the registry key: [HKEY_CURRENT_USER\Control Panel\International\Locale] It contains a numerical Language ID (LCID). Possible values are in the registry subtree: [HKEY_LOCAL_MACHINE\SOFTWARE\Classes\MIME\Database\Rfc1766] Extract: 0409 en_us <- this is set 0407 de <- this might be acceptable 0807 de_ch <- this would be correct The observed value is "0409" which is en_us. The correct value would be "de_ch". Still acceptable is "de". I found other reports, that this registry key is not reliable using windows 7: http://social.technet.microsoft.com/Forums/en/w7itproinstall/thread/8597901e-47a8-4457-a8dc-653a260808b3 following the links at the end http://blogs.msdn.com/b/michkap/archive/2010/03/19/9980203.aspx it is proposed, that: - there is a new key (since Vista I think) which contain directly the IETF language tag (which is close to the locale): [HKEY_CURRENT_USER\Control Panel\International\LocaleName] In this case, this would help. The contained value "de-CH" is correct. ---- Anyway, the recommended method is to use the system call: GetSystemDefaultLCID() and from Vista on: GetUserDefaultLocaleName() --- Two possible methods to solve the issue: 1) extend msgcat to first look to LocaleName This is officially not supported but would solve in this case The registry entry "LocaleName" is seen as more reliable, as all modern APIs use this instead of CLID's. A patch for this case is attached. 2) extend tcl to return the current system locale by an api This requires binary code ---------------------------------------------------------------------- >Comment By: Harald Oehlmann (oehhar) Date: 2012-06-29 05:01 Message: Thank you for the discussion. My point is not speed but clearty. I don't like to do a "package require" in cases where I know it will fail and thus I will pollute the ::errorInfo without real error. I don't like [info sharedlibextension] to much, as we use a side effect to detect the windows platform. Is there no other way, to detect the windows platform ? If not, I would love to have this in the platform package. Thats why I asked Jan to add a comment and thats what he did. Wiki page http://wiki.tcl.tk/1649 still states, that cygwin tcl returns platform "windows", but this was changed as far as I know. Could someone add/correct this page in respect to Cygwin ? Further development of msgcat: I neither like, that the locale search in Init throws an error to report a non-matching locale. This may pollute ::errorInfo too. IMHO this is ok for user code. Packages should not do that. Just 2 cents, Harald ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-06-29 04:42 Message: Did a quick compare with the table in the registry: HKEY_LOCAL_MACHINE\SOFTWARE\Classes\MIME\Database\Rfc1766 found 3 locales which were missing, so added them now. >Basing off a [package require registry] would be acceptable to me A [package require] is expensive when the package is not found, as it has to traverse all possible directories. So I fully agree with Harald. [info sharedlibextension] is the cheapest way I know of. ---------------------------------------------------------------------- Comment By: Harald Oehlmann (oehhar) Date: 2012-06-29 04:38 Message: I personally don't like to do a if {[catch {package require registry}]} { ...} as this throws systematically an error on non-windows platforms. This will pollute the ::errorInfo variable which is IMHO not good practice for a core package. ---------------------------------------------------------------------- Comment By: Donal K. Fellows (dkf) Date: 2012-06-29 03:52 Message: Basing off a [package require registry] would be acceptable to me; that is a package that is definitively not available on non-Windows platforms. The cost of populating the package database will have already been borne too; this is *inside* a package in the first place, so it will be possible to say definitively whether the package is there or not with fairly low cost. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-06-29 02:32 Message: merged to core-8-5-branch and trunk ---------------------------------------------------------------------- Comment By: Harald Oehlmann (oehhar) Date: 2012-06-29 02:24 Message: Please do it. I am not ready jet, no login etc... ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-06-29 02:16 Message: So, will you do the merge, or do you prefer that I do it for you. (I don't know how familiar you are already with fossil, this would be a good test) ---------------------------------------------------------------------- Comment By: Harald Oehlmann (oehhar) Date: 2012-06-29 02:10 Message: Both points ok ! Thanks, Harald ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-06-29 02:03 Message: > Or we could use > package present registry > instead > [info sharedlibextension] ne ".dll" That wouldn't work: If "registry" is available but not loaded yet, this would resturn false. > - Also I would prefer the registry methods over the environ variable method > on windows, as: > - LANG is sometimes set, for example by an installed CYGWIN > - LANG is normally less detailed - the country is missing while the > registry method extracts the country. By default, LANG is not set in CYGWIN unless the user explicitely sets it. So, this is usefull as a way to override the registry setting. I wouldn't change that. ---------------------------------------------------------------------- Comment By: Harald Oehlmann (oehhar) Date: 2012-06-29 01:43 Message: Or we could use package present registry instead [info sharedlibextension] ne ".dll" This would clearly indicate what is required, the registry package. ---- Additional future thoughts: - We could replace the fix translation table CLID->locale by registry access, as this table is contained in the registry: [HKEY_LOCAL_MACHINE\SOFTWARE\Classes\MIME\Database\Rfc1766] I have to check, if this table is available on Win XP, it is ok since Vista. I would only do this on tcl 8.6, as it might include some incompatibilities. So thats another story... - Also I would prefer the registry methods over the environ variable method on windows, as: - LANG is sometimes set, for example by an installed CYGWIN - LANG is normally less detailed - the country is missing while the registry method extracts the country. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-06-29 01:22 Message: I agree with your comments. The registry package works on Cygwin as on Windows, only $tcl_platform(platform) is "unix", so msgcat didn't try to use the registry package. The easiest way to say "Windows or Cygwin" is [info sharedlibextension], because those two platforms are the only ones using ".dll" So, comments adapted acoording to that. Suggested optimization is OK to me too. Now updated in bug-3536888 branch. ---------------------------------------------------------------------- Comment By: Harald Oehlmann (oehhar) Date: 2012-06-29 00:53 Message: Dear Jan, thank you for the two modifications. "key" was lost somewhere... Test is succesfull. Two remarks: 1) for me, the purpose of '[info sharedlibextension] ne ".dll"' is not obvious. I would like a comment like: "on windows but not cygwin" Or I would like a more explicit "windows and not cygwin" check using the platform array/packages What exactly happens on cygwin ? Is there no registry package ? 2) Small optimisation: Replace: # # The rest of this routine is special processing for Windows; # all other platforms, get out now. # if {[info sharedlibextension] ne ".dll"} { mclocale C return } # # On Windows, try to set locale depending on registry settings, # or fall back on locale of "C". # if {[catch { package require registry }]} { mclocale C return } by: # The rest of this routine is special processing for Windows; # all other platforms, get out now. # if {[info sharedlibextension] ne ".dll" || [catch {package require registry}] } { mclocale C return } # # On Windows, try to set locale depending on registry settings, # or fall back on locale of "C". -END- Thank you, Harald ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-06-28 13:07 Message: Fixed two things in the bug-3536888 branch: - the variable "key" was used before defined - didn't work on cygwin, now it does. Harald, I think it's ready to be merged to core-8-5-branch. Do you agree? Please test it on your german-swiss windows 7 machine. To me everything looks fine now. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-06-22 06:40 Message: committed to branch bug-3536888 ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=3536888&group_id=10894 |