From a765f6c2e11e7557e1184d159952db6f92abca21 Mon Sep 17 00:00:00 2001 From: John Lambert Date: Thu, 13 Aug 2026 15:52:05 -0400 Subject: [PATCH 1/3] perf: speed up bulk string replacement Compute each bulk preview value once. Replace all matches through one native ICU search session. Preserve rich text, Unicode collation, and legacy fallback behavior. Record measured gains and discarded experiments. --- ...ulk-replacement-distillation-2026-08-13.md | 111 ++++ Src/Common/Controls/XMLViews/BulkEditBar.cs | 104 ++-- Src/views/Test/TestVwPattern.h | 314 +++++++++++ Src/views/Views.idh | 15 +- Src/views/VwPattern.cpp | 288 ++++++++-- Src/views/VwPattern.h | 7 +- .../Performance/ReplaceWithMethodTests.cs | 489 +++++++++++++++++ .../Performance/VwPatternReplacementTests.cs | 490 ++++++++++++++++++ Src/xWorks/xWorksTests/BulkEditBarTests.cs | 137 +++++ 9 files changed, 1855 insertions(+), 100 deletions(-) create mode 100644 Docs/superpowers/plans/evidence/bulk-replacement-distillation-2026-08-13.md create mode 100644 Src/xWorks/xWorksTests/Avalonia/Performance/ReplaceWithMethodTests.cs create mode 100644 Src/xWorks/xWorksTests/Avalonia/Performance/VwPatternReplacementTests.cs diff --git a/Docs/superpowers/plans/evidence/bulk-replacement-distillation-2026-08-13.md b/Docs/superpowers/plans/evidence/bulk-replacement-distillation-2026-08-13.md new file mode 100644 index 0000000000..adf61765e6 --- /dev/null +++ b/Docs/superpowers/plans/evidence/bulk-replacement-distillation-2026-08-13.md @@ -0,0 +1,111 @@ +# Bulk replacement optimization evidence + +## Retained scope + +The focused implementation retains four changes: + +- Avoid normalization work when a replacement result is already NFD. +- Compute each preview value once and reuse it for the enabled result. +- Expose one-session rich-string replacement through the optional `IVwPattern2` + capability without changing the `IVwPattern` ABI or coclass default. +- Use the capability when available and preserve the repeated-`FindIn` fallback. + +The correctness corpus covers literal and regular-expression replacement, +captures, zero-width matches, whole-word and case/diacritic options, Unicode +normalization and scripts, ICU tailoring, rich-text properties, object +replacement characters, capability fallback, failures, and terminal state. + +## Single-pass preview measurement + +This is an incremental comparison of the single-pass preview change, not a +branch-point-to-final result. Both builds used Debug x64, 50,000 real entries, +100 percent matching CitationForm values with one match each, one warmup, and +five timed preview repetitions per process. Process order was B-C-B-C. + +- Baseline process 1: 73.973706, 87.429190, 85.850784, 89.671968, + 93.842072 microseconds/entry; median 87.429190. +- Candidate process 1: 53.317292, 49.020970, 49.454658, 47.650816, + 54.720194; median 49.454658. +- Baseline process 2: 68.099794, 68.588244, 70.913828, 70.782702, + 69.719412; median 69.719412. +- Candidate process 2: 48.884480, 49.327828, 58.699344, 78.705420, + 90.564270; median 58.699344. +- Pooled medians: baseline 72.443767, candidate 51.385975 + microseconds/entry; 29.068 percent reduction and 1.409796x throughput. + +The second candidate process was retried unchanged after a transient test +discovery failure produced no timing result. + +## ReplaceAllIn measurement + +This is an incremental comparison of `ReplaceAllIn`, not a cumulative branch +result. Both trees used `9307443e494d23066d9d40bc41c4d4b30944b972`. +The baseline changed only the managed capability assignment from the +`IVwPattern2` cast to `null`; native and search code was identical. Both used +`build.ps1 -BuildTests -Serial`, Debug x64, and the same temporary harness +(SHA-256 `52AB194E89486BFB94722A444B077B894B84128B46F8F0CF7EDD62DAE48D5481`). + +Each fresh process used 50,000 real entries. Matching values were +`old old old old-######` (22 UTF-16 units and four matches); unmatched values +were `keep keep keep-######` (21 units). Each fresh process had one warmup and +five timed apply repetitions. Reset, garbage collection, validation, and undo +were outside the stopwatch. There was no cache prepass. All comparisons +reported zero mismatches. + +### 25 percent matching + +- Baseline: 109.070736, 108.331108, 108.476428, 109.597136, 109.750080; + median 109.070736 microseconds/entry. +- Candidate: 80.400070, 86.023792, 81.024132, 83.734014, 100.501314; + median 83.734014. +- Incremental result: 23.23 percent reduction and 1.303x throughput. + +### 50 percent matching + +- Baseline process 1: 235.016240, 278.735516, 215.068208, 291.738206, + 245.199594; median 245.199594. +- Candidate process 1: 160.970294, 161.559868, 161.132232, 161.602208, + 178.361492; median 161.559868. +- Baseline process 2: 220.388066, 222.015458, 215.116946, 233.213528, + 239.261800; median 222.015458. +- Candidate process 2: 172.943354, 173.446744, 170.288498, 194.350902, + 182.141472; median 173.446744. +- Pooled medians: baseline 234.114884, candidate 171.615926; 26.69 percent + reduction and 1.364x throughput. The two paired reductions were 34.11 and + 21.88 percent. + +### 100 percent matching + +- Baseline process 1: 468.296628, 437.760106, 437.642286, 586.596700, + 518.897366; median 468.296628. +- Candidate process 1: 316.618992, 314.000950, 337.447102, 317.614622, + 315.852194; median 316.618992. +- Baseline process 2: 421.689880, 425.018894, 428.718602, 444.715500, + 429.318474; median 428.718602. +- Candidate process 2: 325.698984, 335.498302, 325.648910, 364.891896, + 362.333156; median 335.498302. +- Pooled medians: baseline 448.507615, candidate 326.058643; 27.30 percent + reduction and 1.376x throughput. The two paired reductions were 32.39 and + 21.75 percent. + +Debug-process variance was material, especially in the first baseline +process. The raw sample bands stayed separated, and each independent 50 and +100 percent pair exceeded the 15 percent acceptance gate. These measurements +cover the tested short CitationForm values only. + +## Rejected and deferred work + +- A printable-ASCII `memcmp` shortcut was reverted because ICU collation can + equate strings that differ in punctuation and other non-ordinal ways. +- A managed ordinal negative precheck was exactly reverted because it did not + produce sufficient benefit. +- A one-character collation-ignorable cache and early extension exit were + reverted. Its answer survived a locale change and produced an incorrect + match span; the single-character premise also lacked an ICU contract for + contextual collation. +- The virtual table and budgeted coordinator were test-owned prototypes with + no shipping activation path. They and their acceptance and attribution + harnesses were discarded. +- Homograph renumber batching was deferred after characterization only. It + needs separate cross-repository design and measurements for undo, redo, + notifications, cache membership, and group numbering. diff --git a/Src/Common/Controls/XMLViews/BulkEditBar.cs b/Src/Common/Controls/XMLViews/BulkEditBar.cs index 80affc51f0..356370c26f 100644 --- a/Src/Common/Controls/XMLViews/BulkEditBar.cs +++ b/Src/Common/Controls/XMLViews/BulkEditBar.cs @@ -4827,9 +4827,10 @@ public void FakeDoit(IEnumerable itemsToChange, int tagFakeFlid, int tagEna state.PercentDone = i * 100 / itemsToChange.Count(); state.Breath(); } - bool fEnable = OkToChange(hvo); + ITsString newValue; + bool fEnable = TryGetNewValue(hvo, out newValue); if (fEnable) - m_sda.SetString(hvo, tagFakeFlid, NewValue(hvo)); + m_sda.SetString(hvo, tagFakeFlid, newValue); m_sda.SetInt(hvo, tagEnable, (fEnable ? 1 : 0)); } } @@ -4861,10 +4862,9 @@ public void Doit(IEnumerable itemsToChange, ProgressState state) /// public virtual void Doit(int hvo) { - if (OkToChange(hvo)) - { - SetNewValue(hvo, NewValue(hvo)); - } + ITsString newValue; + if (TryGetNewValue(hvo, out newValue)) + SetNewValue(hvo, newValue); } /// @@ -4903,6 +4903,18 @@ protected virtual bool OkToChange(int hvo) return true; } + /// + /// Gets the new value when the item can change. + /// + protected virtual bool TryGetNewValue(int hvo, out ITsString newValue) + { + newValue = null; + if (!OkToChange(hvo)) + return false; + newValue = NewValue(hvo); + return newValue != null; + } + protected abstract ITsString NewValue(int hvo); #region IGetReplacedObjects Members @@ -5065,35 +5077,19 @@ protected override ITsString NewValue(int hvo) internal class ReplaceWithMethod : DoItMethod { - IVwPattern m_pattern; - ITsString m_replacement; - IVwTxtSrcInit m_textSourceInit; - IVwTextSource m_ts; + private readonly IVwPattern m_pattern; + private readonly IVwPattern2 m_bulkPattern; + private readonly IVwTxtSrcInit m_textSourceInit; + private readonly IVwTextSource m_ts; public ReplaceWithMethod(LcmCache cache, ISilDataAccessManaged sda, FieldReadWriter accessor, XmlNode spec, IVwPattern pattern, ITsString replacement) : base(cache, sda, accessor, spec) { m_pattern = pattern; - m_replacement = replacement; - m_pattern.ReplaceWith = m_replacement; + m_pattern.ReplaceWith = replacement; + m_bulkPattern = m_pattern as IVwPattern2; m_textSourceInit = VwStringTextSourceClass.Create(); - m_ts = m_textSourceInit as IVwTextSource; - } - - /// - /// We can do a replace if the pattern matches. - /// - /// - /// - protected override bool OkToChange(int hvo) - { - if (!base.OkToChange(hvo)) - return false; - ITsString tss = OldValue(hvo) ?? TsStringUtils.EmptyString(m_accessor.WritingSystem); - m_textSourceInit.SetString(tss); - int ichMin, ichLim; - m_pattern.FindIn(m_ts, 0, tss.Length, true, out ichMin, out ichLim, null); - return ichMin >= 0; + m_ts = (IVwTextSource)m_textSourceInit; } /// @@ -5104,6 +5100,13 @@ protected override bool OkToChange(int hvo) protected override ITsString NewValue(int hvo) { ITsString tss = OldValue(hvo) ?? TsStringUtils.EmptyString(m_accessor.WritingSystem); + ITsString bulkResult; + int matchCount; + if (TryReplaceAllIn(tss, out bulkResult, out matchCount)) + { + return matchCount == 0 ? null : NormalizeResult(bulkResult); + } + m_textSourceInit.SetString(tss); int ichStartSearch = 0; ITsStrBldr tsb = null; @@ -5124,7 +5127,7 @@ protected override ITsString NewValue(int hvo) for ( ; ichStartSearch <= cch; ) { int ichMin, ichLim; - m_pattern.FindIn(m_ts, ichStartSearch, cch, true, out ichMin, out ichLim, null); + FindIn(ichStartSearch, cch, out ichMin, out ichLim); if (ichMin < 0) break; if (ichLim == ichLimLastMatch) @@ -5142,23 +5145,40 @@ protected override ITsString NewValue(int hvo) } if (tsb == null) return null; - return tsb.GetString().get_NormalizedForm(FwNormalizationMode.knmNFD); + return NormalizeResult(tsb.GetString()); + } + + private static ITsString NormalizeResult(ITsString tssResult) + { + string text = tssResult.Text; + if (!string.IsNullOrEmpty(text) && + CustomIcu.GetIcuNormalizer(FwNormalizationMode.knmNFD).IsNormalized(text)) + { + return tssResult; + } + return tssResult.get_NormalizedForm(FwNormalizationMode.knmNFD); } /// - /// This is very like the base Doit, but we can save a duplicate pattern search - /// by calling the BASE version of OkToChange rather than our own version, which - /// tests for at least one match. We DO need to call the base version, e.g., so - /// we don't change wordforms which shouldn't change because they are in use. + /// Attempts bulk replacement when supported. /// - /// - public override void Doit(int hvo) + protected virtual bool TryReplaceAllIn(ITsString source, out ITsString result, + out int matchCount) { - if (!base.OkToChange(hvo)) - return; - ITsString tss = NewValue(hvo); - if (tss != null) - SetNewValue(hvo, tss); + if (m_bulkPattern == null) + { + result = null; + matchCount = 0; + return false; + } + + result = m_bulkPattern.ReplaceAllIn(source, 0, source.Length, out matchCount); + return true; + } + + protected virtual void FindIn(int ichStart, int ichEnd, out int ichMin, out int ichLim) + { + m_pattern.FindIn(m_ts, ichStart, ichEnd, true, out ichMin, out ichLim, null); } } /// diff --git a/Src/views/Test/TestVwPattern.h b/Src/views/Test/TestVwPattern.h index dac2acd22b..f19d3a442d 100644 --- a/Src/views/Test/TestVwPattern.h +++ b/Src/views/Test/TestVwPattern.h @@ -73,6 +73,88 @@ namespace TestViews } }; + class FailSecondLockTsString : public ITsString + { + long m_cref; + ITsStringPtr m_qtss; + int m_cLocks; + + public: + FailSecondLockTsString(ITsString * ptss) : m_cref(1), m_qtss(ptss), m_cLocks(0) + { + } + + STDMETHOD(QueryInterface)(REFIID riid, void ** ppv) + { + if (!ppv) + return E_POINTER; + *ppv = NULL; + if (riid != IID_IUnknown && riid != IID_ITsString) + return E_NOINTERFACE; + *ppv = static_cast(this); + AddRef(); + return S_OK; + } + STDMETHOD_(UCOMINT32, AddRef)() { return InterlockedIncrement(&m_cref); } + STDMETHOD_(UCOMINT32, Release)() + { + long cref = InterlockedDecrement(&m_cref); + if (!cref) + delete this; + return cref; + } + STDMETHOD(get_Text)(BSTR * pbstr) { return m_qtss->get_Text(pbstr); } + STDMETHOD(get_Length)(int * pcch) { return m_qtss->get_Length(pcch); } + STDMETHOD(get_RunCount)(int * pcrun) { return m_qtss->get_RunCount(pcrun); } + STDMETHOD(get_RunAt)(int ich, int * pirun) { return m_qtss->get_RunAt(ich, pirun); } + STDMETHOD(get_MinOfRun)(int irun, int * pichMin) + { return m_qtss->get_MinOfRun(irun, pichMin); } + STDMETHOD(get_LimOfRun)(int irun, int * pichLim) + { return m_qtss->get_LimOfRun(irun, pichLim); } + STDMETHOD(GetBoundsOfRun)(int irun, int * pichMin, int * pichLim) + { return m_qtss->GetBoundsOfRun(irun, pichMin, pichLim); } + STDMETHOD(FetchRunInfoAt)(int ich, TsRunInfo * ptri, ITsTextProps ** ppttp) + { return m_qtss->FetchRunInfoAt(ich, ptri, ppttp); } + STDMETHOD(FetchRunInfo)(int irun, TsRunInfo * ptri, ITsTextProps ** ppttp) + { return m_qtss->FetchRunInfo(irun, ptri, ppttp); } + STDMETHOD(get_RunText)(int irun, BSTR * pbstr) + { return m_qtss->get_RunText(irun, pbstr); } + STDMETHOD(GetChars)(int ichMin, int ichLim, BSTR * pbstr) + { return m_qtss->GetChars(ichMin, ichLim, pbstr); } + STDMETHOD(FetchChars)(int ichMin, int ichLim, OLECHAR * prgch) + { return m_qtss->FetchChars(ichMin, ichLim, prgch); } + STDMETHOD(LockText)(const OLECHAR ** pprgch, int * pcch) + { + if (++m_cLocks == 2) + return E_FAIL; + return m_qtss->LockText(pprgch, pcch); + } + STDMETHOD(UnlockText)(const OLECHAR * prgch) { return m_qtss->UnlockText(prgch); } + STDMETHOD(LockRun)(int irun, const OLECHAR ** pprgch, int * pcch) + { return m_qtss->LockRun(irun, pprgch, pcch); } + STDMETHOD(UnlockRun)(int irun, const OLECHAR * prgch) + { return m_qtss->UnlockRun(irun, prgch); } + STDMETHOD(get_PropertiesAt)(int ich, ITsTextProps ** ppttp) + { return m_qtss->get_PropertiesAt(ich, ppttp); } + STDMETHOD(get_Properties)(int irun, ITsTextProps ** ppttp) + { return m_qtss->get_Properties(irun, ppttp); } + STDMETHOD(GetBldr)(ITsStrBldr ** pptsb) { return m_qtss->GetBldr(pptsb); } + STDMETHOD(GetIncBldr)(ITsIncStrBldr ** pptisb) + { return m_qtss->GetIncBldr(pptisb); } + STDMETHOD(Equals)(ITsString * ptss, ComBool * pfEqual) + { return m_qtss->Equals(ptss, pfEqual); } + STDMETHOD(get_IsNormalizedForm)(FwNormalizationMode nm, ComBool * pfRet) + { return m_qtss->get_IsNormalizedForm(nm, pfRet); } + STDMETHOD(get_NormalizedForm)(FwNormalizationMode nm, ITsString ** pptssRet) + { return m_qtss->get_NormalizedForm(nm, pptssRet); } + STDMETHOD(NfdAndFixOffsets)(ITsString ** pptssRet, int ** prgpichOffsetsToFix, + int cichOffsetsToFix) + { + return m_qtss->NfdAndFixOffsets(pptssRet, prgpichOffsetsToFix, cichOffsetsToFix); + } + STDMETHOD(GetSubstring)(int ichMin, int ichLim, ITsString ** pptssRet) + { return m_qtss->GetSubstring(ichMin, ichLim, pptssRet); } + }; class TestVwPattern : public unitpp::suite { public: @@ -80,6 +162,7 @@ namespace TestViews ILgWritingSystemPtr m_qwsEng; ITsStrFactoryPtr m_qtsf; IVwPatternPtr m_qpat; + IVwPattern2Ptr m_qpat2; VwTxtSrcPtr m_qts; VwPropertyStorePtr m_qzvps; @@ -1622,6 +1705,235 @@ namespace TestViews unitpp::assert_eq("Length of 'in' (search 3)", ichMin + 2, ichLim); } + ITsStringPtr ReplaceWithRepeatedFindIn(ITsString * ptssSource, int ichStart, + int ichEnd, int * pcMatches) + { + IVwTxtSrcInitPtr qtsi; + qtsi.CreateInstance(CLSID_VwStringTextSource); + CheckHr(qtsi->SetString(ptssSource)); + IVwTextSourcePtr qts; + CheckHr(qtsi->QueryInterface(IID_IVwTextSource, (void **)&qts)); + ITsStrBldrPtr qtsb; + int ichStartSearch = ichStart; + int ichLimLastMatch = -1; + int cchDelta = 0; + *pcMatches = 0; + while (ichStartSearch <= ichEnd) + { + int ichMin; + int ichLim; + CheckHr(m_qpat->FindIn(qts, ichStartSearch, ichEnd, true, &ichMin, &ichLim, NULL)); + if (ichMin < 0) + break; + if (ichLim == ichLimLastMatch) + { + ichStartSearch = ichLim + 1; + continue; + } + ichLimLastMatch = ichLim; + ITsStringPtr qtssReplacement; + CheckHr(m_qpat->get_ReplacementText(&qtssReplacement)); + if (!qtsb) + CheckHr(ptssSource->GetBldr(&qtsb)); + CheckHr(qtsb->ReplaceTsString(ichMin + cchDelta, ichLim + cchDelta, + qtssReplacement)); + int cchReplacement; + CheckHr(qtssReplacement->get_Length(&cchReplacement)); + cchDelta += cchReplacement - (ichLim - ichMin); + (*pcMatches)++; + ichStartSearch = ichLim; + } + ITsStringPtr qtssResult; + if (qtsb) + CheckHr(qtsb->GetString(&qtssResult)); + else + qtssResult = ptssSource; + return qtssResult; + } + + void testReplaceAllInMatchesRepeatedFindIn() + { + ITsStringPtr qtssSource; + ITsStringPtr qtssPattern; + ITsStringPtr qtssReplacement; + CheckHr(m_qtsf->MakeStringRgch(L"and and", 7, g_wsEng, &qtssSource)); + CheckHr(m_qtsf->MakeStringRgch(L"and", 3, g_wsEng, &qtssPattern)); + CheckHr(m_qtsf->MakeStringRgch(L"or", 2, g_wsEng, &qtssReplacement)); + CheckHr(m_qpat->putref_Pattern(qtssPattern)); + CheckHr(m_qpat->putref_ReplaceWith(qtssReplacement)); + int cExpected; + ITsStringPtr qtssExpected = ReplaceWithRepeatedFindIn(qtssSource, 0, 7, + &cExpected); + IVwTxtSrcInitPtr qtsi; + qtsi.CreateInstance(CLSID_VwStringTextSource); + CheckHr(qtsi->SetString(qtssSource)); + IVwTextSourcePtr qts; + CheckHr(qtsi->QueryInterface(IID_IVwTextSource, (void **)&qts)); + int cMatches = -1; + ITsStringPtr qtssResult; + HRESULT hr = m_qpat2->ReplaceAllIn(qtssSource, 0, 7, &cMatches, &qtssResult); + unitpp::assert_eq("ReplaceAllIn succeeds", (int)S_OK, (int)hr); + CheckHr(hr); + unitpp::assert_eq("literal match count", 2, cMatches); + unitpp::assert_eq("literal differential match count", cExpected, cMatches); + ComBool fEqual; + CheckHr(qtssResult->Equals(qtssExpected, &fEqual)); + unitpp::assert_true("literal result equals repeated FindIn", fEqual); + SmartBstr sbstrResult; + CheckHr(qtssResult->get_Text(&sbstrResult)); + unitpp::assert_true("literal replacement text", sbstrResult == L"or or"); + + CheckHr(m_qtsf->MakeStringRgch(L"(o|e)(ld)", 10, g_wsEng, &qtssPattern)); + CheckHr(m_qtsf->MakeStringRgch(L"$2-$1", 5, g_wsEng, &qtssReplacement)); + CheckHr(m_qtsf->MakeStringRgch(L"old eld", 7, g_wsEng, &qtssSource)); + ITsStrBldrPtr qtsbSource; + CheckHr(qtssSource->GetBldr(&qtsbSource)); + StrUni stuSourceStyle(L"source-style"); + CheckHr(qtsbSource->SetStrPropValue(4, 7, ktptNamedStyle, stuSourceStyle.Bstr())); + CheckHr(qtsbSource->GetString(&qtssSource)); + CheckHr(m_qpat->putref_Pattern(qtssPattern)); + CheckHr(m_qpat->putref_ReplaceWith(qtssReplacement)); + CheckHr(m_qpat->put_UseRegularExpressions(true)); + qtssExpected = ReplaceWithRepeatedFindIn(qtssSource, 0, 7, &cExpected); + qtsi.CreateInstance(CLSID_VwStringTextSource); + CheckHr(qtsi->SetString(qtssSource)); + CheckHr(qtsi->QueryInterface(IID_IVwTextSource, (void **)&qts)); + cMatches = -1; + qtssResult.Clear(); + CheckHr(m_qpat2->ReplaceAllIn(qtssSource, 0, 7, &cMatches, &qtssResult)); + unitpp::assert_eq("regex capture match count", 2, cMatches); + unitpp::assert_eq("regex differential match count", cExpected, cMatches); + CheckHr(qtssResult->Equals(qtssExpected, &fEqual)); + unitpp::assert_true("regex rich result equals repeated FindIn", fEqual); + CheckHr(qtssResult->get_Text(&sbstrResult)); + unitpp::assert_true("regex captures are materialized per match", + sbstrResult == L"ld-o ld-e"); + + CheckHr(m_qtsf->MakeStringRgch(L"(?=.)", 5, g_wsEng, &qtssPattern)); + CheckHr(m_qtsf->MakeStringRgch(L"x", 1, g_wsEng, &qtssReplacement)); + CheckHr(m_qtsf->MakeStringRgch(L"\xD83D\xDE00" L"a", 3, g_wsEng, &qtssSource)); + CheckHr(m_qpat->putref_Pattern(qtssPattern)); + CheckHr(m_qpat->putref_ReplaceWith(qtssReplacement)); + qtssExpected = ReplaceWithRepeatedFindIn(qtssSource, 0, 3, &cExpected); + qtsi.CreateInstance(CLSID_VwStringTextSource); + CheckHr(qtsi->SetString(qtssSource)); + CheckHr(qtsi->QueryInterface(IID_IVwTextSource, (void **)&qts)); + cMatches = -1; + qtssResult.Clear(); + CheckHr(m_qpat2->ReplaceAllIn(qtssSource, 0, 3, &cMatches, &qtssResult)); + unitpp::assert_eq("UTF-16 zero-width match count", 3, cMatches); + unitpp::assert_eq("zero-width differential match count", cExpected, cMatches); + CheckHr(qtssResult->Equals(qtssExpected, &fEqual)); + unitpp::assert_true("zero-width result equals repeated FindIn", fEqual); + CheckHr(qtssResult->get_Text(&sbstrResult)); + unitpp::assert_true("zero-width matches preserve one-code-unit advancement", + sbstrResult == L"xxx"); + + CheckHr(m_qtsf->MakeStringRgch(L"not present", 11, g_wsEng, &qtssPattern)); + CheckHr(m_qpat->putref_Pattern(qtssPattern)); + CheckHr(m_qpat->put_UseRegularExpressions(false)); + qtssExpected = ReplaceWithRepeatedFindIn(qtssSource, 0, 3, &cExpected); + qtsi.CreateInstance(CLSID_VwStringTextSource); + CheckHr(qtsi->SetString(qtssSource)); + CheckHr(qtsi->QueryInterface(IID_IVwTextSource, (void **)&qts)); + cMatches = -1; + qtssResult.Clear(); + CheckHr(m_qpat2->ReplaceAllIn(qtssSource, 0, 3, &cMatches, &qtssResult)); + unitpp::assert_eq("no-match count", 0, cMatches); + unitpp::assert_eq("no-match differential count", cExpected, cMatches); + CheckHr(qtssResult->Equals(qtssExpected, &fEqual)); + unitpp::assert_true("no-match result equals repeated FindIn", fEqual); + CheckHr(qtssResult->Equals(qtssSource, &fEqual)); + unitpp::assert_true("no-match result preserves source content and properties", fEqual); + + cMatches = 99; + ITsString * ptssRawResult = reinterpret_cast(1); + hr = m_qpat2->ReplaceAllIn(NULL, 0, 3, &cMatches, &ptssRawResult); + unitpp::assert_eq("null source is rejected", (int)E_POINTER, (int)hr); + unitpp::assert_eq("count is cleared before null-source failure", 0, cMatches); + unitpp::assert_true("result is cleared before null-source failure", + ptssRawResult == NULL); + SetErrorInfo(0, NULL); + } + + void testReplaceAllInValidatesOutputsAndRanges() + { + ITsStringPtr qtssSource; + ITsStringPtr qtssPattern; + ITsStringPtr qtssReplacement; + CheckHr(m_qtsf->MakeStringRgch(L"and", 3, g_wsEng, &qtssSource)); + CheckHr(m_qtsf->MakeStringRgch(L"and", 3, g_wsEng, &qtssPattern)); + CheckHr(m_qtsf->MakeStringRgch(L"or", 2, g_wsEng, &qtssReplacement)); + CheckHr(m_qpat->putref_Pattern(qtssPattern)); + CheckHr(m_qpat->putref_ReplaceWith(qtssReplacement)); + ITsString * ptssRawResult = reinterpret_cast(1); + HRESULT hr = m_qpat2->ReplaceAllIn(qtssSource, 0, 3, NULL, &ptssRawResult); + unitpp::assert_eq("null match count is rejected", (int)E_POINTER, (int)hr); + unitpp::assert_true("result is untouched when first output is null", + ptssRawResult == reinterpret_cast(1)); + SetErrorInfo(0, NULL); + + int cMatches = 99; + hr = m_qpat2->ReplaceAllIn(qtssSource, 0, 3, &cMatches, NULL); + unitpp::assert_eq("null result is rejected", (int)E_POINTER, (int)hr); + unitpp::assert_eq("count is cleared before null-result failure", 0, cMatches); + SetErrorInfo(0, NULL); + + cMatches = 99; + ptssRawResult = reinterpret_cast(1); + hr = m_qpat2->ReplaceAllIn(qtssSource, -1, 3, &cMatches, &ptssRawResult); + unitpp::assert_eq("negative range is rejected", (int)E_INVALIDARG, (int)hr); + unitpp::assert_eq("count is cleared before negative-range failure", 0, + cMatches); + unitpp::assert_true("result is cleared before negative-range failure", + ptssRawResult == NULL); + SetErrorInfo(0, NULL); + + cMatches = 99; + ptssRawResult = reinterpret_cast(1); + hr = m_qpat2->ReplaceAllIn(qtssSource, 2, 1, &cMatches, &ptssRawResult); + unitpp::assert_eq("reversed range is rejected", (int)E_INVALIDARG, (int)hr); + unitpp::assert_eq("count is cleared before reversed-range failure", 0, + cMatches); + unitpp::assert_true("result is cleared before reversed-range failure", + ptssRawResult == NULL); + SetErrorInfo(0, NULL); + + cMatches = 99; + ptssRawResult = reinterpret_cast(1); + hr = m_qpat2->ReplaceAllIn(qtssSource, 0, 4, &cMatches, &ptssRawResult); + unitpp::assert_eq("range past source is rejected", (int)E_INVALIDARG, + (int)hr); + unitpp::assert_eq("count is cleared before past-source failure", 0, + cMatches); + unitpp::assert_true("result is cleared before past-source failure", + ptssRawResult == NULL); + SetErrorInfo(0, NULL); + } + + void testReplaceAllInClearsOutputsAfterReplacementFailure() + { + ITsStringPtr qtssSource; + ITsStringPtr qtssPattern; + ITsStringPtr qtssReplacement; + CheckHr(m_qtsf->MakeStringRgch(L"and and", 7, g_wsEng, &qtssSource)); + CheckHr(m_qtsf->MakeStringRgch(L"(and)", 5, g_wsEng, &qtssPattern)); + CheckHr(m_qtsf->MakeStringRgch(L"$1", 2, g_wsEng, &qtssReplacement)); + ITsStringPtr qtssFailingReplacement; + qtssFailingReplacement.Attach(NewObj FailSecondLockTsString(qtssReplacement)); + CheckHr(m_qpat->putref_Pattern(qtssPattern)); + CheckHr(m_qpat->putref_ReplaceWith(qtssFailingReplacement)); + CheckHr(m_qpat->put_UseRegularExpressions(true)); + + int cMatches = 99; + ITsString * ptssResult = reinterpret_cast(1); + HRESULT hr = m_qpat2->ReplaceAllIn(qtssSource, 0, 7, &cMatches, &ptssResult); + SetErrorInfo(0, NULL); + unitpp::assert_eq("replacement failure is returned", (int)E_FAIL, (int)hr); + unitpp::assert_eq("count remains zero after replacement failure", 0, cMatches); + unitpp::assert_true("result remains null after replacement failure", + ptssResult == NULL); + } virtual void Setup() { CreateTestWritingSystemFactory(); @@ -1631,6 +1943,7 @@ namespace TestViews // Use this rather than CreateInstance, because for the ORC test the pattern // and text source need to be in the same compilation unit. m_qpat.Attach(NewObj VwPattern()); + CheckHr(m_qpat->QueryInterface(IID_IVwPattern2, (void **)&m_qpat2)); m_qts.Attach(NewObj VwSimpleTxtSrc()); m_qzvps.Attach(NewObj VwPropertyStore()); m_qts->SetWritingSystemFactory(g_qwsf); @@ -1643,6 +1956,7 @@ namespace TestViews virtual void Teardown() { m_qts.Clear(); + m_qpat2.Clear(); m_qpat.Clear(); m_qwsEng.Clear(); m_qtsf.Clear(); diff --git a/Src/views/Views.idh b/Src/views/Views.idh index 867ae01776..d45d805e66 100644 --- a/Src/views/Views.idh +++ b/Src/views/Views.idh @@ -4252,14 +4252,27 @@ Last reviewed: [propget] HRESULT Group( [in] int iGroup, [out, retval] ITsString ** pptssGroup); - //:> We may want to add a method for returning the text of the paragraph, //:> to help display concordances. }; + // Provides synchronous bulk replacement for rich strings. + DeclareInterface(VwPattern2, VwPattern, FC825E0A-CEFB-461F-AFFB-E5E0E063CB74) + { + // Replaces non-overlapping forward matches in the UTF-16 range. + // No matches returns the source. + // Success leaves the pattern in a terminal no-match state. + HRESULT ReplaceAllIn( + [in] ITsString * ptss, + [in] int ichStart, + [in] int ichEnd, + [out] int * pcMatches, + [out, retval] ITsString ** pptssResult); + }; #ifndef NO_COCLASSES DeclareCoClass(VwPattern, 6C659C76-3991-48dd-93F7-DA65847D4863) { interface IVwPattern; + interface IVwPattern2; }; #endif // !NO_COCLASSES diff --git a/Src/views/VwPattern.cpp b/Src/views/VwPattern.cpp index f0a0958e23..5278c42976 100644 --- a/Src/views/VwPattern.cpp +++ b/Src/views/VwPattern.cpp @@ -25,6 +25,12 @@ DEFINE_THIS_FILE //:> VwPattern Methods //:>******************************************************************************************** +static int CheckedPatternPosition(__int64 value) +{ + if (value < 0 || value > INT_MAX) + ThrowHr(WarnHr(HRESULT_FROM_WIN32(ERROR_ARITHMETIC_OVERFLOW))); + return static_cast(value); +} /*---------------------------------------------------------------------------------------------- Constructor. @@ -38,7 +44,6 @@ VwPattern::VwPattern() m_fMatchDiacritics = true; } - /*---------------------------------------------------------------------------------------------- Destructor. ----------------------------------------------------------------------------------------------*/ @@ -62,9 +67,12 @@ STDMETHODIMP VwPattern::QueryInterface(REFIID riid, void **ppv) *ppv = static_cast(this); else if (riid == IID_IVwPattern) *ppv = static_cast(this); + else if (riid == IID_IVwPattern2) + *ppv = static_cast(this); else if (riid == IID_ISupportErrorInfo) { - *ppv = NewObj CSupportErrorInfo(this, IID_IVwPattern); + *ppv = NewObj CSupportErrorInfo2(static_cast(this), + IID_IVwPattern, IID_IVwPattern2); return S_OK; } else @@ -566,7 +574,6 @@ STDMETHODIMP VwPattern::get_Group(int iGroup, ITsString ** pptssGroup) END_COM_METHOD(g_fact, IID_IVwPattern); } - /*---------------------------------------------------------------------------------------------- Set whether to treat character sequences that are equivalent as defined by Unicode compatibility decompositions as being identical. @@ -915,7 +922,10 @@ class FindInAlgorithmBase int m_ichMinSearch; // Range of text to search (from smallest to largest index) int m_ichLimSearch; OLECHAR * m_pchBuf; // Text contents of m_pts; + Vector m_vchBuf; UErrorCode m_error; + bool m_fSessionInitialized; + bool m_fPropertiesOnly; FindInAlgorithmBase(IVwTextSource * pts, int ichStartLog, int ichEndLog, ComBool fForward, IVwSearchKiller * pxserkl, VwPattern * pat) @@ -929,6 +939,8 @@ class FindInAlgorithmBase m_ichLimFoundSearch = -1; m_pat = pat; m_error = U_ZERO_ERROR; + m_fSessionInitialized = false; + m_fPropertiesOnly = false; } virtual ~FindInAlgorithmBase() @@ -1108,9 +1120,9 @@ class FindInAlgorithmBase virtual void InitSearcher() = 0; // Run the main search loop over a single text source. virtual bool Search() = 0; + virtual bool SearchNext() = 0; - // Run the main body of the algorithm. Return true if a match is made successfully. - bool Run() + bool Prepare() { // Get the characters (paragraph) we have to search. // We are getting the whole paragraph to give ICU enough context for whole word matching. @@ -1131,16 +1143,64 @@ class FindInAlgorithmBase m_pat->Compile(); if ((!m_pat->m_fUseRegularExpressions) && m_pat->m_stuCompiled.Length() == 0) + { + m_fPropertiesOnly = true; + return true; + } + return true; + } + + bool InitializeSession() + { + if (!Prepare()) + return false; + if (m_fPropertiesOnly) + { + m_fSessionInitialized = true; + return true; + } + + m_vchBuf.Resize(CheckedPatternPosition(static_cast<__int64>(m_cchSrcSearch) + 1)); + InitializeSearcher(m_vchBuf.Begin()); + m_fSessionInitialized = true; + return true; + } + + bool Run() + { + if (!Prepare()) + return false; + if (m_fPropertiesOnly) return SearchForProperties(); - m_pchBuf = (OLECHAR *)(_alloca((m_cchSrcSearch + 1) * isizeof(OLECHAR))); - CheckHr(m_pts->FetchSearch(0, m_cchSrcSearch, m_pchBuf)); - * (m_pchBuf + m_cchSrcSearch) = 0; // null termination required. - m_ichLimSearch = std::min(m_cchSrcSearch, m_ichLimSearch); // Because of disregarded ORCs, we might get fewer charcaters in the buffer than we asked for. + OLECHAR * pchBuf = + (OLECHAR *)(_alloca((m_cchSrcSearch + 1) * isizeof(OLECHAR))); + InitializeSearcher(pchBuf); + return Search(); + } + + void InitializeSearcher(OLECHAR * pchBuf) + { + m_pchBuf = pchBuf; + CheckHr(m_pts->FetchSearch(0, m_cchSrcSearch, m_pchBuf)); + m_pchBuf[m_cchSrcSearch] = 0; + m_ichLimSearch = std::min(m_cchSrcSearch, m_ichLimSearch); InitSearcher(); + } - return Search(); + bool NextAcceptedMatch(int ichStartLog) + { + Assert(m_fSessionInitialized); + Assert(m_fForward); + CheckHr(m_pts->LogToSearch(ichStartLog, &m_ichMinSearch)); + if (m_ichMinSearch > m_ichLimSearch) + return Fail(); + m_ichMinFoundSearch = -1; + m_ichLimFoundSearch = -1; + if (m_fPropertiesOnly) + return SearchForProperties(); + return SearchNext(); } // Checks to see if the next char in the searched string is a Diacritic. If it is a diacritic @@ -1154,13 +1214,13 @@ class FindInAlgorithmBase return true; OLECHAR rgchw[2]; - CheckHr(m_pts->FetchSearch(m_ichLimFoundSearch, m_ichLimFoundSearch + 1, &rgchw[0])); + rgchw[0] = m_pchBuf[m_ichLimFoundSearch]; uint ch32; // if chw is the first char of a surrogate pair, and , fetch the next char as well and translate the pair into a UChar32 // otherwise, copy chw to a UChar32. if (U_IS_LEAD(rgchw[0]) && m_ichLimFoundSearch + 1 < m_ichLimSearch) { - CheckHr(m_pts->FetchSearch(m_ichLimFoundSearch + 1, m_ichLimFoundSearch + 2, &rgchw[1])); + rgchw[1] = m_pchBuf[m_ichLimFoundSearch + 1]; Assert(U_IS_TRAIL(rgchw[1])); bool fSurrogateOk = FromSurrogate(rgchw[0], rgchw[1], &ch32); Assert(fSurrogateOk); @@ -1305,6 +1365,7 @@ class FindInAlgorithm : public FindInAlgorithmBase { StringSearch * m_piter; BreakIterator * m_pbi; + bool m_fSearchStarted; public: FindInAlgorithm(IVwTextSource * pts, int ichStartLog, int ichEndLog, ComBool fForward, IVwSearchKiller * pxserkl, VwPattern * pat) @@ -1312,6 +1373,7 @@ class FindInAlgorithm : public FindInAlgorithmBase { m_piter = NULL; m_pbi = NULL; + m_fSearchStarted = false; } virtual ~FindInAlgorithm() @@ -1383,29 +1445,7 @@ class FindInAlgorithm : public FindInAlgorithmBase bool Search() { if (m_fForward) - { - for (m_ichMinFoundSearch = m_piter->first(m_error); - ; // termination checks are inside the loop body - m_ichMinFoundSearch = m_piter->next(m_error) ) - { - CheckError(); // see if first() or next() call failed. - if (m_ichMinFoundSearch == USEARCH_DONE) - return Fail(); - if (m_ichMinFoundSearch < m_ichMinSearch) - continue; - if (m_ichMinFoundSearch >= m_ichLimSearch) - continue; - m_ichLimFoundSearch = m_ichMinFoundSearch + m_piter->getMatchedLength(); - AdjustSearchLimitForDiacritics(); - if (m_ichLimFoundSearch > m_ichLimSearch) - return Fail(); // The first match extends past the end of our range. - if (m_pat->m_fMatchDiacritics && !CheckMatchDiacritic()) - continue; - // We have a candidate match. See if it satisfies ws, style, tag requirements if any - if (CheckAndExtendCandidate()) - return true; - } - } + return SearchNext(); else { for (m_ichMinFoundSearch = m_piter->last(m_error); @@ -1434,6 +1474,33 @@ class FindInAlgorithm : public FindInAlgorithmBase return false; // arbitrary (should never get here). } + bool SearchNext() + { + Assert(m_fForward); + for (m_ichMinFoundSearch = m_fSearchStarted ? m_piter->next(m_error) : + m_piter->first(m_error); + ; // termination checks are inside the loop body + m_ichMinFoundSearch = m_piter->next(m_error)) + { + m_fSearchStarted = true; + CheckError(); // see if first() or next() call failed. + if (m_ichMinFoundSearch == USEARCH_DONE) + return Fail(); + if (m_ichMinFoundSearch < m_ichMinSearch) + continue; + if (m_ichMinFoundSearch >= m_ichLimSearch) + continue; + m_ichLimFoundSearch = m_ichMinFoundSearch + m_piter->getMatchedLength(); + AdjustSearchLimitForDiacritics(); + if (m_ichLimFoundSearch > m_ichLimSearch) + return Fail(); // The first match extends past the end of our range. + if (m_pat->m_fMatchDiacritics && !CheckMatchDiacritic()) + continue; + if (CheckAndExtendCandidate()) + return true; + } + } + /*------------------------------------------------------------------------------------------ As of ICU 4.0 (or at least after ICU 3.6), the string search matches diacritics past the limit even when told not to match diacritics. @@ -1505,27 +1572,7 @@ class RegExFindInAlgorithm : public FindInAlgorithmBase { bool fMatch = true; // did we get a match this iteration? if (m_fForward) - { - for (fMatch = m_pmatcher->find(m_ichMinSearch, m_error); ; fMatch = m_pmatcher->find() ) - { - if (!fMatch) - return Fail(); // no more matches. - m_ichMinFoundSearch = m_pmatcher->start(m_error); - m_ichLimFoundSearch = m_pmatcher->end(m_error); - CheckError(); // see if find() or start() or end() call failed. - if (m_ichLimFoundSearch > m_ichLimSearch) - return Fail(); // The current match extends past the end of our range. - // Doesn't seem to be any reasonable way to do this - //if (m_pat->m_fMatchDiacritics) - // if(!CheckMatchDiacritic()) - // return Fail(); - // We have a candidate match. See if it satisfies ws, style, tag requirements if any - // I don't think we can usefully do this either. - //if (CheckAndExtendCandidate()) - // return true; - return true; // got a useful match. - } - } + return SearchNext(); else { // Simulate searching backwards by searching forwards and keeping the last match in range. @@ -1562,6 +1609,19 @@ class RegExFindInAlgorithm : public FindInAlgorithmBase } return false; // arbitrary (should never get here). } + + bool SearchNext() + { + Assert(m_fForward); + if (!m_pmatcher->find(m_ichMinSearch, m_error)) + return Fail(); + m_ichMinFoundSearch = m_pmatcher->start(m_error); + m_ichLimFoundSearch = m_pmatcher->end(m_error); + CheckError(); + if (m_ichLimFoundSearch > m_ichLimSearch) + return Fail(); + return true; + } }; /*---------------------------------------------------------------------------------------------- @@ -1646,6 +1706,122 @@ STDMETHODIMP VwPattern::FindIn(IVwTextSource * pts, int ichStartLog, int ichEndL END_COM_METHOD(g_fact, IID_IVwPattern); } +void VwPattern::ReplaceAllWithAlgorithm(FindInAlgorithmBase * pfia, IVwTextSource * pts, + ITsString * ptssSource, int ichStart, int ichEnd, int * pcMatches, + ITsString ** pptssResult) +{ + struct Replacement + { + int ichMin; + int ichLim; + ITsStringPtr qtss; + }; + Vector vrepl; + ITsStringPtr qtssSource = ptssSource; + ITsStrBldrPtr qtsb; + int ichStartSearch = ichStart; + int ichLimLastMatch = -1; + int cMatches = 0; + + if (pfia->InitializeSession()) + { + while (ichStartSearch <= ichEnd && pfia->NextAcceptedMatch(ichStartSearch)) + { + int ichMin; + int ichLim; + CheckHr(pts->SearchToLog(pfia->m_ichMinFoundSearch, false, &ichMin)); + CheckHr(pts->SearchToLog(pfia->m_ichLimFoundSearch, true, &ichLim)); + if (ichMin == ichLim) + { + if (ichMin > 0 || !m_fUseRegularExpressions || !m_stuCompiled.Equals(L"^")) + { + ichLim = CheckedPatternPosition(static_cast<__int64>(ichLim) + 1); + if (ichLim > ichEnd) + ichLim = ichEnd; + } + } + + m_ichMinFoundLog = ichMin; + m_ichLimFoundLog = ichLim; + m_qtsWhereFound = pts; + if (ichLim == ichLimLastMatch) + { + ichStartSearch = CheckedPatternPosition(static_cast<__int64>(ichLim) + 1); + continue; + } + ichLimLastMatch = ichLim; + + ITsStringPtr qtssReplacement; + CheckHr(get_ReplacementText(&qtssReplacement)); + if (!qtsb) + CheckHr(qtssSource->GetBldr(&qtsb)); + int ichMinResult; + int ichLimResult; + CheckHr(pts->LogToRen(ichMin, &ichMinResult)); + CheckHr(pts->LogToRen(ichLim, &ichLimResult)); + Replacement repl = { ichMinResult, ichLimResult, qtssReplacement }; + vrepl.Push(repl); + cMatches++; + ichStartSearch = ichLim; + } + } + for (int irepl = vrepl.Size() - 1; irepl >= 0; --irepl) + { + CheckHr(qtsb->ReplaceTsString(vrepl[irepl].ichMin, vrepl[irepl].ichLim, + vrepl[irepl].qtss)); + } + + ITsStringPtr qtssResult; + if (qtsb) + CheckHr(qtsb->GetString(&qtssResult)); + else + qtssResult = qtssSource; + m_ichMinFoundLog = -1; + m_ichLimFoundLog = -1; + m_qtsWhereFound = pts; + *pptssResult = qtssResult.Detach(); + *pcMatches = cMatches; +} + +STDMETHODIMP VwPattern::ReplaceAllIn(ITsString * ptss, int ichStart, int ichEnd, + int * pcMatches, ITsString ** pptssResult) +{ + BEGIN_COM_METHOD; + ChkComOutPtr(pcMatches); + *pcMatches = 0; + ChkComOutPtr(pptssResult); + *pptssResult = NULL; + ChkComArgPtr(ptss); + if (ichStart < 0 || ichEnd < ichStart) + ThrowHr(WarnHr(E_INVALIDARG)); + int cch; + CheckHr(ptss->get_Length(&cch)); + if (ichEnd > cch) + ThrowHr(WarnHr(E_INVALIDARG)); + IVwTxtSrcInitPtr qtsi; + qtsi.CreateInstance(CLSID_VwStringTextSource); + CheckHr(qtsi->SetString(ptss)); + IVwTextSourcePtr qts; + CheckHr(qtsi->QueryInterface(IID_IVwTextSource, (void **)&qts)); + int cMatches = 0; + ITsStringPtr qtssResult; + if (m_fUseRegularExpressions) + { + RegExFindInAlgorithm refia(qts, ichStart, ichEnd, true, NULL, this); + ReplaceAllWithAlgorithm(&refia, qts, ptss, ichStart, ichEnd, &cMatches, + &qtssResult); + } + else + { + FindInAlgorithm fia(qts, ichStart, ichEnd, true, NULL, this); + ReplaceAllWithAlgorithm(&fia, qts, ptss, ichStart, ichEnd, &cMatches, + &qtssResult); + } + *pptssResult = qtssResult.Detach(); + *pcMatches = cMatches; + END_COM_METHOD(g_fact, IID_IVwPattern2); +} + /*---------------------------------------------------------------------------------------------- Install the current Find result as the active selection. ----------------------------------------------------------------------------------------------*/ diff --git a/Src/views/VwPattern.h b/Src/views/VwPattern.h index f4db25965f..032f35f532 100644 --- a/Src/views/VwPattern.h +++ b/Src/views/VwPattern.h @@ -21,7 +21,7 @@ Last reviewed: Not yet. This class implements a search pattern and the top level mechanisms to do the actual searching. @h3{Hungarian: zpat} ----------------------------------------------------------------------------------------------*/ -class VwPattern : public IVwPattern +class VwPattern : public IVwPattern2 { friend class VwLazyBox; // Can set relevant instance vars on successful find. friend class FindInAlgorithm; // used in method implementation. @@ -105,6 +105,8 @@ class VwPattern : public IVwPattern STDMETHOD(get_ErrorMessage)(BSTR * pbstrMsg); STDMETHOD(get_ReplacementText)(ITsString ** pptssText); STDMETHOD(get_Group)(int iGroup, ITsString ** pptssGroup); + STDMETHOD(ReplaceAllIn)(ITsString * ptss, int ichStart, int ichEnd, + int * pcMatches, ITsString ** pptssResult); // Other public methods @@ -180,6 +182,9 @@ class VwPattern : public IVwPattern // Other protected methods void Compile(); void CleanupRegexPattern(); + void ReplaceAllWithAlgorithm(FindInAlgorithmBase * pfia, IVwTextSource * pts, + ITsString * ptssSource, + int ichStart, int ichEnd, int * pcMatches, ITsString ** pptssResult); }; diff --git a/Src/xWorks/xWorksTests/Avalonia/Performance/ReplaceWithMethodTests.cs b/Src/xWorks/xWorksTests/Avalonia/Performance/ReplaceWithMethodTests.cs new file mode 100644 index 0000000000..8765dd73ad --- /dev/null +++ b/Src/xWorks/xWorksTests/Avalonia/Performance/ReplaceWithMethodTests.cs @@ -0,0 +1,489 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.InteropServices; +using System.Text; +using System.Threading; +using System.Xml; +using NUnit.Framework; +using SIL.FieldWorks.Common.Controls; +using SIL.FieldWorks.Common.ViewsInterfaces; +using SIL.LCModel; +using SIL.LCModel.Application; +using SIL.LCModel.Core.KernelInterfaces; +using SIL.LCModel.Core.Text; +using SIL.LCModel.Infrastructure; + +namespace SIL.FieldWorks.XWorks.Performance +{ + [TestFixture] + [Category("BulkReplacement")] + [Apartment(ApartmentState.STA)] + public class ReplaceWithMethodPreviewTests : BulkEditBarTestsBase + { + [TestCase(0)] + [TestCase(1)] + [TestCase(2)] + [TestCase(4)] + public void FakeDoit_UsesOneReplacementPassAndProducesTheImmediateApplyResult(int matchingRows) + { + var entries = CreateEntries(4, matchingRows, "old", "keep"); + var document = BuildColumnSpec(); + var accessor = FieldReadWriter.Create(document.DocumentElement, Cache); + var pattern = BuildPattern("old", "new"); + var method = new CountingReplaceWithMethod(Cache, m_bv.SpecialCache, accessor, + document.DocumentElement, pattern, pattern.ReplaceWith); + var sentinels = entries.ToDictionary(entry => entry.Hvo, + entry => TsStringUtils.MakeString("previous preview " + entry.Hvo, Cache.DefaultVernWs)); + + foreach (var entry in entries) + m_bv.SpecialCache.SetString(entry.Hvo, XMLViewsDataCache.ktagAlternateValue, sentinels[entry.Hvo]); + + method.FakeDoit(entries.Select(entry => entry.Hvo), XMLViewsDataCache.ktagAlternateValue, + XMLViewsDataCache.ktagItemEnabled, new NullProgressState()); + + Assert.That(method.FindInCount, Is.Zero, + "the bulk replacement capability must not restart the public search for each match"); + Assert.That(method.ReplaceAllInCount, Is.EqualTo(entries.Count), + "each eligible value must use exactly one bulk replacement call"); + + Cache.DomainDataByFlid.BeginUndoTask("preview apply parity", "preview apply parity"); + try + { + try + { + foreach (var entry in entries) + method.Doit(entry.Hvo); + } + finally + { + Cache.DomainDataByFlid.EndUndoTask(); + } + + foreach (var entry in entries) + { + var matched = entries.IndexOf(entry) < matchingRows; + var enabled = m_bv.SpecialCache.get_IntProp(entry.Hvo, XMLViewsDataCache.ktagItemEnabled); + var preview = m_bv.SpecialCache.get_StringProp(entry.Hvo, XMLViewsDataCache.ktagAlternateValue); + Assert.That(enabled, Is.EqualTo(matched ? 1 : 0)); + Assert.That(accessor.CurrentValue(entry.Hvo).Text, Is.EqualTo(matched ? "new" : "keep")); + if (!matched) + Assert.That(preview, Is.SameAs(sentinels[entry.Hvo])); + else + Assert.That(preview.Text, Is.EqualTo(accessor.CurrentValue(entry.Hvo).Text)); + } + Assert.That(method.ReplaceAllInCount, Is.EqualTo(entries.Count * 2)); + Assert.That(method.FindInCount, Is.Zero); + } + finally + { + if (matchingRows > 0) + Cache.ActionHandlerAccessor.Undo(); + } + } + + [TestCase("old-old", "old", "new", false, false, false, false, "new-new")] + [TestCase("ab-12 cd-34", "([a-z]+)-([0-9]+)", "$2:$1", true, false, false, false, "12:ab 34:cd")] + [TestCase("cafeteria cafe", "cafe", "tea", false, true, true, true, "cafeteria tea")] + [TestCase("CAF\u00C9 cafe", "cafe", "tea", false, false, false, false, "tea tea")] + [TestCase("cafe\u0301", "caf\u00e9", "tea", false, false, true, true, "tea")] + [TestCase("old", "^", "new", true, false, true, true, "newold")] + public void FakeDoit_MatchesImmediateApplyAcrossPatternModes(string source, string find, + string replace, bool regularExpression, bool wholeWord, bool matchCase, bool matchDiacritics, + string expected) + { + var entry = CreateEntries(1, 1, source, source).Single(); + var document = BuildColumnSpec(); + var accessor = FieldReadWriter.Create(document.DocumentElement, Cache); + var pattern = BuildPattern(find, replace, regularExpression, wholeWord, matchCase, matchDiacritics); + var method = new CountingReplaceWithMethod(Cache, m_bv.SpecialCache, accessor, + document.DocumentElement, pattern, pattern.ReplaceWith); + + method.FakeDoit(new[] { entry.Hvo }, XMLViewsDataCache.ktagAlternateValue, + XMLViewsDataCache.ktagItemEnabled, new NullProgressState()); + + Assert.That(method.ReplaceAllInCount, Is.EqualTo(1)); + Assert.That(method.FindInCount, Is.Zero); + Assert.That(m_bv.SpecialCache.get_IntProp(entry.Hvo, XMLViewsDataCache.ktagItemEnabled), Is.EqualTo(1)); + Assert.That(m_bv.SpecialCache.get_StringProp(entry.Hvo, XMLViewsDataCache.ktagAlternateValue).Text, + Is.EqualTo(expected)); + + Cache.DomainDataByFlid.BeginUndoTask("preview apply parity", "preview apply parity"); + try + { + try + { + method.Doit(entry.Hvo); + } + finally + { + Cache.DomainDataByFlid.EndUndoTask(); + } + + Assert.That(accessor.CurrentValue(entry.Hvo).Text, Is.EqualTo(expected)); + Assert.That(method.ReplaceAllInCount, Is.EqualTo(2)); + Assert.That(method.FindInCount, Is.Zero); + } + finally + { + Cache.ActionHandlerAccessor.Undo(); + } + } + + [Test] + public void FakeDoit_MatchesImmediateApplyForMultiStringRichRuns() + { + ILexEntry entry = null; + ILexSense sense = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + entry = Cache.ServiceLocator.GetInstance().Create(); + sense = Cache.ServiceLocator.GetInstance().Create(entry, null, string.Empty); + m_createdObjectList.Add(entry); + }); + var document = BuildColumnSpec(); + var accessor = new OwnMlPropReadWriter(Cache, LexSenseTags.kflidDefinition, Cache.DefaultAnalWs); + var sourceBuilder = TsStringUtils.MakeString("old keep", Cache.DefaultAnalWs).GetBldr(); + sourceBuilder.SetIntPropValues(4, 8, (int)FwTextPropType.ktptWs, + (int)FwTextPropVar.ktpvDefault, Cache.DefaultVernWs); + sourceBuilder.SetStrPropValue(4, 8, (int)FwTextPropType.ktptNamedStyle, "Emphasis"); + sourceBuilder.SetStrPropValue(4, 8, (int)FwTextPropType.ktptObjData, + (char)FwObjDataTypes.kodtExternalPathName + "https://example.test/keep"); + var source = sourceBuilder.GetString(); + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, + () => accessor.SetNewValue(sense.Hvo, source)); + Assert.That(accessor.CurrentValue(sense.Hvo).RunCount, Is.EqualTo(2)); + + var pattern = BuildPattern("old", "new"); + var method = new CountingReplaceWithMethod(Cache, m_bv.SpecialCache, accessor, + document.DocumentElement, pattern, pattern.ReplaceWith); + method.FakeDoit(new[] { sense.Hvo }, XMLViewsDataCache.ktagAlternateValue, + XMLViewsDataCache.ktagItemEnabled, new NullProgressState()); + var preview = m_bv.SpecialCache.get_StringProp(sense.Hvo, XMLViewsDataCache.ktagAlternateValue); + + Assert.That(method.ReplaceAllInCount, Is.EqualTo(1)); + Assert.That(method.FindInCount, Is.Zero); + Assert.That(preview.Text, Is.EqualTo("new keep")); + var keepRun = Enumerable.Range(0, preview.RunCount).Single(i => preview.get_RunText(i) == "keep"); + Assert.That(TsStringUtils.GetWsOfRun(preview, keepRun), Is.EqualTo(Cache.DefaultVernWs)); + Assert.That(preview.get_Properties(keepRun).GetStrPropValue((int)FwTextPropType.ktptNamedStyle), + Is.EqualTo("Emphasis")); + Assert.That(preview.get_Properties(keepRun).GetStrPropValue((int)FwTextPropType.ktptObjData), + Is.EqualTo((char)FwObjDataTypes.kodtExternalPathName + "https://example.test/keep")); + + Cache.DomainDataByFlid.BeginUndoTask("preview apply parity", "preview apply parity"); + try + { + try + { + method.Doit(sense.Hvo); + } + finally + { + Cache.DomainDataByFlid.EndUndoTask(); + } + + Assert.That(preview.Equals(accessor.CurrentValue(sense.Hvo)), Is.True); + Assert.That(method.ReplaceAllInCount, Is.EqualTo(2)); + Assert.That(method.FindInCount, Is.Zero); + } + finally + { + Cache.ActionHandlerAccessor.Undo(); + } + } + + [TestCase("old-old", true, "new-new", 3)] + [TestCase("keep", false, null, 1)] + public void FakeDoit_FallsBackWhenBulkReplacementIsUnavailable(string source, + bool matched, string expected, int expectedFindInCount) + { + var entry = CreateEntries(1, 1, source, source).Single(); + var document = BuildColumnSpec(); + var accessor = FieldReadWriter.Create(document.DocumentElement, Cache); + var pattern = BuildPattern("old", "new"); + var method = new CountingReplaceWithMethod(Cache, m_bv.SpecialCache, accessor, + document.DocumentElement, pattern, pattern.ReplaceWith, false); + var sentinel = TsStringUtils.MakeString("previous preview", Cache.DefaultVernWs); + m_bv.SpecialCache.SetString(entry.Hvo, XMLViewsDataCache.ktagAlternateValue, sentinel); + + method.FakeDoit(new[] { entry.Hvo }, XMLViewsDataCache.ktagAlternateValue, + XMLViewsDataCache.ktagItemEnabled, new NullProgressState()); + + Assert.That(method.FindInCount, Is.EqualTo(expectedFindInCount)); + Assert.That(m_bv.SpecialCache.get_IntProp(entry.Hvo, XMLViewsDataCache.ktagItemEnabled), + Is.EqualTo(matched ? 1 : 0)); + var preview = m_bv.SpecialCache.get_StringProp(entry.Hvo, + XMLViewsDataCache.ktagAlternateValue); + if (matched) + Assert.That(preview.Text, Is.EqualTo(expected)); + else + Assert.That(preview, Is.SameAs(sentinel)); + } + + [Test] + public void FakeDoit_PropagatesBulkReplacementFailureWithoutFallback() + { + var entry = CreateEntries(1, 1, "old", "old").Single(); + var document = BuildColumnSpec(); + var accessor = FieldReadWriter.Create(document.DocumentElement, Cache); + var pattern = BuildPattern("old", "new"); + var failure = new COMException("bulk replacement failed", unchecked((int)0x80004005)); + var method = new CountingReplaceWithMethod(Cache, m_bv.SpecialCache, accessor, + document.DocumentElement, pattern, pattern.ReplaceWith, true, failure); + + var actual = Assert.Throws(() => method.FakeDoit(new[] { entry.Hvo }, + XMLViewsDataCache.ktagAlternateValue, XMLViewsDataCache.ktagItemEnabled, + new NullProgressState())); + + Assert.That(actual, Is.SameAs(failure)); + Assert.That(method.ReplaceAllInCount, Is.EqualTo(1)); + Assert.That(method.FindInCount, Is.Zero); + } + + [Test] + public void FakeDoit_NormalizesBulkReplacementResultToNfd() + { + var entry = CreateEntries(1, 1, "old caf\u00e9", "old caf\u00e9").Single(); + var document = BuildColumnSpec(); + var accessor = FieldReadWriter.Create(document.DocumentElement, Cache); + var pattern = BuildPattern("old", "new"); + var method = new CountingReplaceWithMethod(Cache, m_bv.SpecialCache, accessor, + document.DocumentElement, pattern, pattern.ReplaceWith); + + method.FakeDoit(new[] { entry.Hvo }, XMLViewsDataCache.ktagAlternateValue, + XMLViewsDataCache.ktagItemEnabled, new NullProgressState()); + + var preview = m_bv.SpecialCache.get_StringProp(entry.Hvo, + XMLViewsDataCache.ktagAlternateValue); + Assert.That(method.ReplaceAllInCount, Is.EqualTo(1)); + Assert.That(method.FindInCount, Is.Zero); + Assert.That(preview.Text, Is.EqualTo("new cafe\u0301")); + Assert.That(preview.Text.IsNormalized(NormalizationForm.FormD), Is.True); + } + + [Test] + public void FakeDoit_BaseGateRejectionDoesNotSearchOrChangePreview() + { + var entry = CreateEntries(1, 1, "old", "old").Single(); + var document = BuildColumnSpec(); + var accessor = FieldReadWriter.Create(document.DocumentElement, Cache); + var pattern = BuildPattern("old", "new"); + var method = new CountingReplaceWithMethod(Cache, m_bv.SpecialCache, accessor, + document.DocumentElement, pattern, pattern.ReplaceWith, canChange: false); + var sentinel = TsStringUtils.MakeString("previous preview", Cache.DefaultVernWs); + m_bv.SpecialCache.SetString(entry.Hvo, XMLViewsDataCache.ktagAlternateValue, sentinel); + + method.FakeDoit(new[] { entry.Hvo }, XMLViewsDataCache.ktagAlternateValue, + XMLViewsDataCache.ktagItemEnabled, new NullProgressState()); + + Assert.That(method.ReplaceAllInCount, Is.Zero); + Assert.That(method.FindInCount, Is.Zero); + Assert.That(m_bv.SpecialCache.get_IntProp(entry.Hvo, + XMLViewsDataCache.ktagItemEnabled), Is.Zero); + Assert.That(m_bv.SpecialCache.get_StringProp(entry.Hvo, + XMLViewsDataCache.ktagAlternateValue), Is.SameAs(sentinel)); + Assert.That(accessor.CurrentValue(entry.Hvo).Text, Is.EqualTo("old")); + } + + private List CreateEntries(int count, int matchingRows, string matchedValue, + string unmatchedValue) + { + var entries = new List(count); + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + for (var index = 0; index < count; index++) + { + var entry = Cache.ServiceLocator.GetInstance().Create(); + entry.CitationForm.set_String(Cache.DefaultVernWs, + index < matchingRows ? matchedValue : unmatchedValue); + entries.Add(entry); + m_createdObjectList.Add(entry); + } + }); + return entries; + } + + private static XmlDocument BuildColumnSpec() + { + var document = new XmlDocument(); + document.LoadXml(""); + return document; + } + + private IVwPattern BuildPattern(string find, string replace, bool regularExpression = false, + bool wholeWord = false, bool matchCase = true, bool matchDiacritics = true) + { + var pattern = VwPatternClass.Create(); + pattern.Pattern = TsStringUtils.MakeString(find, Cache.DefaultVernWs); + pattern.ReplaceWith = TsStringUtils.MakeString(replace, Cache.DefaultVernWs); + pattern.UseRegularExpressions = regularExpression; + pattern.MatchWholeWord = wholeWord; + pattern.MatchCase = matchCase; + pattern.MatchDiacritics = matchDiacritics; + return pattern; + } + + internal sealed class CountingReplaceWithMethod : ReplaceWithMethod + { + private readonly bool m_exposeBulkReplacement; + private readonly Exception m_bulkReplacementFailure; + private readonly bool? m_canChange; + + internal CountingReplaceWithMethod(LcmCache cache, ISilDataAccessManaged sda, + FieldReadWriter accessor, XmlNode spec, IVwPattern pattern, ITsString replacement, + bool exposeBulkReplacement = true, Exception bulkReplacementFailure = null, + bool? canChange = null) + : base(cache, sda, accessor, spec, pattern, replacement) + { + m_exposeBulkReplacement = exposeBulkReplacement; + m_bulkReplacementFailure = bulkReplacementFailure; + m_canChange = canChange; + } + + internal int FindInCount { get; private set; } + internal int ReplaceAllInCount { get; private set; } + + protected override bool OkToChange(int hvo) + { + return m_canChange ?? base.OkToChange(hvo); + } + + protected override bool TryReplaceAllIn(ITsString source, out ITsString result, + out int matchCount) + { + ReplaceAllInCount++; + if (m_bulkReplacementFailure != null) + throw m_bulkReplacementFailure; + if (!m_exposeBulkReplacement) + { + result = null; + matchCount = 0; + return false; + } + return base.TryReplaceAllIn(source, out result, out matchCount); + } + + protected override void FindIn(int ichStart, int ichEnd, out int ichMin, out int ichLim) + { + FindInCount++; + base.FindIn(ichStart, ichEnd, out ichMin, out ichLim); + } + } + } + + [TestFixture] + [Category("BulkReplacement")] + [Apartment(ApartmentState.STA)] + public class ReplaceAllInDecoratorCorrectnessTests : BulkEditBarTestsBase + { + [Test] + public void FakeDoit_MatchesOracleForDecoratorBackedUnnormalizedRichValue() + { + ILexEntry entry = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + entry = Cache.ServiceLocator.GetInstance().Create(); + m_createdObjectList.Add(entry); + }); + var sourceBuilder = TsStringUtils.MakeString("old cafe\u0301 old keep", + Cache.DefaultVernWs).GetBldr(); + sourceBuilder.SetIntPropValues(4, 9, (int)FwTextPropType.ktptWs, + (int)FwTextPropVar.ktpvDefault, Cache.DefaultAnalWs); + sourceBuilder.SetStrPropValue(4, 9, (int)FwTextPropType.ktptNamedStyle, + "Unnormalized Source"); + sourceBuilder.SetStrPropValue(14, 18, (int)FwTextPropType.ktptObjData, + (char)FwObjDataTypes.kodtExternalPathName + "https://example.test/keep"); + m_bv.SpecialCache.SetString(entry.Hvo, XMLViewsDataCache.ktagAlternateValue, + sourceBuilder.GetString()); + var decoratedValue = m_bv.SpecialCache.get_StringProp(entry.Hvo, + XMLViewsDataCache.ktagAlternateValue); + Assert.That(decoratedValue.Text.IsNormalized(NormalizationForm.FormC), Is.False); + Assert.That(decoratedValue.RunCount, Is.GreaterThan(1)); + + var pattern = VwPatternClass.Create(); + pattern.Pattern = TsStringUtils.MakeString("old", Cache.DefaultVernWs); + pattern.ReplaceWith = TsStringUtils.MakeString("new", Cache.DefaultVernWs); + pattern.MatchCase = true; + pattern.MatchDiacritics = true; + var replacementBuilder = pattern.ReplaceWith.GetBldr(); + replacementBuilder.SetStrPropValue(0, 3, (int)FwTextPropType.ktptNamedStyle, + "Decorator Replacement"); + pattern.ReplaceWith = replacementBuilder.GetString(); + var oracleInit = VwStringTextSourceClass.Create(); + oracleInit.SetString(decoratedValue); + var oracleSource = (IVwTextSource)oracleInit; + var expectedBuilder = decoratedValue.GetBldr(); + var expectedCount = 0; + var ichStart = 0; + var delta = 0; + while (ichStart <= decoratedValue.Length) + { + pattern.FindIn(oracleSource, ichStart, decoratedValue.Length, true, + out var ichMin, out var ichLim, null); + if (ichMin < 0) + break; + var replacement = pattern.ReplacementText; + expectedBuilder.ReplaceTsString(ichMin + delta, ichLim + delta, replacement); + delta += replacement.Length - (ichLim - ichMin); + expectedCount++; + ichStart = ichLim; + } + + Assert.That(expectedCount, Is.EqualTo(2)); + var document = new XmlDocument(); + document.LoadXml(""); + var accessor = new DecoratorStringReadWriter(m_bv.SpecialCache, + XMLViewsDataCache.ktagAlternateValue, Cache.DefaultVernWs); + var method = new ReplaceWithMethodPreviewTests.CountingReplaceWithMethod(Cache, + m_bv.SpecialCache, accessor, document.DocumentElement, pattern, pattern.ReplaceWith); + + method.FakeDoit(new[] { entry.Hvo }, XMLViewsDataCache.ktagAlternateValue, + XMLViewsDataCache.ktagItemEnabled, new NullProgressState()); + + var actual = m_bv.SpecialCache.get_StringProp(entry.Hvo, + XMLViewsDataCache.ktagAlternateValue); + var expected = expectedBuilder.GetString().get_NormalizedForm(FwNormalizationMode.knmNFD); + Assert.That(method.ReplaceAllInCount, Is.EqualTo(1)); + Assert.That(method.FindInCount, Is.Zero); + Assert.That(m_bv.SpecialCache.get_IntProp(entry.Hvo, + XMLViewsDataCache.ktagItemEnabled), Is.EqualTo(1)); + Assert.That(actual.Text, Is.EqualTo("new cafe\u0301 new keep")); + Assert.That(actual.Text.IsNormalized(NormalizationForm.FormD), Is.True); + Assert.That(TsStringHelper.TsStringsAreEqual(expected, actual, + out var differences), Is.True, differences); + } + + private sealed class DecoratorStringReadWriter : FieldReadWriter + { + private readonly int m_tag; + private readonly int m_writingSystem; + + internal DecoratorStringReadWriter(ISilDataAccess dataAccess, int tag, int writingSystem) + : base(dataAccess) + { + m_tag = tag; + m_writingSystem = writingSystem; + } + + public override ITsString CurrentValue(int hvo) + { + return m_sda.get_StringProp(hvo, m_tag); + } + + public override void SetNewValue(int hvo, ITsString tss) + { + m_sda.SetString(hvo, m_tag, tss); + } + + public override int WritingSystem + { + get { return m_writingSystem; } + } + } + } +} diff --git a/Src/xWorks/xWorksTests/Avalonia/Performance/VwPatternReplacementTests.cs b/Src/xWorks/xWorksTests/Avalonia/Performance/VwPatternReplacementTests.cs new file mode 100644 index 0000000000..8cdf3ab56e --- /dev/null +++ b/Src/xWorks/xWorksTests/Avalonia/Performance/VwPatternReplacementTests.cs @@ -0,0 +1,490 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System.Collections.Generic; +using System.Runtime.InteropServices; +using NUnit.Framework; +using SIL.FieldWorks.Common.ViewsInterfaces; +using SIL.LCModel; +using SIL.LCModel.Core.KernelInterfaces; +using SIL.LCModel.Core.Text; + +namespace SIL.FieldWorks.XWorks.Performance +{ + [TestFixture] + [Category("BulkReplacement")] + public class VwPatternReplacementTests : MemoryOnlyBackendProviderTestBase + { + private int Ws => Cache.DefaultVernWs; + + private IVwPattern MakePattern(string patternText, bool matchCase = false, + bool matchDiacritics = false, bool matchWholeWord = false) + { + var pattern = VwPatternClass.Create(); + pattern.Pattern = TsStringUtils.MakeString(patternText, Ws); + pattern.MatchCase = matchCase; + pattern.MatchDiacritics = matchDiacritics; + pattern.MatchWholeWord = matchWholeWord; + return pattern; + } + + private IVwTextSource MakeTextSource(string text, out int length) + { + return MakeTextSource(TsStringUtils.MakeString(text, Ws), out length); + } + + private static IVwTextSource MakeTextSource(ITsString tss, out int length) + { + length = tss.Length; + var textSourceInit = VwStringTextSourceClass.Create(); + textSourceInit.SetString(tss); + return (IVwTextSource)textSourceInit; + } + + private static void Find(IVwPattern pattern, IVwTextSource ts, int ichStart, int ichEnd, + bool forward, out int ichMin, out int ichLim) + { + pattern.FindIn(ts, ichStart, ichEnd, forward, out ichMin, out ichLim, null); + } + + private static void AssertFound(int ichMin, int ichLim, int expectedMin, int expectedLim, string why) + { + Assert.That(ichMin, Is.EqualTo(expectedMin), why + " (ichMin)"); + Assert.That(ichLim, Is.EqualTo(expectedLim), why + " (ichLim)"); + } + + private static void AssertNotFound(int ichMin, string why) + { + Assert.That(ichMin, Is.LessThan(0), why); + } + + private sealed class ReplacementMatch + { + public ReplacementMatch(int ichMin, int ichLim, ITsString replacement) + { + IchMin = ichMin; + IchLim = ichLim; + Replacement = replacement; + } + + public int IchMin { get; } + public int IchLim { get; } + public ITsString Replacement { get; } + } + + /// Describes one expected replacement match. + public sealed class ExpectedReplacementMatch + { + internal ExpectedReplacementMatch(int ichMin, int ichLim, string replacementText) + { + IchMin = ichMin; + IchLim = ichLim; + ReplacementText = replacementText; + } + + internal int IchMin { get; } + internal int IchLim { get; } + internal string ReplacementText { get; } + } + + private static ExpectedReplacementMatch ExpectedMatch(int ichMin, int ichLim, string replacementText) + { + return new ExpectedReplacementMatch(ichMin, ichLim, replacementText); + } + + private static void AssertReplacementMatches(IList actual, + params ExpectedReplacementMatch[] expected) + { + Assert.That(actual, Has.Count.EqualTo(expected.Length)); + for (var index = 0; index < expected.Length; index++) + { + Assert.That(actual[index].IchMin, Is.EqualTo(expected[index].IchMin), "ichMin at match " + index); + Assert.That(actual[index].IchLim, Is.EqualTo(expected[index].IchLim), "ichLim at match " + index); + Assert.That(actual[index].Replacement.Text, Is.EqualTo(expected[index].ReplacementText), + "replacement text at match " + index); + } + } + + private static List CollectReplacementMatches(IVwPattern pattern, + IVwTextSource ts, int cch, int ichStartSearch = 0) + { + var results = new List(); + var ichLimLastMatch = -1; + while (ichStartSearch <= cch) + { + Find(pattern, ts, ichStartSearch, cch, true, out var ichMin, out var ichLim); + if (ichMin < 0) + break; + if (ichLim == ichLimLastMatch) + { + ichStartSearch = ichLim + 1; + continue; + } + ichLimLastMatch = ichLim; + results.Add(new ReplacementMatch(ichMin, ichLim, pattern.ReplacementText)); + ichStartSearch = ichLim; + } + return results; + } + + private static void AssertReplaceAllMatchesRepeatedFindIn(IVwPattern pattern, + IVwTextSource oracleSource, int ichStart, int ichEnd, int? expectedCount = null, + string expectedText = null) + { + var source = oracleSource.GetSubString(0, oracleSource.Length); + var matches = CollectReplacementMatches(pattern, oracleSource, ichEnd, ichStart); + var expectedBuilder = source.GetBldr(); + var delta = 0; + foreach (var match in matches) + { + expectedBuilder.ReplaceTsString(match.IchMin + delta, match.IchLim + delta, + match.Replacement); + delta += match.Replacement.Length - (match.IchLim - match.IchMin); + } + + var actual = ((IVwPattern2)pattern).ReplaceAllIn(source, ichStart, ichEnd, + out var actualCount); + Assert.That(actualCount, Is.EqualTo(matches.Count), "bulk match count"); + if (expectedCount.HasValue) + Assert.That(actualCount, Is.EqualTo(expectedCount.Value), "characterized match count"); + if (expectedText != null) + Assert.That(actual.Text, Is.EqualTo(expectedText), "characterized result text"); + Assert.That(TsStringHelper.TsStringsAreEqual(expectedBuilder.GetString(), actual, + out var differences), Is.True, differences); + } + + private static IEnumerable PlainReplacementCases() + { + yield return new TestCaseData("old", "none", "new", new ExpectedReplacementMatch[0]) + .SetName("ReplacementIteration_NoMatches_ReturnsEmptySequence"); + yield return new TestCaseData("old", "old", "new", new[] { ExpectedMatch(0, 3, "new") }) + .SetName("ReplacementIteration_OneMatch_ReturnsOneReplacement"); + yield return new TestCaseData("old", "oldold", "new", new[] { ExpectedMatch(0, 3, "new"), ExpectedMatch(3, 6, "new") }) + .SetName("ReplacementIteration_AdjacentMatches_ReturnsBothReplacements"); + yield return new TestCaseData("old", "old old old", "new", new[] + { + ExpectedMatch(0, 3, "new"), ExpectedMatch(4, 7, "new"), ExpectedMatch(8, 11, "new") + }).SetName("ReplacementIteration_MultipleMatches_ReturnsOrderedReplacements"); + yield return new TestCaseData("aa", "aaa", "x", new[] { ExpectedMatch(0, 2, "x") }) + .SetName("ReplacementIteration_OverlappingCandidates_ReturnsNonOverlappingMatches"); + } + + [TestCaseSource(nameof(PlainReplacementCases))] + public void ReplacementIteration_PlainTextCases_ReturnExpectedSequence(string patternText, string text, + string replacementText, ExpectedReplacementMatch[] expected) + { + var pattern = MakePattern(patternText); + pattern.ReplaceWith = TsStringUtils.MakeString(replacementText, Ws); + var source = MakeTextSource(text, out var length); + + AssertReplacementMatches(CollectReplacementMatches(pattern, source, length), expected); + AssertReplaceAllMatchesRepeatedFindIn(pattern, source, 0, length); + } + + [TestCase("^", "abc", 0, 0)] + [TestCase("$", "abc", 3, 3)] + public void ReplacementIteration_ZeroLengthRegularExpression_ReturnsOneMatch(string expression, string text, + int ichMin, int ichLim) + { + var pattern = MakePattern(expression); + pattern.UseRegularExpressions = true; + pattern.ReplaceWith = TsStringUtils.MakeString("#", Ws); + var source = MakeTextSource(text, out var length); + + AssertReplacementMatches(CollectReplacementMatches(pattern, source, length), ExpectedMatch(ichMin, ichLim, "#")); + AssertReplaceAllMatchesRepeatedFindIn(pattern, source, 0, length); + } + + [Test] + public void ReplacementIteration_ZeroWidthLookaheadAcrossAstralCharacter_ReturnsCurrentSequence() + { + var pattern = MakePattern("(?=.)"); + pattern.UseRegularExpressions = true; + pattern.ReplaceWith = TsStringUtils.MakeString("#", Ws); + var source = MakeTextSource("\uD83D\uDE00a", out var length); + + AssertReplacementMatches(CollectReplacementMatches(pattern, source, length), + ExpectedMatch(0, 1, "#"), ExpectedMatch(1, 2, "#"), ExpectedMatch(2, 3, "#")); + AssertReplaceAllMatchesRepeatedFindIn(pattern, source, 0, length); + } + + [Test] + public void ReplacementIteration_RegularExpressionCapture_ReturnsExpandedReplacementText() + { + var pattern = MakePattern("(o|e)(ld)"); + pattern.UseRegularExpressions = true; + pattern.ReplaceWith = TsStringUtils.MakeString("$2-$1", Ws); + var source = MakeTextSource("old eld", out var length); + + AssertReplacementMatches(CollectReplacementMatches(pattern, source, length), + ExpectedMatch(0, 3, "ld-o"), ExpectedMatch(4, 7, "ld-e")); + AssertReplaceAllMatchesRepeatedFindIn(pattern, source, 0, length); + } + + /// Describes one locale-sensitive replacement case. + public sealed class CollationReplacementCase + { + internal CollationReplacementCase(string patternText, string text, string locale, string rules, + bool matchDiacritics, ExpectedReplacementMatch[] expected, bool matchCase = true) + { + PatternText = patternText; + Text = text; + Locale = locale; + Rules = rules; + MatchDiacritics = matchDiacritics; + Expected = expected; + MatchCase = matchCase; + } + + internal string PatternText { get; } + internal string Text { get; } + internal string Locale { get; } + internal string Rules { get; } + internal bool MatchDiacritics { get; } + internal ExpectedReplacementMatch[] Expected { get; } + internal bool MatchCase { get; } + } + + private static IEnumerable CollationReplacementCases() + { + yield return new TestCaseData(new CollationReplacementCase("h", "ch", "cs", null, true, + new ExpectedReplacementMatch[0])).SetName("ReplacementIteration_CzechContraction_DoesNotExposeSecondCharacter"); + yield return new TestCaseData(new CollationReplacementCase("a\u0308", " AE ", "de__PHONEBOOK", null, false, + new[] { ExpectedMatch(1, 3, "x") }, matchCase: false)).SetName("ReplacementIteration_GermanPhonebookExpansion_ReturnsOneReplacement"); + yield return new TestCaseData(new CollationReplacementCase("ab", "a-b", "de-u-co-phonebk-ka-shifted", null, true, + new[] { ExpectedMatch(0, 3, "x") })).SetName("ReplacementIteration_ShiftedPunctuation_ReturnsExpandedSpan"); + yield return new TestCaseData(new CollationReplacementCase("a", "b", "root", "&a=b", true, + new[] { ExpectedMatch(0, 1, "x") })).SetName("ReplacementIteration_CustomTailoring_ReturnsTailoredMatch"); + yield return new TestCaseData(new CollationReplacementCase("caf\u00e9", "cafe\u0301", "root", null, true, + new[] { ExpectedMatch(0, 5, "x") })).SetName("ReplacementIteration_NfcPatternAgainstNfdText_ReturnsDecomposedSpan"); + yield return new TestCaseData(new CollationReplacementCase("\u1e09", "c\u0301\u0327", "root", null, true, + new ExpectedReplacementMatch[0])).SetName("ReplacementIteration_NonCanonicalCombiningOrder_ReturnsNoMatch"); + yield return new TestCaseData(new CollationReplacementCase("a", "a\u00ad", "root", null, true, + new[] { ExpectedMatch(0, 2, "x") })).SetName("ReplacementIteration_SoftHyphen_ReturnsExtendedSpan"); + yield return new TestCaseData(new CollationReplacementCase("a", "a\u034f", "root", null, true, + new[] { ExpectedMatch(0, 2, "x") })).SetName("ReplacementIteration_CombiningGraphemeJoiner_ReturnsExtendedSpan"); + yield return new TestCaseData(new CollationReplacementCase("a", "a\u200d", "root", null, true, + new[] { ExpectedMatch(0, 2, "x") })).SetName("ReplacementIteration_ZeroWidthJoiner_ReturnsExtendedSpan"); + yield return new TestCaseData(new CollationReplacementCase("\u0628", "\u0628\u0640", "ar", null, true, + new ExpectedReplacementMatch[0])).SetName("ReplacementIteration_ArabicTatweel_ReturnsNoMatch"); + yield return new TestCaseData(new CollationReplacementCase("\u1820", "\u1820\u180b", "mn", null, true, + new[] { ExpectedMatch(0, 2, "x") })).SetName("ReplacementIteration_MongolianFvs1_ReturnsExtendedSpan"); + yield return new TestCaseData(new CollationReplacementCase("\u1100", "\u1100\u1160", "ko", null, true, + new[] { ExpectedMatch(0, 1, "x") })).SetName("ReplacementIteration_HangulFiller_DoesNotExtendSpan"); + yield return new TestCaseData(new CollationReplacementCase("\u0e01", "\u0e01\u0e4d", "th", null, true, + new[] { ExpectedMatch(0, 1, "x") })).SetName("ReplacementIteration_ThaiNikhahit_DoesNotExtendSpan"); + } + + [TestCaseSource(nameof(CollationReplacementCases))] + public void ReplacementIteration_CollationCases_ReturnExpectedSequence(CollationReplacementCase testCase) + { + var pattern = MakePattern(testCase.PatternText, matchCase: testCase.MatchCase, + matchDiacritics: testCase.MatchDiacritics); + pattern.IcuLocale = testCase.Locale; + if (testCase.Rules != null) + pattern.IcuCollatingRules = testCase.Rules; + pattern.ReplaceWith = TsStringUtils.MakeString("x", Ws); + var source = MakeTextSource(testCase.Text, out var length); + + AssertReplacementMatches(CollectReplacementMatches(pattern, source, length), testCase.Expected); + AssertReplaceAllMatchesRepeatedFindIn(pattern, source, 0, length); + } + + [Test] + public void ReplacementIteration_WholeWord_ReturnsOnlyStandaloneOccurrence() + { + var pattern = MakePattern("cafe", matchWholeWord: true); + pattern.ReplaceWith = TsStringUtils.MakeString("x", Ws); + var source = MakeTextSource("cafeteria cafe", out var length); + + AssertReplacementMatches(CollectReplacementMatches(pattern, source, length), ExpectedMatch(10, 14, "x")); + AssertReplaceAllMatchesRepeatedFindIn(pattern, source, 0, length); + } + + [Test] + public void ReplacementIteration_WritingSystem_ReturnsOnlyMatchingRun() + { + var sourceBuilder = TsStringUtils.MakeString("old old", Ws).GetBldr(); + sourceBuilder.SetIntPropValues(4, 7, (int)FwTextPropType.ktptWs, + (int)FwTextPropVar.ktpvDefault, Cache.DefaultAnalWs); + var source = MakeTextSource(sourceBuilder.GetString(), out var length); + + var propertyPatternBuilder = TsStringUtils.MakeString("old", Cache.DefaultAnalWs).GetBldr(); + var pattern = MakePattern("old"); + pattern.Pattern = propertyPatternBuilder.GetString(); + pattern.MatchOldWritingSystem = true; + pattern.ReplaceWith = TsStringUtils.MakeString("x", Ws); + + AssertReplacementMatches(CollectReplacementMatches(pattern, source, length), ExpectedMatch(4, 7, "x")); + AssertReplaceAllMatchesRepeatedFindIn(pattern, source, 0, length); + } + + [Test] + public void ReplacementIteration_Style_ReturnsOnlyMatchingRun() + { + var sourceBuilder = TsStringUtils.MakeString("old old", Ws).GetBldr(); + sourceBuilder.SetStrPropValue(4, 7, (int)FwTextPropType.ktptNamedStyle, "Emphasis"); + var source = MakeTextSource(sourceBuilder.GetString(), out var length); + + var patternBuilder = TsStringUtils.MakeString("old", Ws).GetBldr(); + patternBuilder.SetStrPropValue(0, 3, (int)FwTextPropType.ktptNamedStyle, "Emphasis"); + var pattern = MakePattern("old"); + pattern.Pattern = patternBuilder.GetString(); + pattern.ReplaceWith = TsStringUtils.MakeString("x", Ws); + + AssertReplacementMatches(CollectReplacementMatches(pattern, source, length), ExpectedMatch(4, 7, "x")); + AssertReplaceAllMatchesRepeatedFindIn(pattern, source, 0, length); + } + + [Test] + public void ReplacementIteration_Tag_ReturnsOnlyMatchingRun() + { + var sourceBuilder = TsStringUtils.MakeString("old old", Ws).GetBldr(); + sourceBuilder.SetStrPropValue(4, 7, (int)FwTextPropType.ktptTags, "tag\0value"); + var source = MakeTextSource(sourceBuilder.GetString(), out var length); + + var patternBuilder = TsStringUtils.MakeString("old", Ws).GetBldr(); + patternBuilder.SetStrPropValue(0, 3, (int)FwTextPropType.ktptTags, "tag\0value"); + var pattern = MakePattern("old"); + pattern.Pattern = patternBuilder.GetString(); + pattern.ReplaceWith = TsStringUtils.MakeString("x", Ws); + + AssertReplacementMatches(CollectReplacementMatches(pattern, source, length), ExpectedMatch(4, 7, "x")); + AssertReplaceAllMatchesRepeatedFindIn(pattern, source, 0, length); + } + + [Test] + public void ReplacementIteration_CaptureReplacement_PreservesReplacementRunProperties() + { + var pattern = MakePattern("(old)"); + pattern.UseRegularExpressions = true; + pattern.MatchOldWritingSystem = true; + var replacementBuilder = TsStringUtils.MakeString("$1", Cache.DefaultAnalWs).GetBldr(); + replacementBuilder.SetStrPropValue(0, 2, (int)FwTextPropType.ktptNamedStyle, "Replacement Style"); + pattern.ReplaceWith = replacementBuilder.GetString(); + var source = MakeTextSource("old", out var length); + + var matches = CollectReplacementMatches(pattern, source, length); + AssertReplacementMatches(matches, ExpectedMatch(0, 3, "old")); + Assert.That(TsStringUtils.GetWsOfRun(matches[0].Replacement, 0), Is.EqualTo(Cache.DefaultAnalWs)); + Assert.That(matches[0].Replacement.get_Properties(0) + .GetStrPropValue((int)FwTextPropType.ktptNamedStyle), Is.EqualTo("Replacement Style")); + AssertReplaceAllMatchesRepeatedFindIn(pattern, source, 0, length); + } + + [Test] + public void ReplaceAllIn_PartialRange_EqualsRepeatedFindIn() + { + var pattern = MakePattern("old"); + pattern.ReplaceWith = TsStringUtils.MakeString("new", Ws); + var source = MakeTextSource("old old old", out var length); + + AssertReplaceAllMatchesRepeatedFindIn(pattern, source, 4, length - 1); + } + + [TestCase(false, 2, "\u0645 \u0645")] + [TestCase(true, 2, "\u0645 \u0645")] + public void ReplaceAllIn_ArabicHarakat_EqualsRepeatedFindIn(bool matchDiacritics, + int expectedCount, string expectedText) + { + var pattern = MakePattern("\u0628\u064e", matchDiacritics: matchDiacritics); + pattern.IcuLocale = "ar"; + var replacementBuilder = TsStringUtils.MakeString("\u0645", Ws).GetBldr(); + replacementBuilder.SetStrPropValue(0, 1, (int)FwTextPropType.ktptNamedStyle, + "Arabic Replacement"); + pattern.ReplaceWith = replacementBuilder.GetString(); + var sourceBuilder = TsStringUtils.MakeString("\u0628 \u0628\u064e", Ws).GetBldr(); + sourceBuilder.SetStrPropValue(2, 4, (int)FwTextPropType.ktptNamedStyle, + "Arabic Source"); + var source = MakeTextSource(sourceBuilder.GetString(), out var length); + + AssertReplaceAllMatchesRepeatedFindIn(pattern, source, 0, length, expectedCount, + expectedText); + } + + [Test] + public void ReplaceAllIn_EmptyTextPattern_ReplacesRunMatchingAllProperties() + { + const string style = "Target Style"; + const string tags = "target\0tag"; + var sourceBuilder = TsStringUtils.MakeString("one two three", Ws).GetBldr(); + sourceBuilder.SetIntPropValues(4, 7, (int)FwTextPropType.ktptWs, + (int)FwTextPropVar.ktpvDefault, Cache.DefaultAnalWs); + sourceBuilder.SetStrPropValue(4, 7, (int)FwTextPropType.ktptNamedStyle, style); + sourceBuilder.SetStrPropValue(4, 7, (int)FwTextPropType.ktptTags, tags); + var source = sourceBuilder.GetString(); + + var patternBuilder = TsStringUtils.MakeString(string.Empty, Cache.DefaultAnalWs).GetBldr(); + patternBuilder.SetStrPropValue(0, 0, (int)FwTextPropType.ktptNamedStyle, style); + patternBuilder.SetStrPropValue(0, 0, (int)FwTextPropType.ktptTags, tags); + var pattern = MakePattern(string.Empty); + pattern.Pattern = patternBuilder.GetString(); + pattern.MatchOldWritingSystem = true; + var replacementBuilder = TsStringUtils.MakeString("X", Ws).GetBldr(); + replacementBuilder.SetStrPropValue(0, 1, (int)FwTextPropType.ktptNamedStyle, + "Replacement Style"); + pattern.ReplaceWith = replacementBuilder.GetString(); + + var actual = ((IVwPattern2)pattern).ReplaceAllIn(source, 0, source.Length, + out var count); + + Assert.That(count, Is.EqualTo(1)); + Assert.That(actual.Text, Is.EqualTo("one X three")); + Assert.That(actual.RunCount, Is.EqualTo(3)); + Assert.That(actual.get_Properties(1).GetStrPropValue( + (int)FwTextPropType.ktptNamedStyle), Is.EqualTo("Replacement Style")); + Assert.That(TsStringUtils.GetWsOfRun(actual, 1), Is.EqualTo(Ws)); + } + + [Test] + public void ReplaceAllIn_PreservesObjectReplacementCharacterAndObjectData() + { + const string objectData = "\u0001object-data"; + var sourceBuilder = TsStringUtils.MakeString("old \uFFFC keep", Ws).GetBldr(); + sourceBuilder.SetStrPropValue(4, 5, (int)FwTextPropType.ktptObjData, objectData); + var source = sourceBuilder.GetString(); + var pattern = MakePattern("old"); + pattern.ReplaceWith = TsStringUtils.MakeString("new", Ws); + + var actual = ((IVwPattern2)pattern).ReplaceAllIn(source, 0, source.Length, + out var count); + + Assert.That(count, Is.EqualTo(1)); + Assert.That(actual.Text, Is.EqualTo("new \uFFFC keep")); + var objectRun = actual.get_RunAt(4); + Assert.That(actual.get_RunText(objectRun), Is.EqualTo("\uFFFC")); + Assert.That(actual.get_Properties(objectRun).GetStrPropValue( + (int)FwTextPropType.ktptObjData), Is.EqualTo(objectData)); + } + + [Test] + public void ReplaceAllIn_LeavesTerminalNoMatchState() + { + var pattern = MakePattern("old"); + pattern.ReplaceWith = TsStringUtils.MakeString("new", Ws); + var source = TsStringUtils.MakeString("old old", Ws); + + var actual = ((IVwPattern2)pattern).ReplaceAllIn(source, 0, source.Length, + out var count); + + Assert.That(actual.Text, Is.EqualTo("new new")); + Assert.That(count, Is.EqualTo(2)); + Assert.Throws(() => { var unused = pattern.ReplacementText; }); + } + + [Test] + public void ReusedPatternAfterLocaleChange_ExtendsShiftedPunctuation() + { + var pattern = MakePattern("a", matchCase: true, matchDiacritics: true); + var source = MakeTextSource("a-", out var length); + Find(pattern, source, 0, length, true, out var rootMin, out var rootLim); + AssertFound(rootMin, rootLim, 0, 1, "root collation span"); + + pattern.IcuLocale = "de-u-co-phonebk-ka-shifted"; + Find(pattern, source, 0, length, true, out var shiftedMin, out var shiftedLim); + + AssertFound(shiftedMin, shiftedLim, 0, 2, "shifted collation span"); + } + } +} diff --git a/Src/xWorks/xWorksTests/BulkEditBarTests.cs b/Src/xWorks/xWorksTests/BulkEditBarTests.cs index 389144135d..4b49d3125b 100644 --- a/Src/xWorks/xWorksTests/BulkEditBarTests.cs +++ b/Src/xWorks/xWorksTests/BulkEditBarTests.cs @@ -10,6 +10,7 @@ using System.Windows.Forms; using System.Xml; using NUnit.Framework; +using SIL.LCModel.Core.KernelInterfaces; using SIL.LCModel.Core.Text; using SIL.FieldWorks.Common.ViewsInterfaces; using SIL.FieldWorks.Common.Controls; @@ -616,7 +617,52 @@ protected override void PersistSortSequence() [TestFixture] public class BulkEditBarTests : BulkEditBarTestsBase { + public enum ChangePath { Preview, Direct } + #region BulkEditEntries tests + [TestCase(ChangePath.Preview, true, true, TestName = "FakeDoit_ComputesAndCachesAnEnabledResultOnce")] + [TestCase(ChangePath.Preview, false, true, TestName = "FakeDoit_DisablesRowsThatCannotChange")] + [TestCase(ChangePath.Preview, true, false, TestName = "FakeDoit_DisablesRowsWithoutAValue")] + [TestCase(ChangePath.Direct, true, true, TestName = "Doit_AppliesAnEnabledResultOnce")] + [TestCase(ChangePath.Direct, false, true, TestName = "Doit_LeavesTheDestinationUnchangedWhenDisabled")] + [TestCase(ChangePath.Direct, true, false, TestName = "Doit_LeavesTheDestinationUnchangedWithoutAValue")] + public void Doit_ComputesEachResultOnce(ChangePath path, bool canChange, bool hasValue) + { + var hvo = Cache.LangProject.LexDbOA.Entries.First().Hvo; + var value = hasValue + ? TsStringUtils.MakeString("changed", Cache.DefaultVernWs) + : null; + CountingFieldReadWriter accessor; + var method = CreateCountingDoItMethod(path, canChange, value, out accessor); + var before = accessor.CurrentValue(hvo).Text; + + if (path == ChangePath.Preview) + { + var sentinel = TsStringUtils.MakeString("previous preview", Cache.DefaultVernWs); + m_bv.SpecialCache.SetString(hvo, XMLViewsDataCache.ktagAlternateValue, sentinel); + method.FakeDoit(new[] { hvo }, XMLViewsDataCache.ktagAlternateValue, + XMLViewsDataCache.ktagItemEnabled, new NullProgressState()); + + var expected = canChange && hasValue ? value : sentinel; + Assert.That(m_bv.SpecialCache.get_StringProp(hvo, + XMLViewsDataCache.ktagAlternateValue), Is.SameAs(expected)); + Assert.That(m_bv.SpecialCache.get_IntProp(hvo, + XMLViewsDataCache.ktagItemEnabled), Is.EqualTo(canChange && hasValue ? 1 : 0)); + } + else + { + DoWithUndoTask(() => method.Doit(hvo)); + Assert.That(accessor.SetNewValueCount, + Is.EqualTo(canChange && hasValue ? 1 : 0)); + Assert.That(accessor.CurrentValue(hvo).Text, + Is.EqualTo(canChange && hasValue ? value.Text : before)); + } + + Assert.That(method.TryGetNewValueCount, Is.EqualTo(1)); + Assert.That(method.OkToChangeCount, Is.EqualTo(1)); + Assert.That(method.NewValueCount, Is.EqualTo(canChange ? 1 : 0)); + } + [Test] public void FilterBar_HeaderAndFilterControlsExposeReachableBaseline() { @@ -1621,6 +1667,97 @@ private ILexEntryRef MakeComplexFormLexEntryRef(ILexEntry ownerEntry) } #endregion BulkEditEntries tests + + private CountingDoItMethod CreateCountingDoItMethod(ChangePath path, bool canChange, + ITsString value, out CountingFieldReadWriter accessor) + { + var document = new XmlDocument(); + document.LoadXml(""); + accessor = new CountingFieldReadWriter(FieldReadWriter.Create(document.DocumentElement, Cache)); + var dataAccess = path == ChangePath.Preview + ? m_bv.SpecialCache + : (ISilDataAccessManaged)Cache.DomainDataByFlid; + return new CountingDoItMethod(Cache, dataAccess, + accessor, document.DocumentElement, canChange, value); + } + + private void DoWithUndoTask(Action action) + { + Cache.DomainDataByFlid.BeginUndoTask("test", "test"); + try + { + action(); + } + finally + { + Cache.DomainDataByFlid.EndUndoTask(); + } + } + + private sealed class CountingDoItMethod : DoItMethod + { + private readonly bool m_canChange; + private readonly ITsString m_value; + + internal CountingDoItMethod(LcmCache cache, ISilDataAccessManaged sda, + FieldReadWriter accessor, XmlNode spec, bool canChange, ITsString value) + : base(cache, sda, accessor, spec) + { + m_canChange = canChange; + m_value = value; + } + + internal int TryGetNewValueCount { get; private set; } + internal int OkToChangeCount { get; private set; } + internal int NewValueCount { get; private set; } + + protected override bool TryGetNewValue(int hvo, out ITsString newValue) + { + TryGetNewValueCount++; + return base.TryGetNewValue(hvo, out newValue); + } + + protected override bool OkToChange(int hvo) + { + OkToChangeCount++; + return m_canChange; + } + + protected override ITsString NewValue(int hvo) + { + NewValueCount++; + return m_value; + } + } + + private sealed class CountingFieldReadWriter : FieldReadWriter + { + private readonly FieldReadWriter m_inner; + + internal CountingFieldReadWriter(FieldReadWriter inner) + : base(inner.DataAccess) + { + m_inner = inner; + } + + internal int SetNewValueCount { get; private set; } + + public override ITsString CurrentValue(int hvo) + { + return m_inner.CurrentValue(hvo); + } + + public override void SetNewValue(int hvo, ITsString tss) + { + SetNewValueCount++; + m_inner.SetNewValue(hvo, tss); + } + + public override int WritingSystem + { + get { return m_inner.WritingSystem; } + } + } } /// From e6de72c06ea50af089a6cb9bd5635a3c0040d362 Mon Sep 17 00:00:00 2001 From: John Lambert Date: Fri, 14 Aug 2026 03:06:04 -0400 Subject: [PATCH 2/3] Fix bulk-edit undo-task safety and close review gaps found in preflight Guard the outer bulk-edit undo task with try/finally so a native ReplaceAllIn fault mid-batch cannot leave it unterminated. Share one computed value between OkToChange and TryGetNewValue in BulkCopyMethod and TransduceMethod instead of computing twice, scoped to stay correct across separate preview/apply calls. Add native ReplaceAllIn coverage for ORC, writing-system restriction, case/diacritics, and canonical equivalence. Harden the NFD-normalization skip check with non-Latin script and rich-run test cases. Co-Authored-By: Claude Sonnet 5 --- Src/Common/Controls/XMLViews/BulkEditBar.cs | 66 +++- Src/views/Test/TestVwPattern.h | 306 +++++++++++++++++- .../Performance/ReplaceWithMethodTests.cs | 65 ++++ Src/xWorks/xWorksTests/BulkEditBarTests.cs | 72 +++++ 4 files changed, 490 insertions(+), 19 deletions(-) diff --git a/Src/Common/Controls/XMLViews/BulkEditBar.cs b/Src/Common/Controls/XMLViews/BulkEditBar.cs index 356370c26f..3e1167e357 100644 --- a/Src/Common/Controls/XMLViews/BulkEditBar.cs +++ b/Src/Common/Controls/XMLViews/BulkEditBar.cs @@ -4838,22 +4838,28 @@ public void FakeDoit(IEnumerable itemsToChange, int tagFakeFlid, int tagEna public void Doit(IEnumerable itemsToChange, ProgressState state) { m_sda.BeginUndoTask(XMLViewsStrings.ksUndoBulkEdit, XMLViewsStrings.ksRedoBulkEdit); - string commitChanges = XmlUtils.GetOptionalAttributeValue(m_nodeSpec, "commitChanges"); - int i = 0; - // Report progress 50 times or every 100 items, whichever is more (but no more than once per item!) - int interval = Math.Min(100, Math.Max(itemsToChange.Count() / 50, 1)); - foreach (int hvo in itemsToChange) + try { - i++; - if (i % interval == 0) + string commitChanges = XmlUtils.GetOptionalAttributeValue(m_nodeSpec, "commitChanges"); + int i = 0; + // Report progress 50 times or every 100 items, whichever is more (but no more than once per item!) + int interval = Math.Min(100, Math.Max(itemsToChange.Count() / 50, 1)); + foreach (int hvo in itemsToChange) { - state.PercentDone = i * 100 / itemsToChange.Count(); - state.Breath(); + i++; + if (i % interval == 0) + { + state.PercentDone = i * 100 / itemsToChange.Count(); + state.Breath(); + } + Doit(hvo); + BulkEditBar.CommitChanges(hvo, commitChanges, m_cache, m_accessor.WritingSystem); } - Doit(hvo); - BulkEditBar.CommitChanges(hvo, commitChanges, m_cache, m_accessor.WritingSystem); } - m_sda.EndUndoTask(); + finally + { + m_sda.EndUndoTask(); + } } /// @@ -4910,11 +4916,41 @@ protected virtual bool TryGetNewValue(int hvo, out ITsString newValue) { newValue = null; if (!OkToChange(hvo)) + { + ClearNewValueCache(); return false; - newValue = NewValue(hvo); + } + newValue = NewValueCached(hvo); + ClearNewValueCache(); return newValue != null; } + private int? m_hvoCachedNewValue; + private ITsString m_cachedNewValue; + + /// + /// Returns NewValue(hvo), reusing a value an OkToChange override already computed + /// for the same hvo within this same TryGetNewValue call (to decide whether a change + /// would occur) instead of recomputing it. The cache is cleared at the end of every + /// TryGetNewValue call: a later call for the same hvo (e.g. preview, then apply) must + /// still recompute, since the underlying data or pattern state may have changed. + /// + protected ITsString NewValueCached(int hvo) + { + if (m_hvoCachedNewValue != hvo) + { + m_cachedNewValue = NewValue(hvo); + m_hvoCachedNewValue = hvo; + } + return m_cachedNewValue; + } + + private void ClearNewValueCache() + { + m_hvoCachedNewValue = null; + m_cachedNewValue = null; + } + protected abstract ITsString NewValue(int hvo); #region IGetReplacedObjects Members @@ -4961,7 +4997,7 @@ protected override bool OkToChange(int hvo) string sOld = null; if (tssOld != null) sOld = tssOld.Text; - ITsString tssNew = NewValue(hvo); + ITsString tssNew = NewValueCached(hvo); string sNew = null; if (tssNew != null) sNew = tssNew.Text; @@ -5032,7 +5068,7 @@ protected override bool OkToChange(int hvo) return false; } ITsString tssSrc = m_srcAccessor.CurrentValue(hvo); - return tssSrc != null && !m_srcAccessor.CurrentValue(hvo).Equals(NewValue(hvo)); + return tssSrc != null && !m_srcAccessor.CurrentValue(hvo).Equals(NewValueCached(hvo)); } diff --git a/Src/views/Test/TestVwPattern.h b/Src/views/Test/TestVwPattern.h index f19d3a442d..c5c4f6d1ec 100644 --- a/Src/views/Test/TestVwPattern.h +++ b/Src/views/Test/TestVwPattern.h @@ -732,19 +732,19 @@ namespace TestViews int ichMin, ichLim; CheckHr(m_qpat->put_MatchDiacritics(false)); // This is set true by default. CheckHr(m_qpat->FindIn(m_qts, 0, stuSearch.Length(), true, &ichMin, &ichLim, NULL)); - unitpp::assert_eq("Found 'Änd' at start (ignoring diacritics)", 0, ichMin); - unitpp::assert_eq("End of 'Änd' at start (ignoring diacritics)", 3, ichLim); + unitpp::assert_eq("Found '�nd' at start (ignoring diacritics)", 0, ichMin); + unitpp::assert_eq("End of '�nd' at start (ignoring diacritics)", 3, ichLim); // ...but not match requiring diacritics to match CheckHr(m_qpat->put_MatchDiacritics(true)); CheckHr(m_qpat->FindIn(m_qts, 0, stuSearch.Length(), true, &ichMin, &ichLim, NULL)); - unitpp::assert_eq("Skipped 'Änd' at start (match diacritics)", kichoffsetofAnd, ichMin); + unitpp::assert_eq("Skipped '�nd' at start (match diacritics)", kichoffsetofAnd, ichMin); // ...still not if we have to match case CheckHr(m_qpat->put_MatchDiacritics(false)); CheckHr(m_qpat->put_MatchCase(true)); CheckHr(m_qpat->FindIn(m_qts, 0, stuSearch.Length(), true, &ichMin, &ichLim, NULL)); - unitpp::assert_eq("Skipped 'Änd' at start, match case", kichoffsetofAnd, ichMin); + unitpp::assert_eq("Skipped '�nd' at start, match case", kichoffsetofAnd, ichMin); } /*-------------------------------------------------------------------------------------- @@ -1934,6 +1934,304 @@ namespace TestViews unitpp::assert_true("result remains null after replacement failure", ptssResult == NULL); } + + // Re-run the TE4727 regression (replacing the character immediately preceding a + // trailing owned ORC) through the bulk ReplaceAllIn path instead of FindIn, and + // confirm the ORC and its footnote guid data survive the splice. + void testReplaceAllInReplaceCharPrecedingFinalORC_TE4727() + { + unitpp::assert_true("English writing system exists", m_qwsEng.Ptr()); + + ITsStrBldrPtr qtsbStringBuilder; + qtsbStringBuilder.CreateInstance(CLSID_TsStrBldr); + ITsPropsBldrPtr qtpbTextPropsBuilder; + qtpbTextPropsBuilder.CreateInstance(CLSID_TsPropsBldr); + CheckHr(qtpbTextPropsBuilder->SetIntPropValues(ktptWs, ktpvDefault, g_wsEng)); + ITsTextPropsPtr qttp; + CheckHr(qtpbTextPropsBuilder->GetTextProps(&qttp)); + StrUni stuSearch(L"Tha Tant of tha Lord's Prasance"); + CheckHr(qtsbStringBuilder->Replace(0, 0, stuSearch.Bstr(), qttp)); + + // Insert a footnote ORC as the very last character of the string. + StrUni stuData; + OLECHAR * prgchData; + GUID uidFootnote; + CheckHr(CoCreateGuid(&uidFootnote)); + stuData.SetSize(isizeof(GUID) / isizeof(OLECHAR) + 1, &prgchData); + *prgchData = kodtOwnNameGuidHot; + memmove(prgchData + 1, &uidFootnote, isizeof(uidFootnote)); + CheckHr(qtpbTextPropsBuilder->SetStrPropValue(ktptObjData, stuData.Bstr())); + CheckHr(qtpbTextPropsBuilder->GetTextProps(&qttp)); + OLECHAR chObj = kchObject; + CheckHr(qtsbStringBuilder->ReplaceRgch(stuSearch.Length(), stuSearch.Length(), + &chObj, 1, qttp)); + ITsStringPtr qtssSource; + CheckHr(qtsbStringBuilder->GetString(&qtssSource)); + + ITsStringPtr qtssPattern; + StrUni stuPattern(L"e"); + CheckHr(m_qtsf->MakeString(stuPattern.Bstr(), g_wsEng, &qtssPattern)); + CheckHr(m_qpat->putref_Pattern(qtssPattern)); + CheckHr(m_qpat->put_UseRegularExpressions(false)); + + ITsStringPtr qtssReplacement; + StrUni stuReplacement(L"E"); + CheckHr(m_qtsf->MakeString(stuReplacement.Bstr(), g_wsEng, &qtssReplacement)); + CheckHr(m_qpat->putref_ReplaceWith(qtssReplacement)); + + int cchTotal; + CheckHr(qtssSource->get_Length(&cchTotal)); + + int cExpected; + ITsStringPtr qtssExpected = ReplaceWithRepeatedFindIn(qtssSource, 0, cchTotal, + &cExpected); + + int cMatches = -1; + ITsStringPtr qtssResult; + CheckHr(m_qpat2->ReplaceAllIn(qtssSource, 0, cchTotal, &cMatches, &qtssResult)); + unitpp::assert_eq("single 'e' preceding final ORC found", 1, cMatches); + unitpp::assert_eq("match count matches repeated FindIn", cExpected, cMatches); + ComBool fEqual; + CheckHr(qtssResult->Equals(qtssExpected, &fEqual)); + unitpp::assert_true("bulk result equals repeated FindIn result", fEqual); + + // Confirm the ORC after the replaced character survived intact, with its + // footnote guid data untouched. + int crun; + CheckHr(qtssResult->get_RunCount(&crun)); + SmartBstr sbstrLastRun; + CheckHr(qtssResult->get_RunText(crun - 1, &sbstrLastRun)); + unitpp::assert_eq("last run is the ORC", kchObject, sbstrLastRun[0]); + ITsTextPropsPtr qttpLastRun; + CheckHr(qtssResult->get_Properties(crun - 1, &qttpLastRun)); + SmartBstr sbstrObjData; + CheckHr(qttpLastRun->GetStrPropValue(ktptObjData, &sbstrObjData)); + unitpp::assert_true("trailing ORC's footnote guid data preserved", + !wcscmp(stuData.Chars(), sbstrObjData.Chars())); + } + + // Re-run the canonical/writing-system-restricted matching combinations from + // testMatchingWs through ReplaceAllIn, confirming bulk match counts and results + // agree with repeated FindIn as MatchOldWritingSystem and MatchDiacritics change. + void testReplaceAllInRespectsMatchOldWritingSystem() + { + unitpp::assert_true("English writing system exists", m_qwsEng.Ptr()); + + ITsStringPtr qtssSearchT; + StrUni stuSearch(L"abc" A_WITH_DIAERESIS COMBINING_DOT_BELOW L"abcA" COMBINING_DOT_BELOW + COMBINING_DIAERESIS L"rubbish"); + CheckHr(m_qtsf->MakeString(stuSearch.Bstr(), g_wsEng, &qtssSearchT)); + ITsStringPtr qtssSearch; + CheckHr(qtssSearchT->get_NormalizedForm(knmNFD, &qtssSearch)); + ITsStrBldrPtr qtsb; + CheckHr(qtssSearch->GetBldr(&qtsb)); + // Make the first A with diacritics and the later A and combining dot french. + CheckHr(qtsb->SetIntPropValues(3, 6, ktptWs, ktpvDefault, g_wsFrn)); + CheckHr(qtsb->SetIntPropValues(9, 11, ktptWs, ktpvDefault, g_wsFrn)); + CheckHr(qtsb->GetString(&qtssSearch)); + + ITsStringPtr qtssPattern; + StrUni stuPattern(L"cA" COMBINING_DOT_BELOW COMBINING_DIAERESIS); + CheckHr(m_qtsf->MakeString(stuPattern.Bstr(), g_wsEng, &qtssPattern)); + CheckHr(qtssPattern->GetBldr(&qtsb)); + // Make the first A with diacritics and the later combining macron french. + CheckHr(qtsb->SetIntPropValues(1, 4, ktptWs, ktpvDefault, g_wsFrn)); + CheckHr(qtsb->GetString(&qtssPattern)); + CheckHr(m_qpat->putref_Pattern(qtssPattern)); + + ITsStringPtr qtssReplacement; + StrUni stuReplacement(L"X"); + CheckHr(m_qtsf->MakeString(stuReplacement.Bstr(), g_wsEng, &qtssReplacement)); + CheckHr(m_qpat->putref_ReplaceWith(qtssReplacement)); + + int len = stuSearch.Length(); + ComBool fEqual; + + // Default: MatchOldWritingSystem is off, so both canonical occurrences match. + int cExpected; + ITsStringPtr qtssExpected = ReplaceWithRepeatedFindIn(qtssSearch, 0, len, + &cExpected); + int cMatches = -1; + ITsStringPtr qtssResult; + CheckHr(m_qpat2->ReplaceAllIn(qtssSearch, 0, len, &cMatches, &qtssResult)); + unitpp::assert_eq("both canonical matches replaced without ws matching", 2, + cMatches); + unitpp::assert_eq("count matches repeated FindIn (no ws)", cExpected, cMatches); + CheckHr(qtssResult->Equals(qtssExpected, &fEqual)); + unitpp::assert_true("bulk result equals repeated FindIn (no ws)", fEqual); + + // Requiring old ws but ignoring diacritics: still both occurrences match. + CheckHr(m_qpat->put_MatchDiacritics(false)); + CheckHr(m_qpat->put_MatchOldWritingSystem(true)); + qtssExpected = ReplaceWithRepeatedFindIn(qtssSearch, 0, len, &cExpected); + cMatches = -1; + qtssResult.Clear(); + CheckHr(m_qpat2->ReplaceAllIn(qtssSearch, 0, len, &cMatches, &qtssResult)); + unitpp::assert_eq("both canonical matches replaced, ws required, diacritics ignored", + 2, cMatches); + unitpp::assert_eq("count matches repeated FindIn (ws, no diacritics)", cExpected, + cMatches); + CheckHr(qtssResult->Equals(qtssExpected, &fEqual)); + unitpp::assert_true("bulk result equals repeated FindIn (ws, no diacritics)", + fEqual); + + // Requiring both old ws and diacritics narrows the match to the first occurrence. + CheckHr(m_qpat->put_MatchDiacritics(true)); + qtssExpected = ReplaceWithRepeatedFindIn(qtssSearch, 0, len, &cExpected); + cMatches = -1; + qtssResult.Clear(); + CheckHr(m_qpat2->ReplaceAllIn(qtssSearch, 0, len, &cMatches, &qtssResult)); + unitpp::assert_eq("only first occurrence matches ws+diacritics", 1, cMatches); + unitpp::assert_eq("count matches repeated FindIn (ws + diacritics)", cExpected, + cMatches); + CheckHr(qtssResult->Equals(qtssExpected, &fEqual)); + unitpp::assert_true("bulk result equals repeated FindIn (ws + diacritics)", fEqual); + } + + // Re-run the MatchCase / MatchDiacritics option combinations from testMatchingCase + // and testMatchingDiacritics through ReplaceAllIn. + void testReplaceAllInWithCaseAndDiacriticsOptions() + { + unitpp::assert_true("English writing system exists", m_qwsEng.Ptr()); + ComBool fEqual; + + // --- Case sensitivity --- + ITsStringPtr qtssSearchCase; + StrUni stuSearchCase(L"And this sentence uses 'and' a lot"); + CheckHr(m_qtsf->MakeString(stuSearchCase.Bstr(), g_wsEng, &qtssSearchCase)); + + ITsStringPtr qtssPattern; + StrUni stuPattern(L"and"); + CheckHr(m_qtsf->MakeString(stuPattern.Bstr(), g_wsEng, &qtssPattern)); + CheckHr(m_qpat->putref_Pattern(qtssPattern)); + + ITsStringPtr qtssReplacement; + StrUni stuReplacement(L"or"); + CheckHr(m_qtsf->MakeString(stuReplacement.Bstr(), g_wsEng, &qtssReplacement)); + CheckHr(m_qpat->putref_ReplaceWith(qtssReplacement)); + + int lenCase = stuSearchCase.Length(); + CheckHr(m_qpat->put_MatchDiacritics(true)); + CheckHr(m_qpat->put_MatchCase(false)); + + int cExpected; + ITsStringPtr qtssExpected = ReplaceWithRepeatedFindIn(qtssSearchCase, 0, lenCase, + &cExpected); + int cMatches = -1; + ITsStringPtr qtssResult; + CheckHr(m_qpat2->ReplaceAllIn(qtssSearchCase, 0, lenCase, &cMatches, &qtssResult)); + unitpp::assert_eq("case-insensitive replaces both 'And' and 'and'", 2, cMatches); + unitpp::assert_eq("case-insensitive count matches repeated FindIn", cExpected, + cMatches); + CheckHr(qtssResult->Equals(qtssExpected, &fEqual)); + unitpp::assert_true("case-insensitive bulk result equals repeated FindIn", fEqual); + + CheckHr(m_qpat->put_MatchCase(true)); + qtssExpected = ReplaceWithRepeatedFindIn(qtssSearchCase, 0, lenCase, &cExpected); + cMatches = -1; + qtssResult.Clear(); + CheckHr(m_qpat2->ReplaceAllIn(qtssSearchCase, 0, lenCase, &cMatches, &qtssResult)); + unitpp::assert_eq("match case skips capitalized 'And'", 1, cMatches); + unitpp::assert_eq("match-case count matches repeated FindIn", cExpected, cMatches); + CheckHr(qtssResult->Equals(qtssExpected, &fEqual)); + unitpp::assert_true("match-case bulk result equals repeated FindIn", fEqual); + SmartBstr sbstrResult; + CheckHr(qtssResult->get_Text(&sbstrResult)); + unitpp::assert_true("only the lowercase 'and' was replaced", + sbstrResult == L"And this sentence uses 'or' a lot"); + + // --- Diacritics sensitivity --- + ITsStringPtr qtssSearchDia; + StrUni stuSearchDia(LATIN_CAPITAL_A_WITH_DIARESIS L"nd this sentence uses 'and' a lot"); + CheckHr(m_qtsf->MakeString(stuSearchDia.Bstr(), g_wsEng, &qtssSearchDia)); + CheckHr(m_qpat->putref_Pattern(qtssPattern)); + CheckHr(m_qpat->putref_ReplaceWith(qtssReplacement)); + CheckHr(m_qpat->put_MatchCase(false)); + CheckHr(m_qpat->put_MatchDiacritics(false)); + + int lenDia = stuSearchDia.Length(); + qtssExpected = ReplaceWithRepeatedFindIn(qtssSearchDia, 0, lenDia, &cExpected); + cMatches = -1; + qtssResult.Clear(); + CheckHr(m_qpat2->ReplaceAllIn(qtssSearchDia, 0, lenDia, &cMatches, &qtssResult)); + unitpp::assert_eq("ignoring diacritics replaces both occurrences", 2, cMatches); + unitpp::assert_eq("ignore-diacritics count matches repeated FindIn", cExpected, + cMatches); + CheckHr(qtssResult->Equals(qtssExpected, &fEqual)); + unitpp::assert_true("ignore-diacritics bulk result equals repeated FindIn", fEqual); + + CheckHr(m_qpat->put_MatchDiacritics(true)); + qtssExpected = ReplaceWithRepeatedFindIn(qtssSearchDia, 0, lenDia, &cExpected); + cMatches = -1; + qtssResult.Clear(); + CheckHr(m_qpat2->ReplaceAllIn(qtssSearchDia, 0, lenDia, &cMatches, &qtssResult)); + unitpp::assert_eq("requiring diacritics skips the accented occurrence", 1, + cMatches); + unitpp::assert_eq("match-diacritics count matches repeated FindIn", cExpected, + cMatches); + CheckHr(qtssResult->Equals(qtssExpected, &fEqual)); + unitpp::assert_true("match-diacritics bulk result equals repeated FindIn", fEqual); + } + + // Re-run testSearchCanonical's NFD canonical-equivalence scenario through + // ReplaceAllIn: the pattern is correctly-ordered decomposed text, while the source + // contains both a fully-composed and an out-of-order-decomposed occurrence. + void testReplaceAllInCanonicalEquivalence() + { + unitpp::assert_true("English writing system exists", m_qwsEng.Ptr()); + + ITsStringPtr qtssSearchT; + StrUni stuSearch(L"abc" A_WITH_DIAERESIS COMBINING_DOT_BELOW L"abcA" COMBINING_DOT_BELOW + COMBINING_DIAERESIS L"rubbish"); + CheckHr(m_qtsf->MakeString(stuSearch.Bstr(), g_wsEng, &qtssSearchT)); + ITsStringPtr qtssSearch; + CheckHr(qtssSearchT->get_NormalizedForm(knmNFD, &qtssSearch)); + + ITsStringPtr qtssPattern; + StrUni stuPattern(L"cA" COMBINING_DOT_BELOW COMBINING_DIAERESIS); + CheckHr(m_qtsf->MakeString(stuPattern.Bstr(), g_wsEng, &qtssPattern)); + CheckHr(m_qpat->putref_Pattern(qtssPattern)); + + ITsStringPtr qtssReplacement; + StrUni stuReplacement(L"XYZ"); + CheckHr(m_qtsf->MakeString(stuReplacement.Bstr(), g_wsEng, &qtssReplacement)); + CheckHr(m_qpat->putref_ReplaceWith(qtssReplacement)); + + int len = stuSearch.Length(); + ComBool fEqual; + + int cExpected; + ITsStringPtr qtssExpected = ReplaceWithRepeatedFindIn(qtssSearch, 0, len, + &cExpected); + int cMatches = -1; + ITsStringPtr qtssResult; + CheckHr(m_qpat2->ReplaceAllIn(qtssSearch, 0, len, &cMatches, &qtssResult)); + unitpp::assert_eq("both canonically-equivalent occurrences replaced", 2, cMatches); + unitpp::assert_eq("canonical count matches repeated FindIn", cExpected, cMatches); + CheckHr(qtssResult->Equals(qtssExpected, &fEqual)); + unitpp::assert_true("canonical bulk result equals repeated FindIn", fEqual); + SmartBstr sbstrResult; + CheckHr(qtssResult->get_Text(&sbstrResult)); + unitpp::assert_true("both occurrences replaced with XYZ", + sbstrResult == L"abXYZabXYZrubbish"); + + // Same result even requiring case and diacritics to match: the canonical + // equivalence logic still finds both. + CheckHr(m_qpat->put_MatchDiacritics(true)); + CheckHr(m_qpat->put_MatchCase(true)); + qtssExpected = ReplaceWithRepeatedFindIn(qtssSearch, 0, len, &cExpected); + cMatches = -1; + qtssResult.Clear(); + CheckHr(m_qpat2->ReplaceAllIn(qtssSearch, 0, len, &cMatches, &qtssResult)); + unitpp::assert_eq("both occurrences still replaced, match case/diacritics", 2, + cMatches); + unitpp::assert_eq("canonical count matches repeated FindIn, match case/diacritics", + cExpected, cMatches); + CheckHr(qtssResult->Equals(qtssExpected, &fEqual)); + unitpp::assert_true( + "canonical bulk result equals repeated FindIn, match case/diacritics", fEqual); + } + virtual void Setup() { CreateTestWritingSystemFactory(); diff --git a/Src/xWorks/xWorksTests/Avalonia/Performance/ReplaceWithMethodTests.cs b/Src/xWorks/xWorksTests/Avalonia/Performance/ReplaceWithMethodTests.cs index 8765dd73ad..81bd814a41 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Performance/ReplaceWithMethodTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Performance/ReplaceWithMethodTests.cs @@ -266,6 +266,71 @@ public void FakeDoit_NormalizesBulkReplacementResultToNfd() Assert.That(preview.Text.IsNormalized(NormalizationForm.FormD), Is.True); } + [Test] + public void FakeDoit_NormalizesNonLatinScriptReplacementResultToNfd() + { + // U+AC00 (Hangul syllable "GA") has a real canonical decomposition to two + // Jamo characters under NFD, unlike the single-diacritic Latin case above. + // This hardens the NormalizeResult skip-check for a non-Latin script. + var entry = CreateEntries(1, 1, "old \uac00", "old \uac00").Single(); + var document = BuildColumnSpec(); + var accessor = FieldReadWriter.Create(document.DocumentElement, Cache); + var pattern = BuildPattern("old", "new \uac00"); + var method = new CountingReplaceWithMethod(Cache, m_bv.SpecialCache, accessor, + document.DocumentElement, pattern, pattern.ReplaceWith); + + method.FakeDoit(new[] { entry.Hvo }, XMLViewsDataCache.ktagAlternateValue, + XMLViewsDataCache.ktagItemEnabled, new NullProgressState()); + + var preview = m_bv.SpecialCache.get_StringProp(entry.Hvo, + XMLViewsDataCache.ktagAlternateValue); + Assert.That(method.ReplaceAllInCount, Is.EqualTo(1)); + Assert.That(preview.Text, Is.EqualTo("new \uac00 \uac00".Normalize(NormalizationForm.FormD))); + Assert.That(preview.Text.IsNormalized(NormalizationForm.FormD), Is.True); + } + + [Test] + public void FakeDoit_PreservesRichRunPropertiesWhenNormalizingNonLatinReplacementResult() + { + ILexEntry entry = null; + ILexSense sense = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + entry = Cache.ServiceLocator.GetInstance().Create(); + sense = Cache.ServiceLocator.GetInstance().Create(entry, null, string.Empty); + m_createdObjectList.Add(entry); + }); + var document = BuildColumnSpec(); + var accessor = new OwnMlPropReadWriter(Cache, LexSenseTags.kflidDefinition, Cache.DefaultAnalWs); + // U+AC01 ("GAG") canonically decomposes to THREE Jamo characters under NFD, + // so this run must grow during normalization while keeping its style intact. + var sourceBuilder = TsStringUtils.MakeString("old \uac01", Cache.DefaultAnalWs).GetBldr(); + sourceBuilder.SetIntPropValues(4, 5, (int)FwTextPropType.ktptWs, + (int)FwTextPropVar.ktpvDefault, Cache.DefaultVernWs); + sourceBuilder.SetStrPropValue(4, 5, (int)FwTextPropType.ktptNamedStyle, "Emphasis"); + var source = sourceBuilder.GetString(); + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, + () => accessor.SetNewValue(sense.Hvo, source)); + Assert.That(accessor.CurrentValue(sense.Hvo).RunCount, Is.EqualTo(2)); + + // Replacement text is itself a precomposed Hangul syllable, so the result + // needs real NFD decomposition work, not the already-normalized skip path. + var pattern = BuildPattern("old", "new \uac00"); + var method = new CountingReplaceWithMethod(Cache, m_bv.SpecialCache, accessor, + document.DocumentElement, pattern, pattern.ReplaceWith); + method.FakeDoit(new[] { sense.Hvo }, XMLViewsDataCache.ktagAlternateValue, + XMLViewsDataCache.ktagItemEnabled, new NullProgressState()); + var preview = m_bv.SpecialCache.get_StringProp(sense.Hvo, XMLViewsDataCache.ktagAlternateValue); + + Assert.That(method.ReplaceAllInCount, Is.EqualTo(1)); + Assert.That(preview.Text, Is.EqualTo("new \uac00 \uac01".Normalize(NormalizationForm.FormD))); + Assert.That(preview.Text.IsNormalized(NormalizationForm.FormD), Is.True); + + var styledRun = Enumerable.Range(0, preview.RunCount) + .Single(i => preview.get_Properties(i).GetStrPropValue((int)FwTextPropType.ktptNamedStyle) == "Emphasis"); + Assert.That(TsStringUtils.GetWsOfRun(preview, styledRun), Is.EqualTo(Cache.DefaultVernWs)); + } + [Test] public void FakeDoit_BaseGateRejectionDoesNotSearchOrChangePreview() { diff --git a/Src/xWorks/xWorksTests/BulkEditBarTests.cs b/Src/xWorks/xWorksTests/BulkEditBarTests.cs index 4b49d3125b..a9bd390927 100644 --- a/Src/xWorks/xWorksTests/BulkEditBarTests.cs +++ b/Src/xWorks/xWorksTests/BulkEditBarTests.cs @@ -663,6 +663,39 @@ public void Doit_ComputesEachResultOnce(ChangePath path, bool canChange, bool ha Assert.That(method.NewValueCount, Is.EqualTo(canChange ? 1 : 0)); } + [Test] + public void Doit_OuterLoop_EndsUndoTaskWhenAnItemThrows() + { + var hvo = Cache.LangProject.LexDbOA.Entries.First().Hvo; + var document = new XmlDocument(); + document.LoadXml(""); + var accessor = FieldReadWriter.Create(document.DocumentElement, Cache); + var method = new ThrowingDoItMethod(Cache, (ISilDataAccessManaged)Cache.DomainDataByFlid, + accessor, document.DocumentElement); + + Assert.That(Cache.ActionHandlerAccessor.CurrentDepth, Is.EqualTo(0)); + + Assert.Throws(() => + method.Doit(new[] { hvo }, new NullProgressState())); + + Assert.That(Cache.ActionHandlerAccessor.CurrentDepth, Is.EqualTo(0), + "the outer bulk-edit undo task must be ended even when applying one item throws"); + } + + private sealed class ThrowingDoItMethod : DoItMethod + { + internal ThrowingDoItMethod(LcmCache cache, ISilDataAccessManaged sda, + FieldReadWriter accessor, XmlNode spec) + : base(cache, sda, accessor, spec) + { + } + + protected override ITsString NewValue(int hvo) + { + throw new InvalidOperationException("simulated failure applying one bulk-edit item"); + } + } + [Test] public void FilterBar_HeaderAndFilterControlsExposeReachableBaseline() { @@ -1730,6 +1763,37 @@ protected override ITsString NewValue(int hvo) } } + [Test] + public void BulkCopy_ComputesSourceValueOnce() + { + var hvo = Cache.LangProject.LexDbOA.Entries.First().Hvo; + + var srcDoc = new XmlDocument(); + srcDoc.LoadXml(""); + var srcAccessor = new CountingFieldReadWriter(FieldReadWriter.Create(srcDoc.DocumentElement, Cache)); + + var dstDoc = new XmlDocument(); + dstDoc.LoadXml(""); + var dstAccessor = FieldReadWriter.Create(dstDoc.DocumentElement, Cache); + + DoWithUndoTask(() => + { + srcAccessor.SetNewValue(hvo, TsStringUtils.MakeString("source note", Cache.DefaultAnalWs)); + dstAccessor.SetNewValue(hvo, TsStringUtils.MakeString("existing citation form", Cache.DefaultVernWs)); + }); + srcAccessor.ResetCounts(); + + var method = new BulkCopyMethod(Cache, m_bv.SpecialCache, dstAccessor, dstDoc.DocumentElement, + srcAccessor, null, NonEmptyTargetOptions.Overwrite); + + method.FakeDoit(new[] { hvo }, XMLViewsDataCache.ktagAlternateValue, + XMLViewsDataCache.ktagItemEnabled, new NullProgressState()); + + Assert.That(m_bv.SpecialCache.get_IntProp(hvo, XMLViewsDataCache.ktagItemEnabled), Is.EqualTo(1)); + Assert.That(srcAccessor.CurrentValueCount, Is.EqualTo(1), + "OkToChange and NewValue must share one computed source value per item instead of reading it twice"); + } + private sealed class CountingFieldReadWriter : FieldReadWriter { private readonly FieldReadWriter m_inner; @@ -1741,9 +1805,17 @@ internal CountingFieldReadWriter(FieldReadWriter inner) } internal int SetNewValueCount { get; private set; } + internal int CurrentValueCount { get; private set; } + + internal void ResetCounts() + { + SetNewValueCount = 0; + CurrentValueCount = 0; + } public override ITsString CurrentValue(int hvo) { + CurrentValueCount++; return m_inner.CurrentValue(hvo); } From 30baeb43a984ce03c9c4cf257ab4f9ca9ae6f580 Mon Sep 17 00:00:00 2001 From: John Lambert Date: Fri, 14 Aug 2026 04:06:21 -0400 Subject: [PATCH 3/3] Remove working perf-measurement log; move reasoning to PR body Docs/superpowers/plans/evidence/bulk-replacement-distillation-2026-08-13.md was a one-time investigation log, not durable guidance. Its measurements, rejected approaches, and follow-up items now live in the PR body. Co-Authored-By: Claude Sonnet 5 --- ...ulk-replacement-distillation-2026-08-13.md | 111 ------------------ 1 file changed, 111 deletions(-) delete mode 100644 Docs/superpowers/plans/evidence/bulk-replacement-distillation-2026-08-13.md diff --git a/Docs/superpowers/plans/evidence/bulk-replacement-distillation-2026-08-13.md b/Docs/superpowers/plans/evidence/bulk-replacement-distillation-2026-08-13.md deleted file mode 100644 index adf61765e6..0000000000 --- a/Docs/superpowers/plans/evidence/bulk-replacement-distillation-2026-08-13.md +++ /dev/null @@ -1,111 +0,0 @@ -# Bulk replacement optimization evidence - -## Retained scope - -The focused implementation retains four changes: - -- Avoid normalization work when a replacement result is already NFD. -- Compute each preview value once and reuse it for the enabled result. -- Expose one-session rich-string replacement through the optional `IVwPattern2` - capability without changing the `IVwPattern` ABI or coclass default. -- Use the capability when available and preserve the repeated-`FindIn` fallback. - -The correctness corpus covers literal and regular-expression replacement, -captures, zero-width matches, whole-word and case/diacritic options, Unicode -normalization and scripts, ICU tailoring, rich-text properties, object -replacement characters, capability fallback, failures, and terminal state. - -## Single-pass preview measurement - -This is an incremental comparison of the single-pass preview change, not a -branch-point-to-final result. Both builds used Debug x64, 50,000 real entries, -100 percent matching CitationForm values with one match each, one warmup, and -five timed preview repetitions per process. Process order was B-C-B-C. - -- Baseline process 1: 73.973706, 87.429190, 85.850784, 89.671968, - 93.842072 microseconds/entry; median 87.429190. -- Candidate process 1: 53.317292, 49.020970, 49.454658, 47.650816, - 54.720194; median 49.454658. -- Baseline process 2: 68.099794, 68.588244, 70.913828, 70.782702, - 69.719412; median 69.719412. -- Candidate process 2: 48.884480, 49.327828, 58.699344, 78.705420, - 90.564270; median 58.699344. -- Pooled medians: baseline 72.443767, candidate 51.385975 - microseconds/entry; 29.068 percent reduction and 1.409796x throughput. - -The second candidate process was retried unchanged after a transient test -discovery failure produced no timing result. - -## ReplaceAllIn measurement - -This is an incremental comparison of `ReplaceAllIn`, not a cumulative branch -result. Both trees used `9307443e494d23066d9d40bc41c4d4b30944b972`. -The baseline changed only the managed capability assignment from the -`IVwPattern2` cast to `null`; native and search code was identical. Both used -`build.ps1 -BuildTests -Serial`, Debug x64, and the same temporary harness -(SHA-256 `52AB194E89486BFB94722A444B077B894B84128B46F8F0CF7EDD62DAE48D5481`). - -Each fresh process used 50,000 real entries. Matching values were -`old old old old-######` (22 UTF-16 units and four matches); unmatched values -were `keep keep keep-######` (21 units). Each fresh process had one warmup and -five timed apply repetitions. Reset, garbage collection, validation, and undo -were outside the stopwatch. There was no cache prepass. All comparisons -reported zero mismatches. - -### 25 percent matching - -- Baseline: 109.070736, 108.331108, 108.476428, 109.597136, 109.750080; - median 109.070736 microseconds/entry. -- Candidate: 80.400070, 86.023792, 81.024132, 83.734014, 100.501314; - median 83.734014. -- Incremental result: 23.23 percent reduction and 1.303x throughput. - -### 50 percent matching - -- Baseline process 1: 235.016240, 278.735516, 215.068208, 291.738206, - 245.199594; median 245.199594. -- Candidate process 1: 160.970294, 161.559868, 161.132232, 161.602208, - 178.361492; median 161.559868. -- Baseline process 2: 220.388066, 222.015458, 215.116946, 233.213528, - 239.261800; median 222.015458. -- Candidate process 2: 172.943354, 173.446744, 170.288498, 194.350902, - 182.141472; median 173.446744. -- Pooled medians: baseline 234.114884, candidate 171.615926; 26.69 percent - reduction and 1.364x throughput. The two paired reductions were 34.11 and - 21.88 percent. - -### 100 percent matching - -- Baseline process 1: 468.296628, 437.760106, 437.642286, 586.596700, - 518.897366; median 468.296628. -- Candidate process 1: 316.618992, 314.000950, 337.447102, 317.614622, - 315.852194; median 316.618992. -- Baseline process 2: 421.689880, 425.018894, 428.718602, 444.715500, - 429.318474; median 428.718602. -- Candidate process 2: 325.698984, 335.498302, 325.648910, 364.891896, - 362.333156; median 335.498302. -- Pooled medians: baseline 448.507615, candidate 326.058643; 27.30 percent - reduction and 1.376x throughput. The two paired reductions were 32.39 and - 21.75 percent. - -Debug-process variance was material, especially in the first baseline -process. The raw sample bands stayed separated, and each independent 50 and -100 percent pair exceeded the 15 percent acceptance gate. These measurements -cover the tested short CitationForm values only. - -## Rejected and deferred work - -- A printable-ASCII `memcmp` shortcut was reverted because ICU collation can - equate strings that differ in punctuation and other non-ordinal ways. -- A managed ordinal negative precheck was exactly reverted because it did not - produce sufficient benefit. -- A one-character collation-ignorable cache and early extension exit were - reverted. Its answer survived a locale change and produced an incorrect - match span; the single-character premise also lacked an ICU contract for - contextual collation. -- The virtual table and budgeted coordinator were test-owned prototypes with - no shipping activation path. They and their acceptance and attribution - harnesses were discarded. -- Homograph renumber batching was deferred after characterization only. It - needs separate cross-repository design and measurements for undo, redo, - notifications, cache membership, and group numbering.