[MERGE #5264 @msdimitrov] Implement JavascriptArray::GetLocaleSeparator for Linux/macOS.

Merge pull request #5264 from msdimitrov:locale-separator

Implement JavascriptArray::GetLocaleSeparator for Linux/macOS. Refactor JavascriptArray::GetLocaleSeparator into new module PlatformAgnostic::Arrays

Author: jackhorton

lib/Common/CommonPal.h

@@ -744,6 +744,7 @@ namespace PlatformAgnostic
 
 #include "PlatformAgnostic/DateTime.h"
 #include "PlatformAgnostic/Numbers.h"
+#include "PlatformAgnostic/Arrays.h"
 #include "PlatformAgnostic/SystemInfo.h"
 #include "PlatformAgnostic/Thread.h"
 #include "PlatformAgnostic/AssemblyCommon.h"

lib/Common/PlatformAgnostic/Arrays.h

@@ -0,0 +1,40 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+#pragma once
+
+namespace PlatformAgnostic
+{
+namespace Arrays
+{
+#ifndef ENABLE_GLOBALIZATION
+
+    class ArrayLocalization
+    {
+        char16 localeSeparator;
+
+        static const uint16 BufSize = 256;
+
+    public:
+
+        ArrayLocalization();
+
+        inline char16 GetLocaleSeparator() { return localeSeparator; }
+    };
+
+#endif   
+
+    static const uint32 SeparatorBufferSize = 6;
+
+    // According to MSDN the maximum number of characters for the list separator (LOCALE_SLIST) is four, including a terminating null character.
+    bool GetLocaleSeparator(char16* szSeparator, uint32* sepOutLength, uint32 sepBufferSize);
+
+    template <uint32 sepBufferSize>
+    inline bool GetLocaleSeparator(char16(&szSepatator)[sepBufferSize], uint32 *sepOutLength)
+    {
+        return GetLocaleSeparator(szSeparator, sepOutLength, sepBufferSize);
+    }
+
+} // namespace Arrays
+} // namespace PlatformAgnostic

lib/Runtime/Library/JavascriptArray.cpp

@@ -8246,37 +8246,6 @@ using namespace Js;
     }
 #endif
 
