-
Notifications
You must be signed in to change notification settings - Fork 7.1k
[cmdpal] Fix TimeAndDate extension crash issue when typing query #40245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
do we really need all thos complicated stuff? Can't we simply get data an return it? In my last pr i tried to fix outdated results by never having cache marked as up to date. but maybe it makes more sense to remove it at all. |
oh... hard to do it. You can repro this issue if you type anything in this extension
|
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
If we are sure that every time the page is opened or the query changes we get live time results and not cached ones, I am okay with this code. |
Yeah, I can confirm it works. Will not break your previours PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
) <!-- 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 When we call RaiseItemsChanged, it will trigger function GetItems. So if you enter this function through typing query, it will trap in an inf loop. Eventually leading to crash. related change: #40050 <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] **Closes:** #39973 - [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 - [ ] **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>
Summary of the Pull Request
When we call RaiseItemsChanged, it will trigger function GetItems. So if you enter this function through typing query, it will trap in an inf loop. Eventually leading to crash.
related change: #40050
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed