Files
PowerToys/src/modules/powerrename/lib/PowerRenameRegEx.cpp

694 lines
21 KiB
C++
Raw Normal View History

#include "pch.h"
#include "PowerRenameRegEx.h"
#include "Enumerating.h"
PowerRename: Add Lookbehind (#7571) * Add boost-regex library * If enabled use boost lib for regex Add property `_useBoostLib` to `CPowerRenameRegEx`. If enabled for replacements with regular expressions the Boost Library is used instead of the Standard Library. * Extend signatures to create RegEx with Boost Extend create and constructor singatures of `CPowerRenameRegEx` with an option to enable (or disabled, which is default) the Boost Library. * Verify Lookbehind fails with STD library To verify that the boost library is disabled as expected, check if a lookbehind fails. * Add Unit tests for RegEx with Boost Add unit tests to verify regex replacement with Boost Library. They are copied and adapted from the Standard Library tests. * Improve verify capturing groups test with Boost It is possible to use a capturing group followed by numbers as replacement if the group number is enclosed in curly braces. Added test cases based on the Standard Library tests. * Add useBoostLib to settings interface * Get library option from settings object * Reduce signatures of RegEx by "useBoost" Remove the parameter added in 19105cf, as it became obsolete. * Settings: Read useBoostLib from JSON file * Add UseBoostLib Option to UI * Boost Lib label states the regex syntax difference * Fix Regex with Boost Lib tests - Do not load settings another time in CPowerRenameRegEx ctor - Set flag correctly in standard library regex tests * Add "lookbehind" to dictionary * change Library to lowercase, and also add a comment As suggested by @enricogior. Co-authored-by: Enrico Giordani <enricogior@users.noreply.github.com> * Change Library to lowercase and add a comment As suggested by @enricogior. Co-authored-by: Enrico Giordani <enricogior@users.noreply.github.com>
2020-11-09 19:13:43 +01:00
#include "Settings.h"
#include <regex>
#include <string>
#include <algorithm>
PowerRename: Add Lookbehind (#7571) * Add boost-regex library * If enabled use boost lib for regex Add property `_useBoostLib` to `CPowerRenameRegEx`. If enabled for replacements with regular expressions the Boost Library is used instead of the Standard Library. * Extend signatures to create RegEx with Boost Extend create and constructor singatures of `CPowerRenameRegEx` with an option to enable (or disabled, which is default) the Boost Library. * Verify Lookbehind fails with STD library To verify that the boost library is disabled as expected, check if a lookbehind fails. * Add Unit tests for RegEx with Boost Add unit tests to verify regex replacement with Boost Library. They are copied and adapted from the Standard Library tests. * Improve verify capturing groups test with Boost It is possible to use a capturing group followed by numbers as replacement if the group number is enclosed in curly braces. Added test cases based on the Standard Library tests. * Add useBoostLib to settings interface * Get library option from settings object * Reduce signatures of RegEx by "useBoost" Remove the parameter added in 19105cf, as it became obsolete. * Settings: Read useBoostLib from JSON file * Add UseBoostLib Option to UI * Boost Lib label states the regex syntax difference * Fix Regex with Boost Lib tests - Do not load settings another time in CPowerRenameRegEx ctor - Set flag correctly in standard library regex tests * Add "lookbehind" to dictionary * change Library to lowercase, and also add a comment As suggested by @enricogior. Co-authored-by: Enrico Giordani <enricogior@users.noreply.github.com> * Change Library to lowercase and add a comment As suggested by @enricogior. Co-authored-by: Enrico Giordani <enricogior@users.noreply.github.com>
2020-11-09 19:13:43 +01:00
#include <boost/regex.hpp>
#include <helpers.h>
using std::conditional_t;
using std::regex_error;
IFACEMETHODIMP_(ULONG)
CPowerRenameRegEx::AddRef()
{
return InterlockedIncrement(&m_refCount);
}
IFACEMETHODIMP_(ULONG)
CPowerRenameRegEx::Release()
{
long refCount = InterlockedDecrement(&m_refCount);
if (refCount == 0)
{
delete this;
}
return refCount;
}
IFACEMETHODIMP CPowerRenameRegEx::QueryInterface(_In_ REFIID riid, _Outptr_ void** ppv)
{
static const QITAB qit[] = {
QITABENT(CPowerRenameRegEx, IPowerRenameRegEx),
{ 0 }
};
return QISearch(this, qit, riid, ppv);
}
IFACEMETHODIMP CPowerRenameRegEx::Advise(_In_ IPowerRenameRegExEvents* regExEvents, _Out_ DWORD* cookie)
{
CSRWExclusiveAutoLock lock(&m_lockEvents);
m_cookie++;
RENAME_REGEX_EVENT srre;
srre.cookie = m_cookie;
srre.pEvents = regExEvents;
regExEvents->AddRef();
m_renameRegExEvents.push_back(srre);
*cookie = m_cookie;
return S_OK;
}
IFACEMETHODIMP CPowerRenameRegEx::UnAdvise(_In_ DWORD cookie)
{
HRESULT hr = E_FAIL;
CSRWExclusiveAutoLock lock(&m_lockEvents);
for (std::vector<RENAME_REGEX_EVENT>::iterator it = m_renameRegExEvents.begin(); it != m_renameRegExEvents.end(); ++it)
{
if (it->cookie == cookie)
{
hr = S_OK;
it->cookie = 0;
if (it->pEvents)
{
it->pEvents->Release();
it->pEvents = nullptr;
}
break;
}
}
return hr;
}
IFACEMETHODIMP CPowerRenameRegEx::GetSearchTerm(_Outptr_ PWSTR* searchTerm)
{
*searchTerm = nullptr;
HRESULT hr = S_OK;
if (m_searchTerm)
{
CSRWSharedAutoLock lock(&m_lock);
hr = SHStrDup(m_searchTerm, searchTerm);
}
return hr;
}
IFACEMETHODIMP CPowerRenameRegEx::PutSearchTerm(_In_ PCWSTR searchTerm, bool forceRenaming)
{
bool changed = false || forceRenaming;
HRESULT hr = S_OK;
if (searchTerm)
{
CSRWExclusiveAutoLock lock(&m_lock);
if (m_searchTerm == nullptr || lstrcmp(searchTerm, m_searchTerm) != 0)
{
changed = true;
CoTaskMemFree(m_searchTerm);
if (lstrcmp(searchTerm, L"") == 0)
{
m_searchTerm = NULL;
}
else
{
hr = SHStrDup(searchTerm, &m_searchTerm);
}
}
}
if (SUCCEEDED(hr) && changed)
{
_OnSearchTermChanged();
}
return hr;
}
IFACEMETHODIMP CPowerRenameRegEx::GetReplaceTerm(_Outptr_ PWSTR* replaceTerm)
{
*replaceTerm = nullptr;
HRESULT hr = S_OK;
if (m_replaceTerm)
{
CSRWSharedAutoLock lock(&m_lock);
hr = SHStrDup(m_replaceTerm, replaceTerm);
}
return hr;
}
HRESULT CPowerRenameRegEx::_OnEnumerateOrRandomizeItemsChanged()
{
m_enumerators.clear();
m_randomizer.clear();
if (m_flags & RandomizeItems)
{
const auto options = parseRandomizerOptions(m_RawReplaceTerm);
for (const auto& option : options)
{
m_randomizer.emplace_back(option);
}
}
if (m_flags & EnumerateItems)
{
const auto options = parseEnumOptions(m_RawReplaceTerm);
for (const auto& option : options)
{
if (m_randomizer.end() ==
std::find_if(
m_randomizer.begin(),
m_randomizer.end(),
[PowerRename] Fix issue where counter does not update if the filename and replacement result matched (#42006) This PR fixes an issue which has been reported a number of times under slightly different guises. The bug manifests as a counter "stall" or "skip" under certain circumstances, not advancing the counter if the result of the rename operation happens to match the original filename. <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request This issue occurs if all the following are true: - Enumeration features are enabled - Regular expression matching is enabled - The replacement string includes a counter, for example `${}` or `${start=1}` or `${start=10,increment=10}` etc. - There are one or more original filenames which coincide with the result of the renaming operations Previously, the counter was not updated when the renaming operation result was the same as the original filename. For example, here the first rename result matches the original filename and the counter remains at `1` for the second file, whereas it should be `2`: <img width="1002" height="759" alt="image" src="https://github.com/user-attachments/assets/2766f448-adc3-4fe7-9c13-f4c5505ae1d9" /> This fix increments the counter irrespective of these coincidences. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #41950, #31950, #33884 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments **Before discussing the detail of the fix, I'd like to acknowledge the incredible coincidence that it resolves two issues (31950 and 41950) _which are exactly 10,000 issues (and 18 months) apart_.** Now, back to the issue at hand, if you'll forgive the pun... The original flawed code is here: ```cpp bool replacedSomething = false; if (m_flags & UseRegularExpressions) { replaceTerm = regex_replace(replaceTerm, zeroGroupRegex, L"$1$$$0"); replaceTerm = regex_replace(replaceTerm, otherGroupsRegex, L"$1$0$4"); res = RegexReplaceDispatch[_useBoostLib](source, m_searchTerm, replaceTerm, m_flags & MatchAllOccurrences, !(m_flags & CaseSensitive)); replacedSomething = originalSource != res; } ``` Here `replacedSomething` controls whether the counter variable is incremented later in the code. The problem lies in the assumption that only a replacement result which differs from the original string implies a match. This is incorrect, as a successful match and replace operation can still produce a result which coincides with `originalSource` (the source filename), i.e. `originalSource == res`. The solution is to separate the regex matching from the replacement operation, and to advance the counter when the match is successful irrespective of whether the result is the same as the filename. This is what the fix does: ```cpp bool shouldIncrementCounter = false; if (m_flags & UseRegularExpressions) { replaceTerm = regex_replace(replaceTerm, zeroGroupRegex, L"$1$$$0"); replaceTerm = regex_replace(replaceTerm, otherGroupsRegex, L"$1$0$4"); res = RegexReplaceDispatch[_useBoostLib](source, m_searchTerm, replaceTerm, m_flags & MatchAllOccurrences, !(m_flags & CaseSensitive)); // Use regex search to determine if a match exists. This is the basis for incrementing // the counter. if (_useBoostLib) { boost::wregex pattern(m_searchTerm, boost::wregex::ECMAScript | (!(m_flags & CaseSensitive) ? boost::wregex::icase : boost::wregex::normal)); shouldIncrementCounter = boost::regex_search(sourceToUse, pattern); } else { auto regexFlags = std::wregex::ECMAScript; if (!(m_flags & CaseSensitive)) { regexFlags |= std::wregex::icase; } std::wregex pattern(m_searchTerm, regexFlags); shouldIncrementCounter = std::regex_search(sourceToUse, pattern); } } ``` The `regex_search()` call on both the boost and std paths tests whether the regex pattern can be found in the original filename (`sourceToUse`). `shouldIncrementCounter` tracks whether the counter will be incremented later, renamed from `replacedSomething` to reflect the change in behaviour. ## Validation Steps Performed The fix includes an additional unit test for both the std and boost regex paths. Without the fix, the test fails: <img width="1509" height="506" alt="Screenshot 2025-09-25 063611" src="https://github.com/user-attachments/assets/14dbf817-b1d3-456d-80f2-abcd28266b8d" /> and with the fix, all tests pass: <img width="897" height="492" alt="Screenshot 2025-09-25 063749" src="https://github.com/user-attachments/assets/9a587495-d54c-47d3-bc55-ccc64a805a88" />
2025-09-29 03:05:21 +01:00
[option](const Randomizer& r) -> bool { return r.options.replaceStrSpan.offset == option.replaceStrSpan.offset; }))
{
// Only add as enumerator if we didn't find a randomizer already at this offset.
// Every randomizer will also be a valid enumerator according to the definition of enumerators, which allows any string to mean the default enumerator, so it should be interpreted that the user wanted a randomizer if both were found at the same offset of the replace string.
m_enumerators.emplace_back(option);
}
}
}
m_replaceWithRandomizerOffsets.clear();
m_replaceWithEnumeratorOffsets.clear();
int32_t offset = 0;
int ei = 0; // Enumerators index
int ri = 0; // Randomizer index
std::wstring replaceWith{ m_RawReplaceTerm };
// Remove counter expressions and calculate their offsets in replaceWith string.
if ((m_flags & EnumerateItems) && (m_flags & RandomizeItems))
{
// Both flags are on, we need to merge which ones should be applied.
while ((ei < m_enumerators.size()) && (ri < m_randomizer.size()))
{
const auto& e = m_enumerators[ei];
const auto& r = m_randomizer[ri];
if (e.replaceStrSpan.offset < r.options.replaceStrSpan.offset)
{
// if the enumerator is next in line, remove counter expression and calculate offset with it.
replaceWith.erase(e.replaceStrSpan.offset + offset, e.replaceStrSpan.length);
m_replaceWithEnumeratorOffsets.push_back(offset);
offset -= static_cast<int32_t>(e.replaceStrSpan.length);
ei++;
}
else
{
// if the randomizer is next in line, remove randomizer expression and calculate offset with it.
replaceWith.erase(r.options.replaceStrSpan.offset + offset, r.options.replaceStrSpan.length);
m_replaceWithRandomizerOffsets.push_back(offset);
offset -= static_cast<int32_t>(r.options.replaceStrSpan.length);
ri++;
}
}
}
if (m_flags & EnumerateItems)
{
// Continue with all remaining enumerators
while (ei < m_enumerators.size())
{
const auto& e = m_enumerators[ei];
replaceWith.erase(e.replaceStrSpan.offset + offset, e.replaceStrSpan.length);
m_replaceWithEnumeratorOffsets.push_back(offset);
offset -= static_cast<int32_t>(e.replaceStrSpan.length);
ei++;
}
}
if (m_flags & RandomizeItems)
{
// Continue with all remaining randomizer instances
while (ri < m_randomizer.size())
{
const auto& r = m_randomizer[ri];
replaceWith.erase(r.options.replaceStrSpan.offset + offset, r.options.replaceStrSpan.length);
m_replaceWithRandomizerOffsets.push_back(offset);
offset -= static_cast<int32_t>(r.options.replaceStrSpan.length);
ri++;
}
}
return SHStrDup(replaceWith.data(), &m_replaceTerm);
}
IFACEMETHODIMP CPowerRenameRegEx::PutReplaceTerm(_In_ PCWSTR replaceTerm, bool forceRenaming)
{
bool changed = false || forceRenaming;
HRESULT hr = S_OK;
if (replaceTerm)
{
CSRWExclusiveAutoLock lock(&m_lock);
if (m_replaceTerm == nullptr || lstrcmp(replaceTerm, m_RawReplaceTerm.c_str()) != 0)
{
changed = true;
CoTaskMemFree(m_replaceTerm);
m_RawReplaceTerm = replaceTerm;
if ((m_flags & RandomizeItems) || (m_flags & EnumerateItems))
hr = _OnEnumerateOrRandomizeItemsChanged();
else
hr = SHStrDup(replaceTerm, &m_replaceTerm);
}
}
if (SUCCEEDED(hr) && changed)
{
_OnReplaceTermChanged();
}
return hr;
}
IFACEMETHODIMP CPowerRenameRegEx::GetFlags(_Out_ DWORD* flags)
{
*flags = m_flags;
return S_OK;
}
IFACEMETHODIMP CPowerRenameRegEx::PutFlags(_In_ DWORD flags)
{
if (m_flags != flags)
{
const bool newEnumerate = flags & EnumerateItems;
const bool newRandomizer = flags & RandomizeItems;
const bool refreshReplaceTerm =
(!!(m_flags & EnumerateItems) != newEnumerate) ||
(!!(m_flags & RandomizeItems) != newRandomizer);
m_flags = flags;
if (refreshReplaceTerm)
{
CSRWExclusiveAutoLock lock(&m_lock);
if (newEnumerate || newRandomizer)
{
_OnEnumerateOrRandomizeItemsChanged();
}
else
{
CoTaskMemFree(m_replaceTerm);
SHStrDup(m_RawReplaceTerm.c_str(), &m_replaceTerm);
}
}
_OnFlagsChanged();
}
return S_OK;
}
IFACEMETHODIMP CPowerRenameRegEx::PutFileTime(_In_ SYSTEMTIME fileTime)
{
union timeunion
{
FILETIME fileTime;
ULARGE_INTEGER ul;
};
timeunion ft1;
timeunion ft2;
SystemTimeToFileTime(&m_fileTime, &ft1.fileTime);
SystemTimeToFileTime(&fileTime, &ft2.fileTime);
if (ft2.ul.QuadPart != ft1.ul.QuadPart)
{
m_fileTime = fileTime;
m_useFileTime = true;
_OnFileTimeChanged();
}
return S_OK;
}
IFACEMETHODIMP CPowerRenameRegEx::ResetFileTime()
{
SYSTEMTIME ZERO = { 0 };
m_fileTime = ZERO;
m_useFileTime = false;
_OnFileTimeChanged();
return S_OK;
}
[PowerRename] Support using photo metadata to replace in the PowerRename (#41728) <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request 1. Introduce WIC for power rename and add new class WICMetadataExtractor to use WIC to extract metadata. 2. Add some patterns for metadata extract. 3. Support XMP and EXIF metadata extract. 4. Add test data for xmp and exif extractor 5. Add attribution for the test data uploader. UI: <img width="2052" height="1415" alt="image" src="https://github.com/user-attachments/assets/9051b12e-4e66-4fdc-a4d4-3bada661c235" /> <img width="284" height="170" alt="image" src="https://github.com/user-attachments/assets/2fd67193-77a7-48f0-a5ac-08a69fe64e55" /> <img width="715" height="1160" alt="image" src="https://github.com/user-attachments/assets/5fa68a8c-d129-44dd-b747-099dfbcded12" /> demo: https://github.com/user-attachments/assets/e90bc206-62e5-4101-ada2-3187ee7e2039 <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #5612 - [x] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [x] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed --------- Co-authored-by: Yu Leng <yuleng@microsoft.com>
2025-11-04 09:27:16 +08:00
IFACEMETHODIMP CPowerRenameRegEx::PutMetadataPatterns(_In_ const PowerRenameLib::MetadataPatternMap& patterns)
{
m_metadataPatterns = patterns;
m_useMetadata = true;
_OnMetadataChanged();
return S_OK;
}
IFACEMETHODIMP CPowerRenameRegEx::ResetMetadata()
{
m_metadataPatterns.clear();
m_useMetadata = false;
_OnMetadataChanged();
return S_OK;
}
HRESULT CPowerRenameRegEx::s_CreateInstance(_Outptr_ IPowerRenameRegEx** renameRegEx)
{
*renameRegEx = nullptr;
CPowerRenameRegEx* newRenameRegEx = new CPowerRenameRegEx();
HRESULT hr = E_OUTOFMEMORY;
if (newRenameRegEx)
{
hr = newRenameRegEx->QueryInterface(IID_PPV_ARGS(renameRegEx));
newRenameRegEx->Release();
}
return hr;
}
CPowerRenameRegEx::CPowerRenameRegEx() :
m_refCount(1)
{
// Init to empty strings
SHStrDup(L"", &m_searchTerm);
SHStrDup(L"", &m_replaceTerm);
PowerRename: Add Lookbehind (#7571) * Add boost-regex library * If enabled use boost lib for regex Add property `_useBoostLib` to `CPowerRenameRegEx`. If enabled for replacements with regular expressions the Boost Library is used instead of the Standard Library. * Extend signatures to create RegEx with Boost Extend create and constructor singatures of `CPowerRenameRegEx` with an option to enable (or disabled, which is default) the Boost Library. * Verify Lookbehind fails with STD library To verify that the boost library is disabled as expected, check if a lookbehind fails. * Add Unit tests for RegEx with Boost Add unit tests to verify regex replacement with Boost Library. They are copied and adapted from the Standard Library tests. * Improve verify capturing groups test with Boost It is possible to use a capturing group followed by numbers as replacement if the group number is enclosed in curly braces. Added test cases based on the Standard Library tests. * Add useBoostLib to settings interface * Get library option from settings object * Reduce signatures of RegEx by "useBoost" Remove the parameter added in 19105cf, as it became obsolete. * Settings: Read useBoostLib from JSON file * Add UseBoostLib Option to UI * Boost Lib label states the regex syntax difference * Fix Regex with Boost Lib tests - Do not load settings another time in CPowerRenameRegEx ctor - Set flag correctly in standard library regex tests * Add "lookbehind" to dictionary * change Library to lowercase, and also add a comment As suggested by @enricogior. Co-authored-by: Enrico Giordani <enricogior@users.noreply.github.com> * Change Library to lowercase and add a comment As suggested by @enricogior. Co-authored-by: Enrico Giordani <enricogior@users.noreply.github.com>
2020-11-09 19:13:43 +01:00
_useBoostLib = CSettingsInstance().GetUseBoostLib();
}
CPowerRenameRegEx::~CPowerRenameRegEx()
{
CoTaskMemFree(m_searchTerm);
CoTaskMemFree(m_replaceTerm);
}
template<bool Std, class Regex = conditional_t<Std, std::wregex, boost::wregex>, class Options = decltype(Regex::icase)>
static std::wstring RegexReplaceEx(const std::wstring& source, const std::wstring& searchTerm, const std::wstring& replaceTerm, const bool matchAll, const bool caseInsensitive)
{
Regex pattern(searchTerm, Options::ECMAScript | (caseInsensitive ? Options::icase : Options{}));
using Flags = conditional_t<Std, std::regex_constants::match_flag_type, boost::regex_constants::match_flags>;
const auto flags = matchAll ? Flags::match_default : Flags::format_first_only;
return regex_replace(source, pattern, replaceTerm, flags);
}
static constexpr std::array RegexReplaceDispatch = { RegexReplaceEx<true>, RegexReplaceEx<false> };
HRESULT CPowerRenameRegEx::Replace(_In_ PCWSTR source, _Outptr_ PWSTR* result, unsigned long& enumIndex)
{
*result = nullptr;
CSRWSharedAutoLock lock(&m_lock);
HRESULT hr = S_OK;
if (!(m_searchTerm && wcslen(m_searchTerm) > 0 && source && wcslen(source) > 0))
{
return hr;
}
std::wstring res = source;
try
{
// TODO: creating the regex could be costly. May want to cache this.
wchar_t newReplaceTerm[MAX_PATH] = { 0 };
bool fileTimeErrorOccurred = false;
[PowerRename] Support using photo metadata to replace in the PowerRename (#41728) <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request 1. Introduce WIC for power rename and add new class WICMetadataExtractor to use WIC to extract metadata. 2. Add some patterns for metadata extract. 3. Support XMP and EXIF metadata extract. 4. Add test data for xmp and exif extractor 5. Add attribution for the test data uploader. UI: <img width="2052" height="1415" alt="image" src="https://github.com/user-attachments/assets/9051b12e-4e66-4fdc-a4d4-3bada661c235" /> <img width="284" height="170" alt="image" src="https://github.com/user-attachments/assets/2fd67193-77a7-48f0-a5ac-08a69fe64e55" /> <img width="715" height="1160" alt="image" src="https://github.com/user-attachments/assets/5fa68a8c-d129-44dd-b747-099dfbcded12" /> demo: https://github.com/user-attachments/assets/e90bc206-62e5-4101-ada2-3187ee7e2039 <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #5612 - [x] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [x] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed --------- Co-authored-by: Yu Leng <yuleng@microsoft.com>
2025-11-04 09:27:16 +08:00
bool metadataErrorOccurred = false;
bool appliedTemplateTransform = false;
std::wstring replaceTemplate;
if (m_replaceTerm)
{
replaceTemplate = m_replaceTerm;
}
if (m_useFileTime)
{
[PowerRename] Support using photo metadata to replace in the PowerRename (#41728) <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request 1. Introduce WIC for power rename and add new class WICMetadataExtractor to use WIC to extract metadata. 2. Add some patterns for metadata extract. 3. Support XMP and EXIF metadata extract. 4. Add test data for xmp and exif extractor 5. Add attribution for the test data uploader. UI: <img width="2052" height="1415" alt="image" src="https://github.com/user-attachments/assets/9051b12e-4e66-4fdc-a4d4-3bada661c235" /> <img width="284" height="170" alt="image" src="https://github.com/user-attachments/assets/2fd67193-77a7-48f0-a5ac-08a69fe64e55" /> <img width="715" height="1160" alt="image" src="https://github.com/user-attachments/assets/5fa68a8c-d129-44dd-b747-099dfbcded12" /> demo: https://github.com/user-attachments/assets/e90bc206-62e5-4101-ada2-3187ee7e2039 <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #5612 - [x] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [x] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed --------- Co-authored-by: Yu Leng <yuleng@microsoft.com>
2025-11-04 09:27:16 +08:00
if (FAILED(GetDatedFileName(newReplaceTerm, ARRAYSIZE(newReplaceTerm), replaceTemplate.c_str(), m_fileTime)))
{
fileTimeErrorOccurred = true;
[PowerRename] Support using photo metadata to replace in the PowerRename (#41728) <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request 1. Introduce WIC for power rename and add new class WICMetadataExtractor to use WIC to extract metadata. 2. Add some patterns for metadata extract. 3. Support XMP and EXIF metadata extract. 4. Add test data for xmp and exif extractor 5. Add attribution for the test data uploader. UI: <img width="2052" height="1415" alt="image" src="https://github.com/user-attachments/assets/9051b12e-4e66-4fdc-a4d4-3bada661c235" /> <img width="284" height="170" alt="image" src="https://github.com/user-attachments/assets/2fd67193-77a7-48f0-a5ac-08a69fe64e55" /> <img width="715" height="1160" alt="image" src="https://github.com/user-attachments/assets/5fa68a8c-d129-44dd-b747-099dfbcded12" /> demo: https://github.com/user-attachments/assets/e90bc206-62e5-4101-ada2-3187ee7e2039 <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #5612 - [x] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [x] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed --------- Co-authored-by: Yu Leng <yuleng@microsoft.com>
2025-11-04 09:27:16 +08:00
}
else
{
replaceTemplate.assign(newReplaceTerm);
appliedTemplateTransform = true;
}
}
if (m_useMetadata)
{
if (FAILED(GetMetadataFileName(newReplaceTerm, ARRAYSIZE(newReplaceTerm), replaceTemplate.c_str(), m_metadataPatterns)))
{
metadataErrorOccurred = true;
}
else
{
replaceTemplate.assign(newReplaceTerm);
appliedTemplateTransform = true;
}
}
std::wstring sourceToUse;
sourceToUse.reserve(MAX_PATH);
sourceToUse = source;
std::wstring searchTerm(m_searchTerm);
std::wstring replaceTerm;
[PowerRename] Support using photo metadata to replace in the PowerRename (#41728) <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request 1. Introduce WIC for power rename and add new class WICMetadataExtractor to use WIC to extract metadata. 2. Add some patterns for metadata extract. 3. Support XMP and EXIF metadata extract. 4. Add test data for xmp and exif extractor 5. Add attribution for the test data uploader. UI: <img width="2052" height="1415" alt="image" src="https://github.com/user-attachments/assets/9051b12e-4e66-4fdc-a4d4-3bada661c235" /> <img width="284" height="170" alt="image" src="https://github.com/user-attachments/assets/2fd67193-77a7-48f0-a5ac-08a69fe64e55" /> <img width="715" height="1160" alt="image" src="https://github.com/user-attachments/assets/5fa68a8c-d129-44dd-b747-099dfbcded12" /> demo: https://github.com/user-attachments/assets/e90bc206-62e5-4101-ada2-3187ee7e2039 <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #5612 - [x] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [x] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed --------- Co-authored-by: Yu Leng <yuleng@microsoft.com>
2025-11-04 09:27:16 +08:00
if (appliedTemplateTransform)
{
[PowerRename] Support using photo metadata to replace in the PowerRename (#41728) <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request 1. Introduce WIC for power rename and add new class WICMetadataExtractor to use WIC to extract metadata. 2. Add some patterns for metadata extract. 3. Support XMP and EXIF metadata extract. 4. Add test data for xmp and exif extractor 5. Add attribution for the test data uploader. UI: <img width="2052" height="1415" alt="image" src="https://github.com/user-attachments/assets/9051b12e-4e66-4fdc-a4d4-3bada661c235" /> <img width="284" height="170" alt="image" src="https://github.com/user-attachments/assets/2fd67193-77a7-48f0-a5ac-08a69fe64e55" /> <img width="715" height="1160" alt="image" src="https://github.com/user-attachments/assets/5fa68a8c-d129-44dd-b747-099dfbcded12" /> demo: https://github.com/user-attachments/assets/e90bc206-62e5-4101-ada2-3187ee7e2039 <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #5612 - [x] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [x] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed --------- Co-authored-by: Yu Leng <yuleng@microsoft.com>
2025-11-04 09:27:16 +08:00
replaceTerm = replaceTemplate;
}
else if (m_replaceTerm)
{
replaceTerm = m_replaceTerm;
}
static const std::wregex zeroGroupRegex(L"(([^\\$]|^)(\\$\\$)*)\\$[0]");
static const std::wregex otherGroupsRegex(L"(([^\\$]|^)(\\$\\$)*)\\$([1-9])");
if ((m_flags & EnumerateItems) || (m_flags & RandomizeItems))
{
int ei = 0; // Enumerators index
int ri = 0; // Randomizer index
std::array<wchar_t, MAX_PATH> buffer;
int32_t offset = 0;
if ((m_flags & EnumerateItems) && (m_flags & RandomizeItems))
{
// Both flags are on, we need to merge which ones should be applied.
while ((ei < m_enumerators.size()) && (ri < m_randomizer.size()))
{
const auto& e = m_enumerators[ei];
const auto& r = m_randomizer[ri];
if (e.replaceStrSpan.offset < r.options.replaceStrSpan.offset)
{
// if the enumerator is next in line, apply it.
const auto replacementLength = static_cast<int32_t>(e.printTo(buffer.data(), buffer.size(), enumIndex));
replaceTerm.insert(e.replaceStrSpan.offset + offset + m_replaceWithEnumeratorOffsets[ei], buffer.data());
offset += replacementLength;
ei++;
}
else
{
// if the randomizer is next in line, apply it.
std::string randomValue = r.randomize();
std::wstring wRandomValue(randomValue.begin(), randomValue.end());
replaceTerm.insert(r.options.replaceStrSpan.offset + offset + m_replaceWithRandomizerOffsets[ri], wRandomValue);
offset += static_cast<int32_t>(wRandomValue.length());
if (e.replaceStrSpan.offset == r.options.replaceStrSpan.offset)
{
// In theory, this shouldn't happen here as we were guarding against it when filling the randomizer and enumerator structures, but it's still here as a fail safe.
// Every randomizer will also be a valid enumerator according to the definition of enumerators, which allow any string to mean the default enumerator, so it should be interpreted that the user wanted a randomizer if both were found at the same offset of the replace string.
ei++;
}
ri++;
}
}
}
if (m_flags & EnumerateItems)
{
// Replace all remaining enumerators
while (ei < m_enumerators.size())
{
const auto& e = m_enumerators[ei];
const auto replacementLength = static_cast<int32_t>(e.printTo(buffer.data(), buffer.size(), enumIndex));
replaceTerm.insert(e.replaceStrSpan.offset + offset + m_replaceWithEnumeratorOffsets[ei], buffer.data());
offset += replacementLength;
ei++;
}
}
if (m_flags & RandomizeItems)
{
// Replace all remaining randomizer instances
while (ri < m_randomizer.size())
{
const auto& r = m_randomizer[ri];
std::string randomValue = r.randomize();
std::wstring wRandomValue(randomValue.begin(), randomValue.end());
replaceTerm.insert(r.options.replaceStrSpan.offset + offset + m_replaceWithRandomizerOffsets[ri], wRandomValue);
offset += static_cast<int32_t>(wRandomValue.length());
ri++;
}
}
}
[PowerRename] Fix issue where counter does not update if the filename and replacement result matched (#42006) This PR fixes an issue which has been reported a number of times under slightly different guises. The bug manifests as a counter "stall" or "skip" under certain circumstances, not advancing the counter if the result of the rename operation happens to match the original filename. <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request This issue occurs if all the following are true: - Enumeration features are enabled - Regular expression matching is enabled - The replacement string includes a counter, for example `${}` or `${start=1}` or `${start=10,increment=10}` etc. - There are one or more original filenames which coincide with the result of the renaming operations Previously, the counter was not updated when the renaming operation result was the same as the original filename. For example, here the first rename result matches the original filename and the counter remains at `1` for the second file, whereas it should be `2`: <img width="1002" height="759" alt="image" src="https://github.com/user-attachments/assets/2766f448-adc3-4fe7-9c13-f4c5505ae1d9" /> This fix increments the counter irrespective of these coincidences. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #41950, #31950, #33884 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments **Before discussing the detail of the fix, I'd like to acknowledge the incredible coincidence that it resolves two issues (31950 and 41950) _which are exactly 10,000 issues (and 18 months) apart_.** Now, back to the issue at hand, if you'll forgive the pun... The original flawed code is here: ```cpp bool replacedSomething = false; if (m_flags & UseRegularExpressions) { replaceTerm = regex_replace(replaceTerm, zeroGroupRegex, L"$1$$$0"); replaceTerm = regex_replace(replaceTerm, otherGroupsRegex, L"$1$0$4"); res = RegexReplaceDispatch[_useBoostLib](source, m_searchTerm, replaceTerm, m_flags & MatchAllOccurrences, !(m_flags & CaseSensitive)); replacedSomething = originalSource != res; } ``` Here `replacedSomething` controls whether the counter variable is incremented later in the code. The problem lies in the assumption that only a replacement result which differs from the original string implies a match. This is incorrect, as a successful match and replace operation can still produce a result which coincides with `originalSource` (the source filename), i.e. `originalSource == res`. The solution is to separate the regex matching from the replacement operation, and to advance the counter when the match is successful irrespective of whether the result is the same as the filename. This is what the fix does: ```cpp bool shouldIncrementCounter = false; if (m_flags & UseRegularExpressions) { replaceTerm = regex_replace(replaceTerm, zeroGroupRegex, L"$1$$$0"); replaceTerm = regex_replace(replaceTerm, otherGroupsRegex, L"$1$0$4"); res = RegexReplaceDispatch[_useBoostLib](source, m_searchTerm, replaceTerm, m_flags & MatchAllOccurrences, !(m_flags & CaseSensitive)); // Use regex search to determine if a match exists. This is the basis for incrementing // the counter. if (_useBoostLib) { boost::wregex pattern(m_searchTerm, boost::wregex::ECMAScript | (!(m_flags & CaseSensitive) ? boost::wregex::icase : boost::wregex::normal)); shouldIncrementCounter = boost::regex_search(sourceToUse, pattern); } else { auto regexFlags = std::wregex::ECMAScript; if (!(m_flags & CaseSensitive)) { regexFlags |= std::wregex::icase; } std::wregex pattern(m_searchTerm, regexFlags); shouldIncrementCounter = std::regex_search(sourceToUse, pattern); } } ``` The `regex_search()` call on both the boost and std paths tests whether the regex pattern can be found in the original filename (`sourceToUse`). `shouldIncrementCounter` tracks whether the counter will be incremented later, renamed from `replacedSomething` to reflect the change in behaviour. ## Validation Steps Performed The fix includes an additional unit test for both the std and boost regex paths. Without the fix, the test fails: <img width="1509" height="506" alt="Screenshot 2025-09-25 063611" src="https://github.com/user-attachments/assets/14dbf817-b1d3-456d-80f2-abcd28266b8d" /> and with the fix, all tests pass: <img width="897" height="492" alt="Screenshot 2025-09-25 063749" src="https://github.com/user-attachments/assets/9a587495-d54c-47d3-bc55-ccc64a805a88" />
2025-09-29 03:05:21 +01:00
bool shouldIncrementCounter = false;
const bool isCaseInsensitive = !(m_flags & CaseSensitive);
if (m_flags & UseRegularExpressions)
{
replaceTerm = regex_replace(replaceTerm, zeroGroupRegex, L"$1$$$0");
replaceTerm = regex_replace(replaceTerm, otherGroupsRegex, L"$1$0$4");
[PowerRename] Fix issue where counter does not update if the filename and replacement result matched (#42006) This PR fixes an issue which has been reported a number of times under slightly different guises. The bug manifests as a counter "stall" or "skip" under certain circumstances, not advancing the counter if the result of the rename operation happens to match the original filename. <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request This issue occurs if all the following are true: - Enumeration features are enabled - Regular expression matching is enabled - The replacement string includes a counter, for example `${}` or `${start=1}` or `${start=10,increment=10}` etc. - There are one or more original filenames which coincide with the result of the renaming operations Previously, the counter was not updated when the renaming operation result was the same as the original filename. For example, here the first rename result matches the original filename and the counter remains at `1` for the second file, whereas it should be `2`: <img width="1002" height="759" alt="image" src="https://github.com/user-attachments/assets/2766f448-adc3-4fe7-9c13-f4c5505ae1d9" /> This fix increments the counter irrespective of these coincidences. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #41950, #31950, #33884 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments **Before discussing the detail of the fix, I'd like to acknowledge the incredible coincidence that it resolves two issues (31950 and 41950) _which are exactly 10,000 issues (and 18 months) apart_.** Now, back to the issue at hand, if you'll forgive the pun... The original flawed code is here: ```cpp bool replacedSomething = false; if (m_flags & UseRegularExpressions) { replaceTerm = regex_replace(replaceTerm, zeroGroupRegex, L"$1$$$0"); replaceTerm = regex_replace(replaceTerm, otherGroupsRegex, L"$1$0$4"); res = RegexReplaceDispatch[_useBoostLib](source, m_searchTerm, replaceTerm, m_flags & MatchAllOccurrences, !(m_flags & CaseSensitive)); replacedSomething = originalSource != res; } ``` Here `replacedSomething` controls whether the counter variable is incremented later in the code. The problem lies in the assumption that only a replacement result which differs from the original string implies a match. This is incorrect, as a successful match and replace operation can still produce a result which coincides with `originalSource` (the source filename), i.e. `originalSource == res`. The solution is to separate the regex matching from the replacement operation, and to advance the counter when the match is successful irrespective of whether the result is the same as the filename. This is what the fix does: ```cpp bool shouldIncrementCounter = false; if (m_flags & UseRegularExpressions) { replaceTerm = regex_replace(replaceTerm, zeroGroupRegex, L"$1$$$0"); replaceTerm = regex_replace(replaceTerm, otherGroupsRegex, L"$1$0$4"); res = RegexReplaceDispatch[_useBoostLib](source, m_searchTerm, replaceTerm, m_flags & MatchAllOccurrences, !(m_flags & CaseSensitive)); // Use regex search to determine if a match exists. This is the basis for incrementing // the counter. if (_useBoostLib) { boost::wregex pattern(m_searchTerm, boost::wregex::ECMAScript | (!(m_flags & CaseSensitive) ? boost::wregex::icase : boost::wregex::normal)); shouldIncrementCounter = boost::regex_search(sourceToUse, pattern); } else { auto regexFlags = std::wregex::ECMAScript; if (!(m_flags & CaseSensitive)) { regexFlags |= std::wregex::icase; } std::wregex pattern(m_searchTerm, regexFlags); shouldIncrementCounter = std::regex_search(sourceToUse, pattern); } } ``` The `regex_search()` call on both the boost and std paths tests whether the regex pattern can be found in the original filename (`sourceToUse`). `shouldIncrementCounter` tracks whether the counter will be incremented later, renamed from `replacedSomething` to reflect the change in behaviour. ## Validation Steps Performed The fix includes an additional unit test for both the std and boost regex paths. Without the fix, the test fails: <img width="1509" height="506" alt="Screenshot 2025-09-25 063611" src="https://github.com/user-attachments/assets/14dbf817-b1d3-456d-80f2-abcd28266b8d" /> and with the fix, all tests pass: <img width="897" height="492" alt="Screenshot 2025-09-25 063749" src="https://github.com/user-attachments/assets/9a587495-d54c-47d3-bc55-ccc64a805a88" />
2025-09-29 03:05:21 +01:00
res = RegexReplaceDispatch[_useBoostLib](source, m_searchTerm, replaceTerm, m_flags & MatchAllOccurrences, isCaseInsensitive);
// Use regex search to determine if a match exists. This is the basis for incrementing
// the counter.
if (_useBoostLib)
{
boost::wregex pattern(m_searchTerm, boost::wregex::ECMAScript | (isCaseInsensitive ? boost::wregex::icase : boost::wregex::normal));
shouldIncrementCounter = boost::regex_search(sourceToUse, pattern);
}
else
{
auto regexFlags = std::wregex::ECMAScript;
if (isCaseInsensitive)
{
regexFlags |= std::wregex::icase;
}
std::wregex pattern(m_searchTerm, regexFlags);
shouldIncrementCounter = std::regex_search(sourceToUse, pattern);
}
}
else
{
[PowerRename] Fix issue where counter does not update if the filename and replacement result matched (#42006) This PR fixes an issue which has been reported a number of times under slightly different guises. The bug manifests as a counter "stall" or "skip" under certain circumstances, not advancing the counter if the result of the rename operation happens to match the original filename. <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request This issue occurs if all the following are true: - Enumeration features are enabled - Regular expression matching is enabled - The replacement string includes a counter, for example `${}` or `${start=1}` or `${start=10,increment=10}` etc. - There are one or more original filenames which coincide with the result of the renaming operations Previously, the counter was not updated when the renaming operation result was the same as the original filename. For example, here the first rename result matches the original filename and the counter remains at `1` for the second file, whereas it should be `2`: <img width="1002" height="759" alt="image" src="https://github.com/user-attachments/assets/2766f448-adc3-4fe7-9c13-f4c5505ae1d9" /> This fix increments the counter irrespective of these coincidences. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #41950, #31950, #33884 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments **Before discussing the detail of the fix, I'd like to acknowledge the incredible coincidence that it resolves two issues (31950 and 41950) _which are exactly 10,000 issues (and 18 months) apart_.** Now, back to the issue at hand, if you'll forgive the pun... The original flawed code is here: ```cpp bool replacedSomething = false; if (m_flags & UseRegularExpressions) { replaceTerm = regex_replace(replaceTerm, zeroGroupRegex, L"$1$$$0"); replaceTerm = regex_replace(replaceTerm, otherGroupsRegex, L"$1$0$4"); res = RegexReplaceDispatch[_useBoostLib](source, m_searchTerm, replaceTerm, m_flags & MatchAllOccurrences, !(m_flags & CaseSensitive)); replacedSomething = originalSource != res; } ``` Here `replacedSomething` controls whether the counter variable is incremented later in the code. The problem lies in the assumption that only a replacement result which differs from the original string implies a match. This is incorrect, as a successful match and replace operation can still produce a result which coincides with `originalSource` (the source filename), i.e. `originalSource == res`. The solution is to separate the regex matching from the replacement operation, and to advance the counter when the match is successful irrespective of whether the result is the same as the filename. This is what the fix does: ```cpp bool shouldIncrementCounter = false; if (m_flags & UseRegularExpressions) { replaceTerm = regex_replace(replaceTerm, zeroGroupRegex, L"$1$$$0"); replaceTerm = regex_replace(replaceTerm, otherGroupsRegex, L"$1$0$4"); res = RegexReplaceDispatch[_useBoostLib](source, m_searchTerm, replaceTerm, m_flags & MatchAllOccurrences, !(m_flags & CaseSensitive)); // Use regex search to determine if a match exists. This is the basis for incrementing // the counter. if (_useBoostLib) { boost::wregex pattern(m_searchTerm, boost::wregex::ECMAScript | (!(m_flags & CaseSensitive) ? boost::wregex::icase : boost::wregex::normal)); shouldIncrementCounter = boost::regex_search(sourceToUse, pattern); } else { auto regexFlags = std::wregex::ECMAScript; if (!(m_flags & CaseSensitive)) { regexFlags |= std::wregex::icase; } std::wregex pattern(m_searchTerm, regexFlags); shouldIncrementCounter = std::regex_search(sourceToUse, pattern); } } ``` The `regex_search()` call on both the boost and std paths tests whether the regex pattern can be found in the original filename (`sourceToUse`). `shouldIncrementCounter` tracks whether the counter will be incremented later, renamed from `replacedSomething` to reflect the change in behaviour. ## Validation Steps Performed The fix includes an additional unit test for both the std and boost regex paths. Without the fix, the test fails: <img width="1509" height="506" alt="Screenshot 2025-09-25 063611" src="https://github.com/user-attachments/assets/14dbf817-b1d3-456d-80f2-abcd28266b8d" /> and with the fix, all tests pass: <img width="897" height="492" alt="Screenshot 2025-09-25 063749" src="https://github.com/user-attachments/assets/9a587495-d54c-47d3-bc55-ccc64a805a88" />
2025-09-29 03:05:21 +01:00
// Simple search and replace.
size_t pos = 0;
do
{
[PowerRename] Fix issue where counter does not update if the filename and replacement result matched (#42006) This PR fixes an issue which has been reported a number of times under slightly different guises. The bug manifests as a counter "stall" or "skip" under certain circumstances, not advancing the counter if the result of the rename operation happens to match the original filename. <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request This issue occurs if all the following are true: - Enumeration features are enabled - Regular expression matching is enabled - The replacement string includes a counter, for example `${}` or `${start=1}` or `${start=10,increment=10}` etc. - There are one or more original filenames which coincide with the result of the renaming operations Previously, the counter was not updated when the renaming operation result was the same as the original filename. For example, here the first rename result matches the original filename and the counter remains at `1` for the second file, whereas it should be `2`: <img width="1002" height="759" alt="image" src="https://github.com/user-attachments/assets/2766f448-adc3-4fe7-9c13-f4c5505ae1d9" /> This fix increments the counter irrespective of these coincidences. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #41950, #31950, #33884 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments **Before discussing the detail of the fix, I'd like to acknowledge the incredible coincidence that it resolves two issues (31950 and 41950) _which are exactly 10,000 issues (and 18 months) apart_.** Now, back to the issue at hand, if you'll forgive the pun... The original flawed code is here: ```cpp bool replacedSomething = false; if (m_flags & UseRegularExpressions) { replaceTerm = regex_replace(replaceTerm, zeroGroupRegex, L"$1$$$0"); replaceTerm = regex_replace(replaceTerm, otherGroupsRegex, L"$1$0$4"); res = RegexReplaceDispatch[_useBoostLib](source, m_searchTerm, replaceTerm, m_flags & MatchAllOccurrences, !(m_flags & CaseSensitive)); replacedSomething = originalSource != res; } ``` Here `replacedSomething` controls whether the counter variable is incremented later in the code. The problem lies in the assumption that only a replacement result which differs from the original string implies a match. This is incorrect, as a successful match and replace operation can still produce a result which coincides with `originalSource` (the source filename), i.e. `originalSource == res`. The solution is to separate the regex matching from the replacement operation, and to advance the counter when the match is successful irrespective of whether the result is the same as the filename. This is what the fix does: ```cpp bool shouldIncrementCounter = false; if (m_flags & UseRegularExpressions) { replaceTerm = regex_replace(replaceTerm, zeroGroupRegex, L"$1$$$0"); replaceTerm = regex_replace(replaceTerm, otherGroupsRegex, L"$1$0$4"); res = RegexReplaceDispatch[_useBoostLib](source, m_searchTerm, replaceTerm, m_flags & MatchAllOccurrences, !(m_flags & CaseSensitive)); // Use regex search to determine if a match exists. This is the basis for incrementing // the counter. if (_useBoostLib) { boost::wregex pattern(m_searchTerm, boost::wregex::ECMAScript | (!(m_flags & CaseSensitive) ? boost::wregex::icase : boost::wregex::normal)); shouldIncrementCounter = boost::regex_search(sourceToUse, pattern); } else { auto regexFlags = std::wregex::ECMAScript; if (!(m_flags & CaseSensitive)) { regexFlags |= std::wregex::icase; } std::wregex pattern(m_searchTerm, regexFlags); shouldIncrementCounter = std::regex_search(sourceToUse, pattern); } } ``` The `regex_search()` call on both the boost and std paths tests whether the regex pattern can be found in the original filename (`sourceToUse`). `shouldIncrementCounter` tracks whether the counter will be incremented later, renamed from `replacedSomething` to reflect the change in behaviour. ## Validation Steps Performed The fix includes an additional unit test for both the std and boost regex paths. Without the fix, the test fails: <img width="1509" height="506" alt="Screenshot 2025-09-25 063611" src="https://github.com/user-attachments/assets/14dbf817-b1d3-456d-80f2-abcd28266b8d" /> and with the fix, all tests pass: <img width="897" height="492" alt="Screenshot 2025-09-25 063749" src="https://github.com/user-attachments/assets/9a587495-d54c-47d3-bc55-ccc64a805a88" />
2025-09-29 03:05:21 +01:00
pos = _Find(sourceToUse, searchTerm, isCaseInsensitive, pos);
if (pos != std::string::npos)
{
res = sourceToUse.replace(pos, searchTerm.length(), replaceTerm);
pos += replaceTerm.length();
[PowerRename] Fix issue where counter does not update if the filename and replacement result matched (#42006) This PR fixes an issue which has been reported a number of times under slightly different guises. The bug manifests as a counter "stall" or "skip" under certain circumstances, not advancing the counter if the result of the rename operation happens to match the original filename. <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request This issue occurs if all the following are true: - Enumeration features are enabled - Regular expression matching is enabled - The replacement string includes a counter, for example `${}` or `${start=1}` or `${start=10,increment=10}` etc. - There are one or more original filenames which coincide with the result of the renaming operations Previously, the counter was not updated when the renaming operation result was the same as the original filename. For example, here the first rename result matches the original filename and the counter remains at `1` for the second file, whereas it should be `2`: <img width="1002" height="759" alt="image" src="https://github.com/user-attachments/assets/2766f448-adc3-4fe7-9c13-f4c5505ae1d9" /> This fix increments the counter irrespective of these coincidences. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #41950, #31950, #33884 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments **Before discussing the detail of the fix, I'd like to acknowledge the incredible coincidence that it resolves two issues (31950 and 41950) _which are exactly 10,000 issues (and 18 months) apart_.** Now, back to the issue at hand, if you'll forgive the pun... The original flawed code is here: ```cpp bool replacedSomething = false; if (m_flags & UseRegularExpressions) { replaceTerm = regex_replace(replaceTerm, zeroGroupRegex, L"$1$$$0"); replaceTerm = regex_replace(replaceTerm, otherGroupsRegex, L"$1$0$4"); res = RegexReplaceDispatch[_useBoostLib](source, m_searchTerm, replaceTerm, m_flags & MatchAllOccurrences, !(m_flags & CaseSensitive)); replacedSomething = originalSource != res; } ``` Here `replacedSomething` controls whether the counter variable is incremented later in the code. The problem lies in the assumption that only a replacement result which differs from the original string implies a match. This is incorrect, as a successful match and replace operation can still produce a result which coincides with `originalSource` (the source filename), i.e. `originalSource == res`. The solution is to separate the regex matching from the replacement operation, and to advance the counter when the match is successful irrespective of whether the result is the same as the filename. This is what the fix does: ```cpp bool shouldIncrementCounter = false; if (m_flags & UseRegularExpressions) { replaceTerm = regex_replace(replaceTerm, zeroGroupRegex, L"$1$$$0"); replaceTerm = regex_replace(replaceTerm, otherGroupsRegex, L"$1$0$4"); res = RegexReplaceDispatch[_useBoostLib](source, m_searchTerm, replaceTerm, m_flags & MatchAllOccurrences, !(m_flags & CaseSensitive)); // Use regex search to determine if a match exists. This is the basis for incrementing // the counter. if (_useBoostLib) { boost::wregex pattern(m_searchTerm, boost::wregex::ECMAScript | (!(m_flags & CaseSensitive) ? boost::wregex::icase : boost::wregex::normal)); shouldIncrementCounter = boost::regex_search(sourceToUse, pattern); } else { auto regexFlags = std::wregex::ECMAScript; if (!(m_flags & CaseSensitive)) { regexFlags |= std::wregex::icase; } std::wregex pattern(m_searchTerm, regexFlags); shouldIncrementCounter = std::regex_search(sourceToUse, pattern); } } ``` The `regex_search()` call on both the boost and std paths tests whether the regex pattern can be found in the original filename (`sourceToUse`). `shouldIncrementCounter` tracks whether the counter will be incremented later, renamed from `replacedSomething` to reflect the change in behaviour. ## Validation Steps Performed The fix includes an additional unit test for both the std and boost regex paths. Without the fix, the test fails: <img width="1509" height="506" alt="Screenshot 2025-09-25 063611" src="https://github.com/user-attachments/assets/14dbf817-b1d3-456d-80f2-abcd28266b8d" /> and with the fix, all tests pass: <img width="897" height="492" alt="Screenshot 2025-09-25 063749" src="https://github.com/user-attachments/assets/9a587495-d54c-47d3-bc55-ccc64a805a88" />
2025-09-29 03:05:21 +01:00
shouldIncrementCounter = true;
}
[ci]Upgrade to check-spelling 0.0.20alpha7 (#19127) * spelling: added Signed-off-by: Josh Soref <jsoref@users.noreply.github.com> * spelling: and Signed-off-by: Josh Soref <jsoref@users.noreply.github.com> * spelling: another Signed-off-by: Josh Soref <jsoref@users.noreply.github.com> * spelling: color Signed-off-by: Josh Soref <jsoref@users.noreply.github.com> * spelling: file Signed-off-by: Josh Soref <jsoref@users.noreply.github.com> * spelling: github Signed-off-by: Josh Soref <jsoref@users.noreply.github.com> * spelling: not Signed-off-by: Josh Soref <jsoref@users.noreply.github.com> * spelling: occurrences Signed-off-by: Josh Soref <jsoref@users.noreply.github.com> * spelling: stamp Signed-off-by: Josh Soref <jsoref@users.noreply.github.com> * spelling: suppressions Signed-off-by: Josh Soref <jsoref@users.noreply.github.com> * spelling: the Signed-off-by: Josh Soref <jsoref@users.noreply.github.com> * spelling: up to Signed-off-by: Josh Soref <jsoref@users.noreply.github.com> * spelling: whether Signed-off-by: Josh Soref <jsoref@users.noreply.github.com> * spelling: whichdoes Signed-off-by: Josh Soref <jsoref@users.noreply.github.com> * Upgrade check-spelling to v0.0.20-alpha7 Config based on: https://github.com/check-spelling/spell-check-this/tree/a5001170a754da309ca324ce7eed8a076af2f4ac * Adding duplicate detection to patterns.txt * Adding line_forbidden.patterns * Adding reject.txt * Updated excludes (and sorted) * Switching to unified workflow * moving `wil` to allow.txt to clarify that it's a term of art (https://github.com/microsoft/wil), whereas often it's a typo for `will`. * Update src/runner/main.cpp Co-authored-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
2022-07-01 10:09:41 -04:00
if (!(m_flags & MatchAllOccurrences))
{
break;
}
} while (pos != std::string::npos);
}
hr = SHStrDup(res.c_str(), result);
[PowerRename] Fix issue where counter does not update if the filename and replacement result matched (#42006) This PR fixes an issue which has been reported a number of times under slightly different guises. The bug manifests as a counter "stall" or "skip" under certain circumstances, not advancing the counter if the result of the rename operation happens to match the original filename. <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request This issue occurs if all the following are true: - Enumeration features are enabled - Regular expression matching is enabled - The replacement string includes a counter, for example `${}` or `${start=1}` or `${start=10,increment=10}` etc. - There are one or more original filenames which coincide with the result of the renaming operations Previously, the counter was not updated when the renaming operation result was the same as the original filename. For example, here the first rename result matches the original filename and the counter remains at `1` for the second file, whereas it should be `2`: <img width="1002" height="759" alt="image" src="https://github.com/user-attachments/assets/2766f448-adc3-4fe7-9c13-f4c5505ae1d9" /> This fix increments the counter irrespective of these coincidences. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #41950, #31950, #33884 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments **Before discussing the detail of the fix, I'd like to acknowledge the incredible coincidence that it resolves two issues (31950 and 41950) _which are exactly 10,000 issues (and 18 months) apart_.** Now, back to the issue at hand, if you'll forgive the pun... The original flawed code is here: ```cpp bool replacedSomething = false; if (m_flags & UseRegularExpressions) { replaceTerm = regex_replace(replaceTerm, zeroGroupRegex, L"$1$$$0"); replaceTerm = regex_replace(replaceTerm, otherGroupsRegex, L"$1$0$4"); res = RegexReplaceDispatch[_useBoostLib](source, m_searchTerm, replaceTerm, m_flags & MatchAllOccurrences, !(m_flags & CaseSensitive)); replacedSomething = originalSource != res; } ``` Here `replacedSomething` controls whether the counter variable is incremented later in the code. The problem lies in the assumption that only a replacement result which differs from the original string implies a match. This is incorrect, as a successful match and replace operation can still produce a result which coincides with `originalSource` (the source filename), i.e. `originalSource == res`. The solution is to separate the regex matching from the replacement operation, and to advance the counter when the match is successful irrespective of whether the result is the same as the filename. This is what the fix does: ```cpp bool shouldIncrementCounter = false; if (m_flags & UseRegularExpressions) { replaceTerm = regex_replace(replaceTerm, zeroGroupRegex, L"$1$$$0"); replaceTerm = regex_replace(replaceTerm, otherGroupsRegex, L"$1$0$4"); res = RegexReplaceDispatch[_useBoostLib](source, m_searchTerm, replaceTerm, m_flags & MatchAllOccurrences, !(m_flags & CaseSensitive)); // Use regex search to determine if a match exists. This is the basis for incrementing // the counter. if (_useBoostLib) { boost::wregex pattern(m_searchTerm, boost::wregex::ECMAScript | (!(m_flags & CaseSensitive) ? boost::wregex::icase : boost::wregex::normal)); shouldIncrementCounter = boost::regex_search(sourceToUse, pattern); } else { auto regexFlags = std::wregex::ECMAScript; if (!(m_flags & CaseSensitive)) { regexFlags |= std::wregex::icase; } std::wregex pattern(m_searchTerm, regexFlags); shouldIncrementCounter = std::regex_search(sourceToUse, pattern); } } ``` The `regex_search()` call on both the boost and std paths tests whether the regex pattern can be found in the original filename (`sourceToUse`). `shouldIncrementCounter` tracks whether the counter will be incremented later, renamed from `replacedSomething` to reflect the change in behaviour. ## Validation Steps Performed The fix includes an additional unit test for both the std and boost regex paths. Without the fix, the test fails: <img width="1509" height="506" alt="Screenshot 2025-09-25 063611" src="https://github.com/user-attachments/assets/14dbf817-b1d3-456d-80f2-abcd28266b8d" /> and with the fix, all tests pass: <img width="897" height="492" alt="Screenshot 2025-09-25 063749" src="https://github.com/user-attachments/assets/9a587495-d54c-47d3-bc55-ccc64a805a88" />
2025-09-29 03:05:21 +01:00
if (shouldIncrementCounter)
enumIndex++;
}
catch (regex_error e)
{
hr = E_FAIL;
}
return hr;
}
size_t CPowerRenameRegEx::_Find(std::wstring data, std::wstring toSearch, bool caseInsensitive, size_t pos)
{
if (caseInsensitive)
{
// Convert to lower
std::transform(data.begin(), data.end(), data.begin(), ::towlower);
std::transform(toSearch.begin(), toSearch.end(), toSearch.begin(), ::towlower);
}
// Find sub string position in given string starting at position pos
return data.find(toSearch, pos);
}
void CPowerRenameRegEx::_OnSearchTermChanged()
{
CSRWSharedAutoLock lock(&m_lockEvents);
for (auto it : m_renameRegExEvents)
{
if (it.pEvents)
{
it.pEvents->OnSearchTermChanged(m_searchTerm);
}
}
}
void CPowerRenameRegEx::_OnReplaceTermChanged()
{
CSRWSharedAutoLock lock(&m_lockEvents);
for (auto it : m_renameRegExEvents)
{
if (it.pEvents)
{
it.pEvents->OnReplaceTermChanged(m_replaceTerm);
}
}
}
void CPowerRenameRegEx::_OnFlagsChanged()
{
CSRWSharedAutoLock lock(&m_lockEvents);
for (auto it : m_renameRegExEvents)
{
if (it.pEvents)
{
it.pEvents->OnFlagsChanged(m_flags);
}
}
}
void CPowerRenameRegEx::_OnFileTimeChanged()
{
CSRWSharedAutoLock lock(&m_lockEvents);
for (auto it : m_renameRegExEvents)
{
if (it.pEvents)
{
it.pEvents->OnFileTimeChanged(m_fileTime);
}
}
}
[PowerRename] Support using photo metadata to replace in the PowerRename (#41728) <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request 1. Introduce WIC for power rename and add new class WICMetadataExtractor to use WIC to extract metadata. 2. Add some patterns for metadata extract. 3. Support XMP and EXIF metadata extract. 4. Add test data for xmp and exif extractor 5. Add attribution for the test data uploader. UI: <img width="2052" height="1415" alt="image" src="https://github.com/user-attachments/assets/9051b12e-4e66-4fdc-a4d4-3bada661c235" /> <img width="284" height="170" alt="image" src="https://github.com/user-attachments/assets/2fd67193-77a7-48f0-a5ac-08a69fe64e55" /> <img width="715" height="1160" alt="image" src="https://github.com/user-attachments/assets/5fa68a8c-d129-44dd-b747-099dfbcded12" /> demo: https://github.com/user-attachments/assets/e90bc206-62e5-4101-ada2-3187ee7e2039 <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #5612 - [x] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [x] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed --------- Co-authored-by: Yu Leng <yuleng@microsoft.com>
2025-11-04 09:27:16 +08:00
void CPowerRenameRegEx::_OnMetadataChanged()
{
CSRWSharedAutoLock lock(&m_lockEvents);
for (auto it : m_renameRegExEvents)
{
if (it.pEvents)
{
it.pEvents->OnMetadataChanged();
}
}
}
PowerRenameLib::MetadataType CPowerRenameRegEx::_GetMetadataTypeFromFlags() const
{
if (m_flags & MetadataSourceXMP)
return PowerRenameLib::MetadataType::XMP;
// Default to EXIF
return PowerRenameLib::MetadataType::EXIF;
}
// Interface method implementation
IFACEMETHODIMP CPowerRenameRegEx::GetMetadataType(_Out_ PowerRenameLib::MetadataType* metadataType)
{
if (metadataType == nullptr)
return E_POINTER;
*metadataType = _GetMetadataTypeFromFlags();
return S_OK;
}
// Convenience method for internal use
PowerRenameLib::MetadataType CPowerRenameRegEx::GetMetadataType() const
{
return _GetMetadataTypeFromFlags();
}