-    JavascriptString* JavascriptArray::GetLocaleSeparator(ScriptContext* scriptContext)
-    {
-#ifdef ENABLE_GLOBALIZATION
-        LCID lcid = GetUserDefaultLCID();
-        int count = 0;
-        char16 szSeparator[6];
-
-        // According to the document for GetLocaleInfo this is a sufficient buffer size.
-        count = GetLocaleInfoW(lcid, LOCALE_SLIST, szSeparator, 5);
-        if( !count)
-        {
-            AssertMsg(FALSE, "GetLocaleInfo failed");
-            return scriptContext->GetLibrary()->GetCommaSpaceDisplayString();
-        }
-        else
-        {
-            // Append ' '  if necessary
-            if( count < 2 || szSeparator[count-2] != ' ')
-            {
-                szSeparator[count-1] = ' ';
-                szSeparator[count] = '\0';
-            }
-
-            return JavascriptString::NewCopyBuffer(szSeparator, count, scriptContext);
-        }
-#else
-        // xplat-todo: Support locale-specific separator
-        return scriptContext->GetLibrary()->GetCommaSpaceDisplayString();
-#endif
-    }
-
     template <typename T>
     JavascriptString* JavascriptArray::ToLocaleString(T* arr, ScriptContext* scriptContext)
     {
@@ -8318,7 +8287,21 @@ using namespace Js;
 
             if (length > 1)
             {
-                JavascriptString* separator = GetLocaleSeparator(scriptContext);
+                uint32 sepSize = 0;
+                char16 szSeparator[Arrays::SeparatorBufferSize];
+
+                bool hasLocaleSeparator = Arrays::GetLocaleSeparator(szSeparator, &sepSize, Arrays::SeparatorBufferSize);
+
+                JavascriptString* separator = nullptr;
+
+                if (hasLocaleSeparator)
+                {
+                    separator = JavascriptString::NewCopyBuffer(szSeparator, static_cast<charcount_t>(sepSize), scriptContext);
+                }
+                else
+                {
+                    separator = scriptContext->GetLibrary()->GetCommaSpaceDisplayString();
+                }
 
                 for (uint32 i = 1; i < length; i++)
                 {

lib/Runtime/Library/JavascriptArray.h

@@ -892,7 +892,6 @@ namespace Js
 
         template <typename T>
         static JavascriptString* ToLocaleString(T* obj, ScriptContext* scriptContext);
-        static JavascriptString* GetLocaleSeparator(ScriptContext* scriptContext);
 
     public:
         static uint32 GetOffsetOfArrayFlags() { return offsetof(JavascriptArray, arrayFlags); }

lib/Runtime/PlatformAgnostic/Chakra.Runtime.PlatformAgnostic.vcxproj

@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
 <Project DefaultTargets="Build" ToolsVersion="12.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
   <Import Condition="'$(ChakraBuildPathImported)'!='true'" Project="$(SolutionDir)Chakra.Build.Paths.props" />
   <Import Project="$(BuildConfigPropsPath)Chakra.Build.ProjectConfiguration.props" />
@@ -44,8 +44,9 @@
     <ClCompile Include="$(MSBuildThisFileDirectory)Platform\Windows\DateTime.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)Platform\Windows\DaylightHelper.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)Platform\Windows\HiResTimer.cpp" />
-    <ClCompile Include="$(MSBuildThisFileDirectory)Platform\Windows\UnicodeText.cpp"/>
+    <ClCompile Include="$(MSBuildThisFileDirectory)Platform\Windows\UnicodeText.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)Platform\Windows\NumbersUtility.cpp" />
+    <ClCompile Include="$(MSBuildThisFileDirectory)Platform\Windows\ArraysUtility.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)Platform\Windows\SystemInfo.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)Platform\Windows\Thread.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)Platform\Windows\PerfTrace.cpp" />
@@ -67,4 +68,4 @@
   </ItemGroup>
   <Import Project="$(BuildConfigPropsPath)Chakra.Build.targets" Condition="exists('$(BuildConfigPropsPath)Chakra.Build.targets')" />
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
-</Project>
+</Project>

lib/Runtime/PlatformAgnostic/Platform/CMakeLists.txt

@@ -5,6 +5,7 @@ set(PL_SOURCE_FILES
   Common/HiResTimer.cpp
   Common/DateTime.cpp
   Linux/NumbersUtility.cpp
+  POSIX/ArraysUtility.cpp
   Common/Thread.cpp
   Common/Trace.cpp
   Common/SystemInfo.Common.cpp

lib/Runtime/PlatformAgnostic/Platform/POSIX/ArraysUtility.cpp

@@ -0,0 +1,63 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+#include "Common.h"
+#include "ChakraPlatform.h"
+#include <locale.h>
+
+namespace PlatformAgnostic
+{
+namespace Arrays
+{
+
+    static char16 commaSeparator = _u(',');
+    static char16 semicolonSeparator = _u(';');
+
+    ArrayLocalization::ArrayLocalization()
+    {
+        char buffer[BufSize];
+        char *old_locale = setlocale(LC_NUMERIC, NULL);
+
+        if (old_locale != NULL)
+        {
+            memcpy(buffer, old_locale, BufSize);
+
+            setlocale(LC_NUMERIC, "");
+        }
+
+        struct lconv *locale_format = localeconv();
+        if (locale_format != NULL)
+        {
+            // No specific list separator on Linux/macOS
+            // Use this pattern to determine the list separator
+            if (*locale_format->decimal_point == commaSeparator)
+            {
+                localeSeparator = semicolonSeparator;
+            }
+            else
+            {
+                localeSeparator = commaSeparator;
+            }
+        }
+
+        if (old_locale != NULL)
+        {
+            setlocale(LC_NUMERIC, buffer);
+        }
+    }
+
+    bool GetLocaleSeparator(char16* szSeparator, uint32* sepOutSize, uint32 sepBufSize)
+    {
+        ArrayLocalization arrayLocalization;
+        //Append ' ' after separator
+        szSeparator[*sepOutSize] = arrayLocalization.GetLocaleSeparator();
+        szSeparator[++(*sepOutSize)] = ' ';
+        szSeparator[++(*sepOutSize)] = '\0';
+
+        return true;
+    }
+
+} // namespace Arrays
+} // namespace PlatformAgnostic

lib/Runtime/PlatformAgnostic/Platform/Windows/ArraysUtility.cpp

@@ -0,0 +1,48 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+#include "RuntimePlatformAgnosticPch.h"
+#include "Common.h"
+#include "ChakraPlatform.h"
+
+namespace PlatformAgnostic
+{
+namespace Arrays
+{
+    // Potential candidate for optimization 
+    bool GetLocaleSeparator(char16* szSeparator, uint32* sepOutSize, uint32 sepBufSize)
+    {
+        char16 localeName[LOCALE_NAME_MAX_LENGTH] = { 0 };
+
+        int ret = GetUserDefaultLocaleName(localeName, _countof(localeName));
+
+        if (ret == 0)
+        {
+            AssertMsg(FALSE, "GetUserDefaultLocaleName failed");
+            return false;
+        }
+
+        *sepOutSize = GetLocaleInfoEx(localeName, LOCALE_SLIST, szSeparator, sepBufSize);
+
+        if (*sepOutSize == 0)
+        {
+            AssertMsg(FALSE, "GetLocaleInfo failed");
+            return false;
+        }
+        else
+        {
+            // Append ' ' if necessary
+            if (*sepOutSize < 2 || szSeparator[*sepOutSize - 2] != ' ')
+            {
+                szSeparator[*sepOutSize - 1] = ' ';
+                szSeparator[*sepOutSize] = '\0';
+            }
+        }
+
+        return true;
+    }
+
+} // namespace Arrays
+} // namespace PlatformAgnostic