From 1472bc56b1b27348a0bb48c4ca731ceff05448a9 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Fri, 4 Aug 2023 18:16:53 -0500 Subject: [PATCH 01/14] =?UTF-8?q?feat(core):=20regex=20test=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For: #9121 --- .../keyboards/k_007_transform_rgx-test.xml | 14 ++++++ .../ldml/keyboards/k_007_transform_rgx.xml | 45 +++++++++++++++++++ core/tests/unit/ldml/keyboards/meson.build | 1 + 3 files changed, 60 insertions(+) create mode 100644 core/tests/unit/ldml/keyboards/k_007_transform_rgx-test.xml create mode 100644 core/tests/unit/ldml/keyboards/k_007_transform_rgx.xml diff --git a/core/tests/unit/ldml/keyboards/k_007_transform_rgx-test.xml b/core/tests/unit/ldml/keyboards/k_007_transform_rgx-test.xml new file mode 100644 index 0000000000..6e4cebf9f1 --- /dev/null +++ b/core/tests/unit/ldml/keyboards/k_007_transform_rgx-test.xml @@ -0,0 +1,14 @@ + + + + + + + + + + + + + + diff --git a/core/tests/unit/ldml/keyboards/k_007_transform_rgx.xml b/core/tests/unit/ldml/keyboards/k_007_transform_rgx.xml new file mode 100644 index 0000000000..d9fe68087e --- /dev/null +++ b/core/tests/unit/ldml/keyboards/k_007_transform_rgx.xml @@ -0,0 +1,45 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/core/tests/unit/ldml/keyboards/meson.build b/core/tests/unit/ldml/keyboards/meson.build index 54ab22f2e8..b09585a77d 100644 --- a/core/tests/unit/ldml/keyboards/meson.build +++ b/core/tests/unit/ldml/keyboards/meson.build @@ -32,6 +32,7 @@ tests_without_testdata = [ # These tests have a k_001_tiny-test.xml file as well. tests_with_testdata = [ 'k_001_tiny', + 'k_007_transform_rgx', 'k_020_fr', # TODO-LDML: move to cldr above (fix vkey) 'k_200_reorder_nod_Lana', 'k_210_marker', From 5c38b183951512d940afee7920088694bcb75087 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Tue, 8 Aug 2023 18:45:03 -0500 Subject: [PATCH 02/14] =?UTF-8?q?feat(core):=20update=20icu=20dependencies?= =?UTF-8?q?=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - enable icui18n - .. without breaking anything else in ldml tests For: #9121 --- core/src/ldml/ldml_transforms.cpp | 10 +++++++ core/src/meson.build | 27 +++++++++++++++++-- core/subprojects/icu-minimal.wrap | 3 +-- .../packagefiles/icu/source/meson.build | 3 +-- core/tests/unit/ldml/ldml_test_source.cpp | 1 - core/tests/unit/ldml/meson.build | 23 +++------------- 6 files changed, 40 insertions(+), 27 deletions(-) diff --git a/core/src/ldml/ldml_transforms.cpp b/core/src/ldml/ldml_transforms.cpp index 821f48e0dd..b8c1f5190e 100644 --- a/core/src/ldml/ldml_transforms.cpp +++ b/core/src/ldml/ldml_transforms.cpp @@ -11,6 +11,16 @@ #include #include "kmx/kmx_xstring.h" +#if defined(HAVE_ICU4C) +// TODO-LDML: Needed this for some compiler warnings +#define U_FALLTHROUGH +#include "unicode/uniset.h" +#include "unicode/usetiter.h" +#include "unicode/regex.h" +#else +#error icu4c is required for this code +#endif + #ifndef assert #define assert(x) // TODO-LDML diff --git a/core/src/meson.build b/core/src/meson.build index 97b9291646..2f8c2a1882 100644 --- a/core/src/meson.build +++ b/core/src/meson.build @@ -21,6 +21,27 @@ if cpp_compiler.get_id() == 'emscripten' defns += ['-DKMN_KBP'] endif +# ICU4C is used for repertoire tests and core implementation + +if target_machine.system() == 'linux' + # use pkg-config when targetting linux + pkgconfig = 'pkg-config' + icu_uc = dependency('icu-uc', required: true) + icu_i18n = dependency('icu-i18n', required: true) +else + # load ICU from wrap + # Requires meson of about 0.57+ + icu4c = subproject('icu-minimal', default_options: [ 'default_library=static', 'cpp_std=c++17', 'warning_level=0', +'werror=false']) # TODO-LDML: options: static, no data (these are in the meson build files) + icu_uc = icu4c.get_variable('icuuc_dep') + icu_i18n = icu4c.get_variable('icui18n_dep') +endif + +if icu_uc.found() + defns += '-DHAVE_ICU4C' +endif + + kmx_files = files( 'option.cpp', 'keyboard.cpp', @@ -85,11 +106,13 @@ lib = library('kmnkbp0', version: lib_version, include_directories: inc, pic: true, - install: true) + install: true, + dependencies: [icu_uc, icu_i18n], + ) headerdirs = [ '.', 'keyman' ] # subdirectories of ${prefix}/include to add to header path -kmnkbp = declare_dependency(link_with: lib, include_directories: inc) +kmnkbp = declare_dependency(link_with: lib, include_directories: inc, dependencies: [icu_uc, icu_i18n]) pkg = import('pkgconfig') pkg.generate( diff --git a/core/subprojects/icu-minimal.wrap b/core/subprojects/icu-minimal.wrap index fe509b48e0..3c85940c60 100644 --- a/core/subprojects/icu-minimal.wrap +++ b/core/subprojects/icu-minimal.wrap @@ -10,5 +10,4 @@ patch_directory = icu [provide] icu-uc = icuuc_dep -# TODO-LDML: not including i18n at present -#icu-i18n = icui18n_dep +icu-i18n = icui18n_dep diff --git a/core/subprojects/packagefiles/icu/source/meson.build b/core/subprojects/packagefiles/icu/source/meson.build index 7d08a1e3cb..a304720dfb 100644 --- a/core/subprojects/packagefiles/icu/source/meson.build +++ b/core/subprojects/packagefiles/icu/source/meson.build @@ -9,8 +9,7 @@ endif subdir('stubdata') subdir('common') -# TODO-LDML: Not used now, will need for regex -# subdir('i18n') +subdir('i18n') ## Note: The following subdirs are not used by Keyman (at present) diff --git a/core/tests/unit/ldml/ldml_test_source.cpp b/core/tests/unit/ldml/ldml_test_source.cpp index da65e62a27..1caae7de60 100644 --- a/core/tests/unit/ldml/ldml_test_source.cpp +++ b/core/tests/unit/ldml/ldml_test_source.cpp @@ -25,7 +25,6 @@ #include "ldml/keyboardprocessor_ldml.h" #include "ldml/ldml_processor.hpp" - #include "path.hpp" #include "state.hpp" #include "utfcodec.hpp" diff --git a/core/tests/unit/ldml/meson.build b/core/tests/unit/ldml/meson.build index 679acbe599..f54c94ef28 100644 --- a/core/tests/unit/ldml/meson.build +++ b/core/tests/unit/ldml/meson.build @@ -4,25 +4,6 @@ # Authors: Marc Durdin # -# ICU4C is used for repertoire tests - - -if target_machine.system() == 'linux' - # use pkg-config when targetting linux - pkgconfig = 'pkg-config' - icu_uc = dependency('icu-uc', required: true) -else - # load ICU from wrap - # Requires meson of about 0.57+ - icu4c = subproject('icu-minimal', default_options: [ 'default_library=static', 'cpp_std=c++17', 'warning_level=0', -'werror=false']) # TODO-LDML: options: static, no data (these are in the meson build files) - icu_uc = icu4c.get_variable('icuuc_dep') -endif - -if icu_uc.found() - defns += '-DHAVE_ICU4C' -endif - # TODO -- why are these differing from the standard.meson.build flags? if cpp_compiler.get_id() == 'gcc' or cpp_compiler.get_id() == 'clang' or cpp_compiler.get_id() == 'emscripten' warns = [ @@ -75,8 +56,9 @@ ldml = executable('ldml', cpp_args: defns + warns, include_directories: [inc, libsrc, '../../../../developer/src/ext/json'], link_args: links + tests_flags, - objects: lib.extract_all_objects(recursive: false), dependencies: [icu_uc], + # link_with: [lib], + objects: lib.extract_all_objects(recursive: false), ) # Run tests on all keyboards (`tests` defined in keyboards/meson.build) @@ -110,5 +92,6 @@ t = executable('test_transforms', 'test_transforms.cpp', cpp_args: defns + warns, include_directories: [inc, libsrc, '../../../../developer/src/ext/json'], link_args: links + tests_flags, + dependencies: [icu_uc], objects: lib.extract_all_objects(recursive: false)) test('test_transforms', t, suite: 'ldml') From 742373b67396fd675878501efea2276e195efb5f Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Wed, 9 Aug 2023 16:19:38 -0500 Subject: [PATCH 03/14] =?UTF-8?q?feat(core):=20u32string=5Fto=5Fu16string?= =?UTF-8?q?=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For: #9121 --- core/src/kmx/kmx_xstring.h | 13 +++++++++ core/tests/unit/kmnkbd/test_kmx_xstring.cpp | 29 +++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/core/src/kmx/kmx_xstring.h b/core/src/kmx/kmx_xstring.h index 9b479dac7a..028cde8e99 100644 --- a/core/src/kmx/kmx_xstring.h +++ b/core/src/kmx/kmx_xstring.h @@ -172,6 +172,19 @@ u16string_to_u32string(const std::u16string &source) { return out; } +inline std::u16string +u32string_to_u16string(const std::u32string &source) { + std::u16string out; + char16_single ch; + for (auto c : source) { + const auto len = Utf32CharToUtf16(c, ch); + for (auto i = 0; i < len; i++) { + out.push_back(ch.ch[i]); + } + } + return out; +} + inline bool Uni_IsEndOfPlaneNonCharacter(km_kbp_usv ch) { return (((ch) & Uni_FFFE_NONCHARACTER) == Uni_FFFE_NONCHARACTER); // matches FFFF or FFFE } diff --git a/core/tests/unit/kmnkbd/test_kmx_xstring.cpp b/core/tests/unit/kmnkbd/test_kmx_xstring.cpp index e3c4077622..92404b1bc8 100644 --- a/core/tests/unit/kmnkbd/test_kmx_xstring.cpp +++ b/core/tests/unit/kmnkbd/test_kmx_xstring.cpp @@ -1329,6 +1329,35 @@ test_u16string_to_u32string() { } } + +void +test_u32string_to_u16string() { + std::cout << "== " << __FUNCTION__ << std::endl; + // normal cases + { + const auto str = u32string_to_u16string(U""); + assert_equal(str.length(), 0); + } + { + const auto str = u32string_to_u16string(U"e"); + assert_equal(str.length(), 1); + assert_equal(str.at(0), 0x0065); + } + { + const auto str = u32string_to_u16string(U"🙀"); + assert_equal(str.length(), 2); + assert_equal(str.at(0), 0xD83D); + assert_equal(str.at(1), 0xDE40); + } + { + const auto str = u32string_to_u16string(U"Ω🙀"); + assert_equal(str.length(), 3); + assert_equal(str.at(0), u'Ω'); + assert_equal(str.at(1), 0xD83D); + assert_equal(str.at(2), 0xDE40); + } +} + void test_is_valid() { std::cout << "== " << __FUNCTION__ << std::endl; // valid From 022265e1b8a3722491d6dba065f7c469d8616cfd Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Wed, 9 Aug 2023 16:27:56 -0500 Subject: [PATCH 04/14] =?UTF-8?q?feat(core):=20regex=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - update more test deps to link with ICU… For: #9121 --- core/tests/unit/kmnkbd/meson.build | 1 + core/tests/unit/kmx/meson.build | 3 +++ core/tests/unit/ldml/meson.build | 5 +++-- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/core/tests/unit/kmnkbd/meson.build b/core/tests/unit/kmnkbd/meson.build index 049770fa2f..9d3ee825e8 100644 --- a/core/tests/unit/kmnkbd/meson.build +++ b/core/tests/unit/kmnkbd/meson.build @@ -37,6 +37,7 @@ foreach t : tests cpp_args: local_defns + defns + warns, include_directories: [inc, libsrc], link_args: links + tests_flags, + dependencies: [icu_uc, icu_i18n], objects: lib.extract_all_objects(recursive: false)) test(t[0], bin, args: ['--color', test_path]) diff --git a/core/tests/unit/kmx/meson.build b/core/tests/unit/kmx/meson.build index 7e4b9f8a77..df40949263 100644 --- a/core/tests/unit/kmx/meson.build +++ b/core/tests/unit/kmx/meson.build @@ -31,6 +31,7 @@ kmx = executable('kmx', cpp_args: defns + warns, include_directories: [inc, libsrc, '../../kmx_test_source'], link_args: links + tests_flags, + dependencies: [icu_uc, icu_i18n], objects: [lib.extract_all_objects(recursive: false), kmx_test_source_lib.extract_all_objects(recursive: false)]) tests = [ @@ -178,6 +179,7 @@ key_e = executable('key_list', ['kmx_key_list.cpp', '../emscripten_filesystem.cp cpp_args: defns + warns, include_directories: [inc, libsrc], link_args: links + tests_flags, + dependencies: [icu_uc, icu_i18n], objects: lib.extract_all_objects(recursive: false)) test_kbd = 'kmx_key_list' @@ -196,6 +198,7 @@ imx_e = executable('imx_list', ['kmx_imx.cpp', '../emscripten_filesystem.cpp'], cpp_args: defns + warns, include_directories: [inc, libsrc], link_args: links + tests_flags, + dependencies: [icu_uc, icu_i18n], objects: lib.extract_all_objects(recursive: false)) test_kbd = 'kmx_imsample' diff --git a/core/tests/unit/ldml/meson.build b/core/tests/unit/ldml/meson.build index f54c94ef28..c02d64bf18 100644 --- a/core/tests/unit/ldml/meson.build +++ b/core/tests/unit/ldml/meson.build @@ -56,7 +56,7 @@ ldml = executable('ldml', cpp_args: defns + warns, include_directories: [inc, libsrc, '../../../../developer/src/ext/json'], link_args: links + tests_flags, - dependencies: [icu_uc], + dependencies: [icu_uc, icu_i18n], # link_with: [lib], objects: lib.extract_all_objects(recursive: false), ) @@ -85,6 +85,7 @@ e = executable('test_kmx_plus', 'test_kmx_plus.cpp', cpp_args: defns + warns, include_directories: [inc, libsrc, '../../../../developer/src/ext/json'], link_args: links + tests_flags, + dependencies: [icu_uc, icu_i18n], objects: lib.extract_all_objects(recursive: false)) test('test_kmx_plus', e, suite: 'ldml') @@ -92,6 +93,6 @@ t = executable('test_transforms', 'test_transforms.cpp', cpp_args: defns + warns, include_directories: [inc, libsrc, '../../../../developer/src/ext/json'], link_args: links + tests_flags, - dependencies: [icu_uc], + dependencies: [icu_uc, icu_i18n], objects: lib.extract_all_objects(recursive: false)) test('test_transforms', t, suite: 'ldml') From 1aba900b0164339e327aff63ced3ac477941273c Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Wed, 9 Aug 2023 17:45:56 -0500 Subject: [PATCH 05/14] =?UTF-8?q?feat(core):=20regex=20match-side=20?= =?UTF-8?q?=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - match side is working - lots of string 'schlepping! For: #9121 --- core/src/ldml/ldml_transforms.cpp | 45 ++++++++++++++----- .../unit/ldml/keyboards/k_003_transform.xml | 2 +- core/tests/unit/ldml/test_transforms.cpp | 2 +- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/core/src/ldml/ldml_transforms.cpp b/core/src/ldml/ldml_transforms.cpp index b8c1f5190e..65ec63aa38 100644 --- a/core/src/ldml/ldml_transforms.cpp +++ b/core/src/ldml/ldml_transforms.cpp @@ -16,7 +16,10 @@ #define U_FALLTHROUGH #include "unicode/uniset.h" #include "unicode/usetiter.h" +#include "unicode/unistr.h" #include "unicode/regex.h" +#include "unicode/utext.h" +#include "unicode/utypes.h" #else #error icu4c is required for this code #endif @@ -408,19 +411,37 @@ transform_entry::transform_entry(const std::u32string &from, const std::u32strin size_t transform_entry::match(const std::u32string &input) const { - if (input.length() < fFrom.length()) { - // TODO-LDML: regex - // Too small, can't match. - return 0; + // TODO-LDML: simple approach, new regex every time + // TODO-LDML: Really? can't go from u32 to UnicodeString? + + const std::u16string patstr = km::kbp::kmx::u32string_to_u16string(fFrom); + UErrorCode status = U_ZERO_ERROR; + /* const */ icu::UnicodeString patustr = icu::UnicodeString(patstr.data(), (int32_t)patstr.length()); + // add '$' to match to end + patustr.append(u'$'); + std::unique_ptr pattern(icu::RegexPattern::compile(patustr, 0, status)); + assert(U_SUCCESS(status)); + + const std::u16string matchstr = km::kbp::kmx::u32string_to_u16string(input); + icu::UnicodeString matchustr = icu::UnicodeString(matchstr.data(), (int32_t)matchstr.length()); + std::unique_ptr matcher(pattern->matcher(matchustr, status)); + assert(U_SUCCESS(status)); + + if (!matcher->find(status)) { // i.e. matches somewhere, in this case at end of str + return 0; // and tear everything down } - // string at end - auto substr = input.substr(input.length() - fFrom.length(), fFrom.length()); - if (substr != fFrom) { - // end of string doesn't match - return 0; - } - // match length == fFrom.length - return substr.length(); + + // TODO-LDML: this is UTF-16 len, not UTF-32 len!! + // auto matchLen = matcher->end64(status) - matcher->start64(status); + // TODO-LDML: if we had an underlying UText this would be simpler. + auto matchStart = matcher->start64(status); + auto matchEnd = matcher->end64(status); + // extract.. + const icu::UnicodeString substr = matchustr.tempSubStringBetween((int32_t)matchStart, (int32_t)matchEnd); + // preflight to UTF-32 to get length + auto matchLen = substr.toUTF32(nullptr, 0, status); + + return matchLen; } std::u32string diff --git a/core/tests/unit/ldml/keyboards/k_003_transform.xml b/core/tests/unit/ldml/keyboards/k_003_transform.xml index aa90e79a3c..565378400a 100644 --- a/core/tests/unit/ldml/keyboards/k_003_transform.xml +++ b/core/tests/unit/ldml/keyboards/k_003_transform.xml @@ -45,7 +45,7 @@ from https://github.com/unicode-org/cldr/blob/keyboard-preview/docs/ldml/tr35-ke - + diff --git a/core/tests/unit/ldml/test_transforms.cpp b/core/tests/unit/ldml/test_transforms.cpp index 7097bcb64b..d0ab45e1ce 100644 --- a/core/tests/unit/ldml/test_transforms.cpp +++ b/core/tests/unit/ldml/test_transforms.cpp @@ -40,7 +40,7 @@ test_transforms() { std::cout << __FILE__ << ":" << __LINE__ << " - basic " << std::endl; { // start with one - transform_entry te(std::u32string(U"e^"), std::u32string(U"E")); // keep it simple + transform_entry te(std::u32string(U"e\\^"), std::u32string(U"E")); // keep it simple // OK now make a group do it transforms tr; transform_group st; From aecb7c5f5eee66c4b0c1531e59862fd67d43810c Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Wed, 9 Aug 2023 18:08:25 -0500 Subject: [PATCH 06/14] =?UTF-8?q?feat(core):=20regex=20apply-side=20?= =?UTF-8?q?=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - apply side is working using replaceFirst() - mapTo is a not-yet, that will be a special case For: #9121 --- core/src/ldml/ldml_transforms.cpp | 54 +++++++++++++++++-- .../ldml/keyboards/k_007_transform_rgx.xml | 3 -- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/core/src/ldml/ldml_transforms.cpp b/core/src/ldml/ldml_transforms.cpp index 65ec63aa38..f1dc2d6916 100644 --- a/core/src/ldml/ldml_transforms.cpp +++ b/core/src/ldml/ldml_transforms.cpp @@ -445,10 +445,56 @@ transform_entry::match(const std::u32string &input) const { } std::u32string -transform_entry::apply(const std::u32string & /*input*/, size_t /*matchLen*/) const { - // TODO-LDML: regex - // For now, we just return the 'to' string literally. - return fTo; +transform_entry::apply(const std::u32string &input, size_t matchLen) const { + // TODO-LDML: and if you thought the previous function was suboptimal, + // TODO-LDML: now we're going to do it all again! + // TODO-LDML: simple approach, new regex every time + // TODO-LDML: Really? can't go from u32 to UnicodeString? + + const std::u16string patstr = km::kbp::kmx::u32string_to_u16string(fFrom); + UErrorCode status = U_ZERO_ERROR; + /* const */ icu::UnicodeString patustr = icu::UnicodeString(patstr.data(), (int32_t)patstr.length()); + // add '$' to match to end + patustr.append(u'$'); + std::unique_ptr pattern(icu::RegexPattern::compile(patustr, 0, status)); + assert(U_SUCCESS(status)); + + // we know the matchLen so we can slice the string… + const std::u16string matchstr = km::kbp::kmx::u32string_to_u16string(input.substr(input.length()-matchLen, matchLen)); + icu::UnicodeString matchustr = icu::UnicodeString(matchstr.data(), (int32_t)matchstr.length()); + std::unique_ptr matcher(pattern->matcher(matchustr, status)); + assert(U_SUCCESS(status)); + // assert(matcher->find(status)); // it better match + const std::u16string rstr = km::kbp::kmx::u32string_to_u16string(fTo); + icu::UnicodeString rustr = icu::UnicodeString(rstr.data(), (int32_t)rstr.length()); + icu::UnicodeString output = matcher->replaceFirst(rustr, status); + assert(U_SUCCESS(status)); + + // if (!matcher->find(status)) { // i.e. matches somewhere, in this case at end of str + // return 0; // and tear everything down + // } + + + + // // TODO-LDML: this is UTF-16 len, not UTF-32 len!! + // // auto matchLen = matcher->end64(status) - matcher->start64(status); + // // TODO-LDML: if we had an underlying UText this would be simpler. + // auto matchStart = matcher->start64(status); + // auto matchEnd = matcher->end64(status); + // // extract.. + // const icu::UnicodeString substr = matchustr.tempSubStringBetween((int32_t)matchStart, (int32_t)matchEnd); + // // preflight to UTF-32 to get length + // auto matchLen = substr.toUTF32(nullptr, 0, status); + + // return matchLen; + assert(U_SUCCESS(status)); + UErrorCode preflightStatus = U_ZERO_ERROR; + auto out32len = output.toUTF32(nullptr, 0, preflightStatus); + char32_t *s = new char32_t[out32len+1]; + output.toUTF32((UChar32*)s, out32len+1, status); + std::u32string out32(s, out32len); + assert(U_SUCCESS(status)); + return out32; } any_group::any_group(const transform_group &g) : type(any_group_type::transform), transform(g), reorder() { diff --git a/core/tests/unit/ldml/keyboards/k_007_transform_rgx.xml b/core/tests/unit/ldml/keyboards/k_007_transform_rgx.xml index d9fe68087e..3f0bdcdd9b 100644 --- a/core/tests/unit/ldml/keyboards/k_007_transform_rgx.xml +++ b/core/tests/unit/ldml/keyboards/k_007_transform_rgx.xml @@ -3,9 +3,6 @@ From 36b402216550988d05f8f7bad0621aae333d6b7d Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Thu, 10 Aug 2023 17:50:00 -0500 Subject: [PATCH 07/14] =?UTF-8?q?chore(core):=20regex=20build=20?= =?UTF-8?q?=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - fix linkage for some scenarios For: #9121 --- core/tests/unit/kmx/meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/core/tests/unit/kmx/meson.build b/core/tests/unit/kmx/meson.build index 7af7452238..e12ed3a995 100644 --- a/core/tests/unit/kmx/meson.build +++ b/core/tests/unit/kmx/meson.build @@ -216,6 +216,7 @@ external_e = executable('ext_event', ['kmx_external_event.cpp', '../emscripten_f cpp_args: defns + warns, include_directories: [inc, libsrc], link_args: links + tests_flags, + dependencies: [icu_uc, icu_i18n], objects: lib.extract_all_objects(recursive: false)) test_kbd = 'k_033___caps_always_off' From 75fcc5f3fefea375cacfbba5c32191bec733ed53 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Thu, 10 Aug 2023 18:35:06 -0500 Subject: [PATCH 08/14] =?UTF-8?q?chore(core):=20regex=20cleanup=20?= =?UTF-8?q?=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - cache the icu::RegexPattern - rename USet to avoid a conflict with the real one! For: #9121 --- core/src/kmx/kmx_plus.cpp | 16 +-- core/src/kmx/kmx_plus.h | 12 ++- core/src/ldml/ldml_transforms.cpp | 116 ++++++++++------------ core/src/ldml/ldml_transforms.hpp | 27 +++-- core/tests/unit/ldml/ldml_test_source.cpp | 1 + core/tests/unit/ldml/test_kmx_plus.cpp | 4 +- core/tests/unit/ldml/test_transforms.cpp | 2 +- 7 files changed, 93 insertions(+), 85 deletions(-) diff --git a/core/src/kmx/kmx_plus.cpp b/core/src/kmx/kmx_plus.cpp index b02fec77d8..a0da603d95 100644 --- a/core/src/kmx/kmx_plus.cpp +++ b/core/src/kmx/kmx_plus.cpp @@ -1072,16 +1072,16 @@ COMP_KMXPLUS_USET_Helper::setUset(const COMP_KMXPLUS_USET *newUset) { return is_valid; } -USet::USet(const COMP_KMXPLUS_USET_RANGE *newRange, size_t newCount) { +SimpleUSet::SimpleUSet(const COMP_KMXPLUS_USET_RANGE *newRange, size_t newCount) { for (size_t i = 0; i < newCount; i++) { ranges.emplace_back(newRange[i].start, newRange[i].end); } } -USet::USet() { +SimpleUSet::SimpleUSet() { } -bool USet::contains(km_kbp_usv ch) const { +bool SimpleUSet::contains(km_kbp_usv ch) const { for (const auto &range : ranges) { if (range.start <= ch && range.end >= ch) { return true; @@ -1091,7 +1091,7 @@ bool USet::contains(km_kbp_usv ch) const { } bool -USet::valid() const { +SimpleUSet::valid() const { // double check for (const auto &range : ranges) { if (!Uni_IsValid(range.start, range.end)) { @@ -1103,7 +1103,7 @@ USet::valid() const { } void -USet::dump() const { +SimpleUSet::dump() const { DebugLog(" - USet size=%d", ranges.size()); for (const auto &range : ranges) { if (range.start == range.end) { @@ -1114,14 +1114,14 @@ USet::dump() const { } } -USet +SimpleUSet COMP_KMXPLUS_USET_Helper::getUset(KMXPLUS_USET i) const { if (!valid() || i >= uset->usetCount) { assert(false); - return USet(nullptr, 0); // empty set + return SimpleUSet(nullptr, 0); // empty set } auto &set = usets[i]; - return USet(getRange(set.range), set.count); + return SimpleUSet(getRange(set.range), set.count); } const COMP_KMXPLUS_USET_RANGE * diff --git a/core/src/kmx/kmx_plus.h b/core/src/kmx/kmx_plus.h index 2962ac8789..12b2cd44e7 100644 --- a/core/src/kmx/kmx_plus.h +++ b/core/src/kmx/kmx_plus.h @@ -694,14 +694,16 @@ struct COMP_KMXPLUS_USET_RANGE { }; /** - * represents one of the uset elements + * represents one of the uset elements. + * TODO-LDML: replace this with a real icu::UnicodeSet? or at least + * a function producing the same? */ -class USet { +class SimpleUSet { public: /** construct a set over the specified range. Data is copied. */ - USet(const COMP_KMXPLUS_USET_RANGE* newStart, size_t newCount); + SimpleUSet(const COMP_KMXPLUS_USET_RANGE* newStart, size_t newCount); /** empty set */ - USet(); + SimpleUSet(); /** true if the uset contains this char */ bool contains(km_kbp_usv ch) const; /** debugging */ @@ -721,7 +723,7 @@ public: bool setUset(const COMP_KMXPLUS_USET *newUset); inline bool valid() const { return is_valid; } - USet getUset(KMXPLUS_USET list) const; + SimpleUSet getUset(KMXPLUS_USET list) const; const COMP_KMXPLUS_USET_RANGE *getRange(KMX_DWORD index) const; private: diff --git a/core/src/ldml/ldml_transforms.cpp b/core/src/ldml/ldml_transforms.cpp index f1dc2d6916..64cc206053 100644 --- a/core/src/ldml/ldml_transforms.cpp +++ b/core/src/ldml/ldml_transforms.cpp @@ -11,20 +11,6 @@ #include #include "kmx/kmx_xstring.h" -#if defined(HAVE_ICU4C) -// TODO-LDML: Needed this for some compiler warnings -#define U_FALLTHROUGH -#include "unicode/uniset.h" -#include "unicode/usetiter.h" -#include "unicode/unistr.h" -#include "unicode/regex.h" -#include "unicode/utext.h" -#include "unicode/utypes.h" -#else -#error icu4c is required for this code -#endif - - #ifndef assert #define assert(x) // TODO-LDML #endif @@ -49,7 +35,7 @@ namespace ldml { #define DebugTran(msg, ...) #endif -element::element(const USet &new_u, KMX_DWORD new_flags) +element::element(const SimpleUSet &new_u, KMX_DWORD new_flags) : chr(), uset(new_u), flags((new_flags & ~LDML_ELEM_FLAGS_TYPE) | LDML_ELEM_FLAGS_TYPE_USET) { } @@ -204,7 +190,7 @@ element_list::load(const kmx::kmx_plus &kplus, kmx::KMXPLUS_ELEM id) { km_kbp_usv ch = e.element; emplace_back(ch, flags); // char } else if (type == LDML_ELEM_FLAGS_TYPE_USET) { - // need to load a USet + // need to load a SimpleUSet auto u = kplus.usetHelper.getUset(e.element); if (!u.valid()) { DebugLog("Error, invalid UnicodeSet at element %d", (int)i); @@ -406,25 +392,36 @@ reorder_group::apply(std::u32string &str) const { return applied; } +transform_entry::transform_entry(const transform_entry &other) : + fFrom(other.fFrom), fTo(other.fTo), fFromPattern(nullptr) { + if (other.fFromPattern) { + // clone pattern + fFromPattern.reset(other.fFromPattern->clone()); + } +} + transform_entry::transform_entry(const std::u32string &from, const std::u32string &to) : fFrom(from), fTo(to) { + assert(!fFrom.empty()); // TODO-LDML: should not happen? + + if (!fFrom.empty()) { + const std::u16string patstr = km::kbp::kmx::u32string_to_u16string(fFrom); + UErrorCode status = U_ZERO_ERROR; + /* const */ icu::UnicodeString patustr = icu::UnicodeString(patstr.data(), (int32_t)patstr.length()); + // add '$' to match to end + patustr.append(u'$'); + fFromPattern.reset(icu::RegexPattern::compile(patustr, 0, status)); + assert(U_SUCCESS(status)); // TODO-LDML: may be best to propagate status up ^^ + } } size_t transform_entry::match(const std::u32string &input) const { - // TODO-LDML: simple approach, new regex every time + assert(fFromPattern); // TODO-LDML: Really? can't go from u32 to UnicodeString? - - const std::u16string patstr = km::kbp::kmx::u32string_to_u16string(fFrom); - UErrorCode status = U_ZERO_ERROR; - /* const */ icu::UnicodeString patustr = icu::UnicodeString(patstr.data(), (int32_t)patstr.length()); - // add '$' to match to end - patustr.append(u'$'); - std::unique_ptr pattern(icu::RegexPattern::compile(patustr, 0, status)); - assert(U_SUCCESS(status)); - + UErrorCode status = U_ZERO_ERROR; const std::u16string matchstr = km::kbp::kmx::u32string_to_u16string(input); icu::UnicodeString matchustr = icu::UnicodeString(matchstr.data(), (int32_t)matchstr.length()); - std::unique_ptr matcher(pattern->matcher(matchustr, status)); + std::unique_ptr matcher(fFromPattern->matcher(matchustr, status)); assert(U_SUCCESS(status)); if (!matcher->find(status)) { // i.e. matches somewhere, in this case at end of str @@ -447,58 +444,53 @@ transform_entry::match(const std::u32string &input) const { std::u32string transform_entry::apply(const std::u32string &input, size_t matchLen) const { // TODO-LDML: and if you thought the previous function was suboptimal, - // TODO-LDML: now we're going to do it all again! - // TODO-LDML: simple approach, new regex every time + // TODO-LDML: We should cache the RegexMatcher from the previous call. // TODO-LDML: Really? can't go from u32 to UnicodeString? - const std::u16string patstr = km::kbp::kmx::u32string_to_u16string(fFrom); - UErrorCode status = U_ZERO_ERROR; - /* const */ icu::UnicodeString patustr = icu::UnicodeString(patstr.data(), (int32_t)patstr.length()); - // add '$' to match to end - patustr.append(u'$'); - std::unique_ptr pattern(icu::RegexPattern::compile(patustr, 0, status)); - assert(U_SUCCESS(status)); + assert(fFromPattern); + // TODO-LDML: simple approach, new regex every time + // TODO-LDML: Really? can't go from u32 to UnicodeString? + UErrorCode status = U_ZERO_ERROR; // we know the matchLen so we can slice the string… - const std::u16string matchstr = km::kbp::kmx::u32string_to_u16string(input.substr(input.length()-matchLen, matchLen)); - icu::UnicodeString matchustr = icu::UnicodeString(matchstr.data(), (int32_t)matchstr.length()); - std::unique_ptr matcher(pattern->matcher(matchustr, status)); + // Note: the matchstr here (unlike in transform_entry::match) is sliced so that + // it only includes the matched portion. This way, when changed it's suitable as the + // output string. + const std::u16string matchstr = km::kbp::kmx::u32string_to_u16string(input.substr(input.length() - matchLen, matchLen)); + icu::UnicodeString matchustr = icu::UnicodeString(matchstr.data(), (int32_t)matchstr.length()); + std::unique_ptr matcher(fFromPattern->matcher(matchustr, status)); assert(U_SUCCESS(status)); - // assert(matcher->find(status)); // it better match + // now, do the replace const std::u16string rstr = km::kbp::kmx::u32string_to_u16string(fTo); - icu::UnicodeString rustr = icu::UnicodeString(rstr.data(), (int32_t)rstr.length()); + icu::UnicodeString rustr = icu::UnicodeString(rstr.data(), (int32_t)rstr.length()); + // This replace will apply $1, $2 etc. TODO-LDML it will NOT handle mapFrom or mapTo. icu::UnicodeString output = matcher->replaceFirst(rustr, status); assert(U_SUCCESS(status)); - // if (!matcher->find(status)) { // i.e. matches somewhere, in this case at end of str - // return 0; // and tear everything down - // } + if (output.length() == 0) { + return std::u32string(); // special case of a zero length output (such as delete) + } - - - // // TODO-LDML: this is UTF-16 len, not UTF-32 len!! - // // auto matchLen = matcher->end64(status) - matcher->start64(status); - // // TODO-LDML: if we had an underlying UText this would be simpler. - // auto matchStart = matcher->start64(status); - // auto matchEnd = matcher->end64(status); - // // extract.. - // const icu::UnicodeString substr = matchustr.tempSubStringBetween((int32_t)matchStart, (int32_t)matchEnd); - // // preflight to UTF-32 to get length - // auto matchLen = substr.toUTF32(nullptr, 0, status); - - // return matchLen; - assert(U_SUCCESS(status)); + // TODO-LDML: All we are trying to do is to extract the output string. Probably too many steps. UErrorCode preflightStatus = U_ZERO_ERROR; - auto out32len = output.toUTF32(nullptr, 0, preflightStatus); - char32_t *s = new char32_t[out32len+1]; - output.toUTF32((UChar32*)s, out32len+1, status); - std::u32string out32(s, out32len); + // calculate how big the buffer is + auto out32len = output.toUTF32(nullptr, 0, preflightStatus); // preflightStatus will be an err, because we know the buffer overruns zero bytes + // allocate + char32_t *s = new char32_t[out32len + 1]; + assert(s != nullptr); + // convert + output.toUTF32((UChar32 *)s, out32len + 1, status); assert(U_SUCCESS(status)); + // now, build a u32string + std::u32string out32(s, out32len); + // clean up buffer + delete [] s; return out32; } any_group::any_group(const transform_group &g) : type(any_group_type::transform), transform(g), reorder() { } + any_group::any_group(const reorder_group &g) : type(any_group_type::reorder), transform(), reorder(g) { } diff --git a/core/src/ldml/ldml_transforms.hpp b/core/src/ldml/ldml_transforms.hpp index 6296399f2b..56a4117c4d 100644 --- a/core/src/ldml/ldml_transforms.hpp +++ b/core/src/ldml/ldml_transforms.hpp @@ -14,11 +14,23 @@ #include #include +#if !defined(HAVE_ICU4C) +#error icu4c is required for this code +#endif + +#define U_FALLTHROUGH +#include "unicode/utypes.h" +#include "unicode/uniset.h" +#include "unicode/usetiter.h" +#include "unicode/unistr.h" +#include "unicode/regex.h" +#include "unicode/utext.h" + namespace km { namespace kbp { namespace ldml { -using km::kbp::kmx::USet; +using km::kbp::kmx::SimpleUSet; /** * Type of a group @@ -33,12 +45,11 @@ enum any_group_type { */ class element { public: - /** construct from a USet */ - element(const USet &u, KMX_DWORD flags); + /** construct from a SimpleUSet */ + element(const SimpleUSet &u, KMX_DWORD flags); /** construct from a single char */ element(km_kbp_usv ch, KMX_DWORD flags); - - /** @returns true if a USet type */ + /** @returns true if a SimpleUSet type */ bool is_uset() const; /** @returns true if prebase bit set*/ bool is_prebase() const; @@ -58,7 +69,7 @@ public: private: // TODO-LDML: support multi-char strings? const km_kbp_usv chr; - const USet uset; + const SimpleUSet uset; const KMX_DWORD flags; }; @@ -67,6 +78,7 @@ private: */ class transform_entry { public: + transform_entry(const transform_entry &other); transform_entry( const std::u32string &from, const std::u32string &to @@ -84,8 +96,9 @@ public: std::u32string apply(const std::u32string &input, size_t matchLen) const; private: - const std::u32string fFrom; // TODO-LDML: regex + const std::u32string fFrom; const std::u32string fTo; + std::unique_ptr fFromPattern; }; /** diff --git a/core/tests/unit/ldml/ldml_test_source.cpp b/core/tests/unit/ldml/ldml_test_source.cpp index 1caae7de60..673f692f1e 100644 --- a/core/tests/unit/ldml/ldml_test_source.cpp +++ b/core/tests/unit/ldml/ldml_test_source.cpp @@ -35,6 +35,7 @@ #if defined(HAVE_ICU4C) // TODO-LDML: Needed this for some compiler warnings #define U_FALLTHROUGH +#include "unicode/utypes.h" #include "unicode/uniset.h" #include "unicode/usetiter.h" #else diff --git a/core/tests/unit/ldml/test_kmx_plus.cpp b/core/tests/unit/ldml/test_kmx_plus.cpp index 74961f797e..7f158dd1c3 100644 --- a/core/tests/unit/ldml/test_kmx_plus.cpp +++ b/core/tests/unit/ldml/test_kmx_plus.cpp @@ -152,12 +152,12 @@ int test_uset() { {0x127, 0x127} // [ħ] }; - USet u0(&r[0], 2); + SimpleUSet u0(&r[0], 2); assert_equal(u0.contains(0x62), true); // b assert_equal(u0.contains(0x41), false); // A assert_equal(u0.contains(0x127), true); // ħ - USet uempty; + SimpleUSet uempty; assert_equal(uempty.contains(0x62), false); assert_equal(uempty.contains(0x127), false); diff --git a/core/tests/unit/ldml/test_transforms.cpp b/core/tests/unit/ldml/test_transforms.cpp index d0ab45e1ce..db51b19ac3 100644 --- a/core/tests/unit/ldml/test_transforms.cpp +++ b/core/tests/unit/ldml/test_transforms.cpp @@ -166,7 +166,7 @@ test_reorder_standalone() { COMP_KMXPLUS_USET_RANGE(0x1A75, 0x1A79)}; const COMP_KMXPLUS_USET_USET usets[] = {{0, 1, 0xFFFFFFFF}}; const COMP_KMXPLUS_USET_USET &toneMarksUset = usets[0]; - const USet toneMarks(&ranges[toneMarksUset.range], toneMarksUset.count); + const SimpleUSet toneMarks(&ranges[toneMarksUset.range], toneMarksUset.count); // validate that the range [1A75, 1A79] matches assert_equal(toneMarks.contains(0x1A76), true); assert_equal(toneMarks.contains(0x1A60), false); From 548acaaf02ac168f6f5e3dc2bb32e3b3861cd7ae Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Fri, 11 Aug 2023 15:29:17 -0500 Subject: [PATCH 09/14] Apply suggestions from code review Co-authored-by: Marc Durdin --- core/src/meson.build | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/meson.build b/core/src/meson.build index 2f8c2a1882..87fd9181dd 100644 --- a/core/src/meson.build +++ b/core/src/meson.build @@ -30,7 +30,6 @@ if target_machine.system() == 'linux' icu_i18n = dependency('icu-i18n', required: true) else # load ICU from wrap - # Requires meson of about 0.57+ icu4c = subproject('icu-minimal', default_options: [ 'default_library=static', 'cpp_std=c++17', 'warning_level=0', 'werror=false']) # TODO-LDML: options: static, no data (these are in the meson build files) icu_uc = icu4c.get_variable('icuuc_dep') From ad03a5cdcf4503a6b35d36e41266724f28f69e00 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Fri, 11 Aug 2023 15:32:57 -0500 Subject: [PATCH 10/14] =?UTF-8?q?chore(core):=20regex=20cleanup=20?= =?UTF-8?q?=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - more comments For: #9121 --- core/src/ldml/ldml_transforms.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/ldml/ldml_transforms.cpp b/core/src/ldml/ldml_transforms.cpp index 64cc206053..6b4e074d37 100644 --- a/core/src/ldml/ldml_transforms.cpp +++ b/core/src/ldml/ldml_transforms.cpp @@ -421,6 +421,7 @@ transform_entry::match(const std::u32string &input) const { UErrorCode status = U_ZERO_ERROR; const std::u16string matchstr = km::kbp::kmx::u32string_to_u16string(input); icu::UnicodeString matchustr = icu::UnicodeString(matchstr.data(), (int32_t)matchstr.length()); + // TODO-LDML: create a new Matcher every time. These could be cached and reset. std::unique_ptr matcher(fFromPattern->matcher(matchustr, status)); assert(U_SUCCESS(status)); @@ -448,7 +449,7 @@ transform_entry::apply(const std::u32string &input, size_t matchLen) const { // TODO-LDML: Really? can't go from u32 to UnicodeString? assert(fFromPattern); - // TODO-LDML: simple approach, new regex every time + // TODO-LDML: simple approach, create a new Matcher every time. These could be cached and reset. // TODO-LDML: Really? can't go from u32 to UnicodeString? UErrorCode status = U_ZERO_ERROR; From 7a312520fbc4234379a01cc3f2b7e1ccaf996146 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Fri, 11 Aug 2023 16:32:09 -0500 Subject: [PATCH 11/14] =?UTF-8?q?feat(core):=20regex=20cleanup=20?= =?UTF-8?q?=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - restructure to not allocate the Matcher twice, save a little processing For: #9121 --- core/src/ldml/ldml_transforms.cpp | 115 +++++++++++++----------------- core/src/ldml/ldml_transforms.hpp | 22 +++--- 2 files changed, 60 insertions(+), 77 deletions(-) diff --git a/core/src/ldml/ldml_transforms.cpp b/core/src/ldml/ldml_transforms.cpp index 6b4e074d37..2d6afe8ff0 100644 --- a/core/src/ldml/ldml_transforms.cpp +++ b/core/src/ldml/ldml_transforms.cpp @@ -415,9 +415,10 @@ transform_entry::transform_entry(const std::u32string &from, const std::u32strin } size_t -transform_entry::match(const std::u32string &input) const { +transform_entry::apply(const std::u32string &input, std::u32string &output) const { assert(fFromPattern); // TODO-LDML: Really? can't go from u32 to UnicodeString? + // TODO-LDML: Also, we could cache the u16 string at the transformGroup level or higher. UErrorCode status = U_ZERO_ERROR; const std::u16string matchstr = km::kbp::kmx::u32string_to_u16string(input); icu::UnicodeString matchustr = icu::UnicodeString(matchstr.data(), (int32_t)matchstr.length()); @@ -426,67 +427,56 @@ transform_entry::match(const std::u32string &input) const { assert(U_SUCCESS(status)); if (!matcher->find(status)) { // i.e. matches somewhere, in this case at end of str - return 0; // and tear everything down + return 0; // no match } // TODO-LDML: this is UTF-16 len, not UTF-32 len!! - // auto matchLen = matcher->end64(status) - matcher->start64(status); // TODO-LDML: if we had an underlying UText this would be simpler. - auto matchStart = matcher->start64(status); - auto matchEnd = matcher->end64(status); - // extract.. - const icu::UnicodeString substr = matchustr.tempSubStringBetween((int32_t)matchStart, (int32_t)matchEnd); - // preflight to UTF-32 to get length - auto matchLen = substr.toUTF32(nullptr, 0, status); - - return matchLen; -} - -std::u32string -transform_entry::apply(const std::u32string &input, size_t matchLen) const { - // TODO-LDML: and if you thought the previous function was suboptimal, - // TODO-LDML: We should cache the RegexMatcher from the previous call. - // TODO-LDML: Really? can't go from u32 to UnicodeString? - - assert(fFromPattern); - // TODO-LDML: simple approach, create a new Matcher every time. These could be cached and reset. - // TODO-LDML: Really? can't go from u32 to UnicodeString? - UErrorCode status = U_ZERO_ERROR; - - // we know the matchLen so we can slice the string… - // Note: the matchstr here (unlike in transform_entry::match) is sliced so that - // it only includes the matched portion. This way, when changed it's suitable as the - // output string. - const std::u16string matchstr = km::kbp::kmx::u32string_to_u16string(input.substr(input.length() - matchLen, matchLen)); - icu::UnicodeString matchustr = icu::UnicodeString(matchstr.data(), (int32_t)matchstr.length()); - std::unique_ptr matcher(fFromPattern->matcher(matchustr, status)); + int32_t matchStart = matcher->start(status); + int32_t matchEnd = matcher->end(status); assert(U_SUCCESS(status)); - // now, do the replace + // extract.. + const icu::UnicodeString substr = matchustr.tempSubStringBetween(matchStart, matchEnd); + // preflight to UTF-32 to get length + UErrorCode substrStatus = U_ZERO_ERROR; + auto matchLen = substr.toUTF32(nullptr, 0, substrStatus); + assert(matchLen > 0); + if (matchLen == 0) { + return 0; + } + // Now, we have a matchLen. + + // now, do the replace. + // Convert the fTo into u16 TODO-LDML (we could cache this?) const std::u16string rstr = km::kbp::kmx::u32string_to_u16string(fTo); icu::UnicodeString rustr = icu::UnicodeString(rstr.data(), (int32_t)rstr.length()); // This replace will apply $1, $2 etc. TODO-LDML it will NOT handle mapFrom or mapTo. - icu::UnicodeString output = matcher->replaceFirst(rustr, status); + icu::UnicodeString entireOutput = matcher->replaceFirst(rustr, status); assert(U_SUCCESS(status)); + // entireOutput includes all of 'input', but modified. Need to substring it. + icu::UnicodeString outu = entireOutput.tempSubString(matchStart); - if (output.length() == 0) { - return std::u32string(); // special case of a zero length output (such as delete) + // Special case if there's no output + if (outu.length() == 0) { + output.clear(); + } else { + // TODO-LDML: All we are trying to do is to extract the output string. Probably too many steps. + UErrorCode preflightStatus = U_ZERO_ERROR; + // calculate how big the buffer is + auto out32len = outu.toUTF32(nullptr, 0, preflightStatus); // preflightStatus will be an err, because we know the buffer overruns zero bytes + // allocate + char32_t *s = new char32_t[out32len + 1]; + assert(s != nullptr); + // convert + outu.toUTF32((UChar32 *)s, out32len + 1, status); + assert(U_SUCCESS(status)); + output.assign(s, out32len); + // now, build a u32string + std::u32string out32(s, out32len); + // clean up buffer + delete [] s; } - - // TODO-LDML: All we are trying to do is to extract the output string. Probably too many steps. - UErrorCode preflightStatus = U_ZERO_ERROR; - // calculate how big the buffer is - auto out32len = output.toUTF32(nullptr, 0, preflightStatus); // preflightStatus will be an err, because we know the buffer overruns zero bytes - // allocate - char32_t *s = new char32_t[out32len + 1]; - assert(s != nullptr); - // convert - output.toUTF32((UChar32 *)s, out32len + 1, status); - assert(U_SUCCESS(status)); - // now, build a u32string - std::u32string out32(s, out32len); - // clean up buffer - delete [] s; - return out32; + return matchLen; } any_group::any_group(const transform_group &g) : type(any_group_type::transform), transform(g), reorder() { @@ -514,17 +504,18 @@ transform_group::transform_group() { /** * return the first transform match in this group */ -const transform_entry * -transform_group::match(const std::u32string &input, size_t &subMatched) const { +size_t +transform_group::apply(const std::u32string &input, std::u32string &output) const { + size_t subMatched = 0; for (auto transform = begin(); (subMatched == 0) && (transform < end()); transform++) { // TODO-LDML: non regex implementation // is the match area too short? - subMatched = transform->match(input); + subMatched = transform->apply(input, output); if (subMatched != 0) { - return &(*transform); // return alias to transform + return subMatched; // matched. break out. } } - return nullptr; + return 0; // no match } /** @@ -577,20 +568,14 @@ transforms::apply(const std::u32string &input, std::u32string &output) { // TODO-LDML: reorders // Assume it's a non reorder group /** Length of match within this group*/ - size_t subMatched = 0; // find the first match in this group (if present) // TODO-LDML: check if reorder if (group->type == any_group_type::transform) { - auto entry = group->transform.match(updatedInput, subMatched); - - if (entry != nullptr) { - // now apply the found transform - - // update subOutput (string) and subMatched - // the returned string must replace the last "subMatched" chars of the string. - std::u32string subOutput = entry->apply(updatedInput, subMatched); + std::u32string subOutput; + size_t subMatched = group->transform.apply(updatedInput, subOutput); + if (subMatched != 0) { // remove the matched part of the updatedInput updatedInput.resize(updatedInput.length() - subMatched); // chop of the subMatched part at end updatedInput.append(subOutput); // subOutput could be empty such as in backspace transform diff --git a/core/src/ldml/ldml_transforms.hpp b/core/src/ldml/ldml_transforms.hpp index 56a4117c4d..cbe1b03dce 100644 --- a/core/src/ldml/ldml_transforms.hpp +++ b/core/src/ldml/ldml_transforms.hpp @@ -86,14 +86,12 @@ public: ); /** - * @returns length if it's a match + * If matching, apply the match to the output string + * @param input input string to match + * @param output output string + * @returns length of 'input' which was matched */ - size_t match(const std::u32string &input) const; - - /** - * @returns output string - */ - std::u32string apply(const std::u32string &input, size_t matchLen) const; + size_t apply(const std::u32string &input, std::u32string &output) const; private: const std::u32string fFrom; @@ -114,12 +112,12 @@ public: transform_group(); /** - * Find the first match in the group + * Find the first match in the group and apply it. * @param input input string to match - * @param subMatched on output, the matched length - * @returns alias to transform_entry or nullptr + * @param output output string + * @returns length of 'input' which was matched */ - const transform_entry *match(const std::u32string &input, size_t &subMatched) const; + size_t apply(const std::u32string &input, std::u32string &output) const; }; /** a single char, categorized according to reorder rules*/ @@ -241,7 +239,7 @@ public: size_t apply(const std::u32string &input, std::u32string &output); /** - * For tests - TODO-LDML only supports reorder + * For tests * @return true if str was altered */ bool apply(std::u32string &str); From 960a55949e591c5e83e27ad00d90b83d0b01fe86 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Fri, 11 Aug 2023 17:56:15 -0500 Subject: [PATCH 12/14] =?UTF-8?q?feat(core):=20icu=20build=20fix=20?= =?UTF-8?q?=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - correctly disable some ICU features For: #9121 --- core/subprojects/packagefiles/icu/meson.build | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/core/subprojects/packagefiles/icu/meson.build b/core/subprojects/packagefiles/icu/meson.build index 32bc209139..b525a35ae8 100644 --- a/core/subprojects/packagefiles/icu/meson.build +++ b/core/subprojects/packagefiles/icu/meson.build @@ -26,9 +26,8 @@ endif # per icudefs.mk.in: # "U_ATTRIBUTE_DEPRECATED is defined to hide warnings about deprecated API warnings." -add_project_arguments('-DU_ATTRIBUTE_DEPRECATED=', '-DUCONFIG_USE_LOCAL=1', language: 'c') - -add_project_arguments('-DU_NOEXCEPT=', language: 'c') +add_project_arguments('-DU_NOEXCEPT=', '-DU_ATTRIBUTE_DEPRECATED=', '-DUCONFIG_USE_LOCAL=1', language: 'c') +add_project_arguments('-DU_NOEXCEPT=', '-DU_ATTRIBUTE_DEPRECATED=', '-DUCONFIG_USE_LOCAL=1', language: 'cpp') uconfig = configuration_data() @@ -42,7 +41,7 @@ uconfig.set('UCONFIG_NO_IDNA', 1) uconfig.set('UCONFIG_NO_COLLATION', 1) uconfig.set('UCONFIG_NO_FORMATTING', 1) uconfig.set('UCONFIG_NO_TRANSLITERATION', 1) -uconfig.set('UCONFIG_NO_REGULAR_EXPRESSIONS', 1) # TODO-LDML: Will probably want this for transforms #7375 +uconfig.set('UCONFIG_NO_REGULAR_EXPRESSIONS', 0) # want these for transforms #7375 uconfig.set('UCONFIG_NO_SERVICE', 1) From 86dba54c068184d27eecc5f37c4be4dcc0bc4cb3 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Mon, 14 Aug 2023 19:05:55 -0500 Subject: [PATCH 13/14] =?UTF-8?q?feat(core):=20icu=20build=20fix=20?= =?UTF-8?q?=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - trim some more files For: #9121 --- core/subprojects/packagefiles/icu/meson.build | 4 ++ .../icu/source/common/meson.build | 56 +++++++++---------- .../packagefiles/icu/source/i18n/meson.build | 2 +- 3 files changed, 33 insertions(+), 29 deletions(-) diff --git a/core/subprojects/packagefiles/icu/meson.build b/core/subprojects/packagefiles/icu/meson.build index b525a35ae8..128fc9fbf9 100644 --- a/core/subprojects/packagefiles/icu/meson.build +++ b/core/subprojects/packagefiles/icu/meson.build @@ -43,6 +43,10 @@ uconfig.set('UCONFIG_NO_FORMATTING', 1) uconfig.set('UCONFIG_NO_TRANSLITERATION', 1) uconfig.set('UCONFIG_NO_REGULAR_EXPRESSIONS', 0) # want these for transforms #7375 uconfig.set('UCONFIG_NO_SERVICE', 1) +uconfig.set('U_OVERRIDE_CXX_ALLOCATION', 1) +uconfig.set('UCONFIG_NO_CONVERSION', 1) +uconfig.set('UCONFIG_USE_WINDOWS_LCID_MAPPING_API', 0) + uconfig_local = configure_file( diff --git a/core/subprojects/packagefiles/icu/source/common/meson.build b/core/subprojects/packagefiles/icu/source/common/meson.build index 4653d88383..4694499a86 100644 --- a/core/subprojects/packagefiles/icu/source/common/meson.build +++ b/core/subprojects/packagefiles/icu/source/common/meson.build @@ -75,10 +75,10 @@ sources = files( 'stringpiece.cpp', 'stringtriebuilder.cpp', 'uarrsort.cpp', - 'ubidi.cpp', + # 'ubidi.cpp', 'ubidi_props.cpp', - 'ubidiln.cpp', - 'ubidiwrt.cpp', + # 'ubidiln.cpp', + # 'ubidiwrt.cpp', 'ubrk.cpp', 'ucase.cpp', 'ucasemap.cpp', @@ -91,29 +91,29 @@ sources = files( 'uchriter.cpp', 'ucln_cmn.cpp', 'ucmndata.cpp', - 'ucnv.cpp', - 'ucnv2022.cpp', - 'ucnv_bld.cpp', - 'ucnv_cb.cpp', - 'ucnv_cnv.cpp', - 'ucnv_ct.cpp', - 'ucnv_err.cpp', - 'ucnv_ext.cpp', - 'ucnv_io.cpp', - 'ucnv_lmb.cpp', - 'ucnv_set.cpp', - 'ucnv_u16.cpp', - 'ucnv_u32.cpp', - 'ucnv_u7.cpp', - 'ucnv_u8.cpp', - 'ucnvbocu.cpp', - 'ucnvdisp.cpp', - 'ucnvhz.cpp', - 'ucnvisci.cpp', - 'ucnvlat1.cpp', - 'ucnvmbcs.cpp', - 'ucnvscsu.cpp', - 'ucnvsel.cpp', + # 'ucnv.cpp', + # 'ucnv2022.cpp', + # 'ucnv_bld.cpp', + # 'ucnv_cb.cpp', + # 'ucnv_cnv.cpp', + # 'ucnv_ct.cpp', + # 'ucnv_err.cpp', + # 'ucnv_ext.cpp', + # 'ucnv_io.cpp', + # 'ucnv_lmb.cpp', + # 'ucnv_set.cpp', + # 'ucnv_u16.cpp', + # 'ucnv_u32.cpp', + # 'ucnv_u7.cpp', + # 'ucnv_u8.cpp', + # 'ucnvbocu.cpp', + # 'ucnvdisp.cpp', + # 'ucnvhz.cpp', + # 'ucnvisci.cpp', + # 'ucnvlat1.cpp', + # 'ucnvmbcs.cpp', + # 'ucnvscsu.cpp', + # 'ucnvsel.cpp', 'ucol_swp.cpp', 'ucptrie.cpp', 'ucurr.cpp', @@ -287,8 +287,8 @@ headers = files( 'unicode/stringpiece.h', 'unicode/stringtriebuilder.h', 'unicode/symtable.h', - 'unicode/ubidi.h', - 'unicode/ubiditransform.h', + # 'unicode/ubidi.h', + # 'unicode/ubiditransform.h', 'unicode/ubrk.h', 'unicode/ucasemap.h', 'unicode/ucat.h', diff --git a/core/subprojects/packagefiles/icu/source/i18n/meson.build b/core/subprojects/packagefiles/icu/source/i18n/meson.build index 787afe3390..b2e64e63be 100644 --- a/core/subprojects/packagefiles/icu/source/i18n/meson.build +++ b/core/subprojects/packagefiles/icu/source/i18n/meson.build @@ -52,7 +52,7 @@ sources = files( 'dayperiodrules.cpp', 'dcfmtsym.cpp', 'decContext.cpp', - 'decNumber.cpp', + # 'decNumber.cpp', 'decimfmt.cpp', 'displayoptions.cpp', 'double-conversion-bignum-dtoa.cpp', From 7f05335fc3d4031c504757a3e6f1de47b8a2c9ff Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Tue, 15 Aug 2023 15:11:38 -0500 Subject: [PATCH 14/14] =?UTF-8?q?chore(window):=20fix=20windows=20build=20?= =?UTF-8?q?for=20ICU=20=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - for regex (and eventually normalization) #9121 --- windows/src/engine/keyman32/keyman32.vcxproj | 8 ++++---- windows/src/engine/keyman64/keyman64.vcxproj | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/windows/src/engine/keyman32/keyman32.vcxproj b/windows/src/engine/keyman32/keyman32.vcxproj index 670df3c1d4..8688133787 100644 --- a/windows/src/engine/keyman32/keyman32.vcxproj +++ b/windows/src/engine/keyman32/keyman32.vcxproj @@ -55,11 +55,11 @@ - $(ProjectDir)..\..\..\..\core\build\x86\$(Configuration)\src;$(ProjectDir)..\..\..\..\core\build\rust\x86\$(Configuration);$(LibraryPath) + $(ProjectDir)..\..\..\..\core\build\x86\$(Configuration)\src;$(ProjectDir)..\..\..\..\core\build\x86\$(Configuration)\subprojects\icu\source\common;$(ProjectDir)..\..\..\..\core\build\x86\$(Configuration)\subprojects\icu\source\i18n;$(ProjectDir)..\..\..\..\core\build\rust\x86\$(Configuration);$(LibraryPath) $(ProjectDir)..\..\..\..\common\include;$(ProjectDir)..\..\..\..\core\include;$(ProjectDir)..\..\..\..\core\build\x86\$(Configuration)\include;$(IncludePath) - $(ProjectDir)..\..\..\..\core\build\x86\$(Configuration)\src;$(ProjectDir)..\..\..\..\core\build\rust\x86\$(Configuration);$(LibraryPath) + $(ProjectDir)..\..\..\..\core\build\x86\$(Configuration)\src;$(ProjectDir)..\..\..\..\core\build\x86\$(Configuration)\subprojects\icu\source\common;$(ProjectDir)..\..\..\..\core\build\x86\$(Configuration)\subprojects\icu\source\i18n;$(ProjectDir)..\..\..\..\core\build\rust\x86\$(Configuration);$(LibraryPath) $(ProjectDir)..\..\..\..\common\include;$(ProjectDir)..\..\..\..\core\include;$(ProjectDir)..\..\..\..\core\build\x86\$(Configuration)\include;$(IncludePath) @@ -96,7 +96,7 @@ /verbose:lib /section:.SHARDATA,rws %(AdditionalOptions) - libkmnkbp0.a;psapi.lib;rpcrt4.lib;version.lib;setupapi.lib;iphlpapi.lib;imm32.lib;crypt32.lib;wintrust.lib;imagehlp.lib;ws2_32.lib;%(AdditionalDependencies) + libicuuc.a;libicuin.a;libkmnkbp0.a;psapi.lib;rpcrt4.lib;version.lib;setupapi.lib;iphlpapi.lib;imm32.lib;crypt32.lib;wintrust.lib;imagehlp.lib;ws2_32.lib;%(AdditionalDependencies) true keyman32.def true @@ -152,7 +152,7 @@ /verbose:lib /section:.SHARDATA,rws %(AdditionalOptions) - libkmnkbp0.a;psapi.lib;rpcrt4.lib;version.lib;setupapi.lib;iphlpapi.lib;imm32.lib;crypt32.lib;wintrust.lib;imagehlp.lib;ws2_32.lib;%(AdditionalDependencies) + libicuuc.a;libicuin.a;libkmnkbp0.a;psapi.lib;rpcrt4.lib;version.lib;setupapi.lib;iphlpapi.lib;imm32.lib;crypt32.lib;wintrust.lib;imagehlp.lib;ws2_32.lib;%(AdditionalDependencies) true keyman32.def true diff --git a/windows/src/engine/keyman64/keyman64.vcxproj b/windows/src/engine/keyman64/keyman64.vcxproj index 15546caef5..89b7ffa2e1 100644 --- a/windows/src/engine/keyman64/keyman64.vcxproj +++ b/windows/src/engine/keyman64/keyman64.vcxproj @@ -54,11 +54,11 @@ - $(ProjectDir)..\..\..\..\core\build\x64\$(Configuration)\src;$(ProjectDir)..\..\..\..\core\build\rust\x64\$(Configuration);$(VC_LibraryPath_x64);$(WindowsSDK_LibraryPath_x64) + $(ProjectDir)..\..\..\..\core\build\x64\$(Configuration)\src;$(ProjectDir)..\..\..\..\core\build\x64\$(Configuration)\subprojects\icu\source\common;$(ProjectDir)..\..\..\..\core\build\x64\$(Configuration)\subprojects\icu\source\i18n;$(ProjectDir)..\..\..\..\core\build\rust\x64\$(Configuration);$(VC_LibraryPath_x64);$(WindowsSDK_LibraryPath_x64) $(ProjectDir)..\..\..\..\common\include;$(ProjectDir)..\..\..\..\core\build\x64\$(Configuration)\include;$(ProjectDir)..\..\..\..\core\include;$(IncludePath) - $(ProjectDir)..\..\..\..\core\build\x64\$(Configuration)\src;$(ProjectDir)..\..\..\..\core\build\rust\x64\$(Configuration);$(VC_LibraryPath_x64);$(WindowsSDK_LibraryPath_x64) + $(ProjectDir)..\..\..\..\core\build\x64\$(Configuration)\src;$(ProjectDir)..\..\..\..\core\build\x64\$(Configuration)\subprojects\icu\source\common;$(ProjectDir)..\..\..\..\core\build\x64\$(Configuration)\subprojects\icu\source\i18n;$(ProjectDir)..\..\..\..\core\build\rust\x64\$(Configuration);$(VC_LibraryPath_x64);$(WindowsSDK_LibraryPath_x64) $(ProjectDir)..\..\..\..\common\include;$(ProjectDir)..\..\..\..\core\build\x64\$(Configuration)\include;$(ProjectDir)..\..\..\..\core\include;$(IncludePath) @@ -96,7 +96,7 @@ /verbose:lib /section:.SHARDATA,rws %(AdditionalOptions) - libkmnkbp0.a;psapi.lib;version.lib;setupapi.lib;iphlpapi.lib;imm32.lib;crypt32.lib;wintrust.lib;imagehlp.lib;ws2_32.lib;libcmt.lib;%(AdditionalDependencies) + libicuuc.a;libicuin.a;libkmnkbp0.a;psapi.lib;version.lib;setupapi.lib;iphlpapi.lib;imm32.lib;crypt32.lib;wintrust.lib;imagehlp.lib;ws2_32.lib;libcmt.lib;%(AdditionalDependencies) true C:\Program Files\Microsoft SDKs\Windows\v7.0\Lib\x64;$(VCInstallDir)lib\amd64;$(VCInstallDir)lib;%(AdditionalLibraryDirectories) false @@ -152,7 +152,7 @@ /verbose:lib /section:.SHARDATA,rws %(AdditionalOptions) - libkmnkbp0.a;psapi.lib;version.lib;setupapi.lib;iphlpapi.lib;imm32.lib;crypt32.lib;wintrust.lib;imagehlp.lib;ws2_32.lib;libcmt.lib;%(AdditionalDependencies) + libicuuc.a;libicuin.a;libkmnkbp0.a;psapi.lib;version.lib;setupapi.lib;iphlpapi.lib;imm32.lib;crypt32.lib;wintrust.lib;imagehlp.lib;ws2_32.lib;libcmt.lib;%(AdditionalDependencies) true C:\Program Files\Microsoft SDKs\Windows\v7.0\Lib\x64;$(VCInstallDir)lib\amd64;$(VCInstallDir)lib;%(AdditionalLibraryDirectories) false