neildhar
commented
1 month ago Hey @manuelpuyol, it's exciting to see this progress! I noticed you're sharing code differently than the original MS implementation, and I wanted to suggest a further slight tweak that will allow much more code to live in PlatformIntlShared.cpp.
This is just something to consider as you're thinking about it, but we can move forward with this PR regardless of whether you adopt it.
The main idea is to have PlatformIntlShared as a middle layer between the PlatformIntl and PlatformIntlICU. So instead of having PlatformIntlICU call into PlatformIntlShared for common dependencies, we have PlatformIntlShared call into PlatformIntlICU for platform specific code. That makes it easy to share the code, since a platform only has to implement a simple well defined API.
So using Collator as an example, it would look roughly like:
PlatformIntlShared.h:
#include "hermes/Platform/Intl/PlatformIntl.h"
class CollatorShared : public Collator {
public:
CollatorShared() = default;
vm::ExecutionStatus initialize(
vm::Runtime &runtime,
const std::vector<std::u16string> &locales,
const Options &options) noexcept;
// Just an example, the signature might need to be different.
void platformInitialize();
Options resolvedOptions() noexcept;
double compare(const std::u16string &x, const std::u16string &y) noexcept;
private:
std::u16string locale_;
std::u16string usage_;
std::u16string collation_;
std::u16string caseFirst_;
std::u16string sensitivity_;
bool numeric_;
bool ignorePunctuation_;
};PlatformIntlShared.cpp
vm::ExecutionStatus CollatorApple::initialize(
vm::Runtime &runtime,
const std::vector<std::u16string> &locales,
const Options &options) noexcept {
// Do platform agnostic init first.
platformInitialize();
}PlatformIntlICU.cpp
class CollatorICU : public CollatorShared {
public:
CollatorICU() = default;
// Just an example, the signature might need to be different.
void platformInitialize();
double compare(const std::u16string &x, const std::u16string &y) noexcept;
private:
// ICU specific fields
};
double CollatorShared::compare(
const std::u16string &x,
const std::u16string &y) noexcept {
return static_cast<CollatorICU *>(this)->compare(x, y);
}
void CollatorShared::platformInitialize() noexcept {
return static_cast<CollatorICU *>(this)->platformInitialize();
}
manuelpuyol
facebook-github-bot
Related to https://github.com/facebook/hermes/issues/1350 and https://github.com/facebook/hermes/discussions/1211 Follow up of https://github.com/facebook/hermes/pull/1361
Summary
I'd like to expand non iOS/android support of
Intl. In this case, I'm implementing bothDateTimeFormat.prototype.resolvedOptionsandDateTimeFormat.prototype.format. The implementation is a mix from the Apple and hermes-windows' implementations.PlatformIntlSharedwith helper functions that are exactly the same between Apple and ICUDateTimeFormatICUbased onDateTimeFormatAppleTest Plan
Testing against
test262/test/intl402/DateTimeFormat