Skip to content

[CmdPalette > Time and Date] Custom formats (Port #37743) and other plugin improvements#38143

Closed
htcfreek wants to merge 23 commits intomicrosoft:mainfrom
htcfreek:PT_CPalTdCFormats
Closed

[CmdPalette > Time and Date] Custom formats (Port #37743) and other plugin improvements#38143
htcfreek wants to merge 23 commits intomicrosoft:mainfrom
htcfreek:PT_CPalTdCFormats

Conversation

@htcfreek
Copy link
Collaborator

@htcfreek htcfreek commented Mar 25, 2025

Summary of the Pull Request

This PR ports #37743 to Command Palette module.

This PR implements support for custom formats in the "Time and Date" plugin.

image

And this PR adds support for the following new formats/results:

Format/Result Input parsing support Built-in result Custom format support
Days in month. x x
Is leap year. x
OLE Automation Date x x
Excel date value (base 1900) x x
Excel date value (base 1904) x x

Additionally this PR ...

  • reorders settings in the ui.
  • removes two settings that make no sense with current CmdPal's behavior.
  • improves the way how error details are shown.
  • fixes week of month calculation.
  • fixes a crash that occurs when converting early dates (e.g. 1.1.100) into windows file time.
  • improves error messages for input parsing formats.

image

image

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

  • Build and run on my local machine.

@htcfreek htcfreek requested a review from zadjii-msft March 25, 2025 13:46
@htcfreek htcfreek self-assigned this Mar 25, 2025
@htcfreek htcfreek added Idea-Enhancement New feature or request on an existing product Product-Command Palette Refers to the Command Palette utility labels Mar 25, 2025
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@htcfreek
Copy link
Collaborator Author

htcfreek commented Mar 25, 2025

@zadjii-msft
Can you please help me. The solution doesn't build anymore after changing ResultHelper.cs. And I do not understand why these types are unknown now and can't convert.

image

@htcfreek htcfreek added the Don't merge - Hold for release Hold off on merging this PR, even if it's approved. label Mar 25, 2025
@zadjii-msft
Copy link
Member

that's weird. It's like the toolkit project wasn't built yet? Sometimes that would happen earlier when the interface changed and we re-generated the winmd, but that hasn't changed since we merged into upstream.

manually clean all the Microsoft.CommandPalette.Extensions.winmd's you find? clean-sdk.ps1 should tidy that up.

I can help more after 0.90 is out.

Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking for 0.90

@htcfreek
Copy link
Collaborator Author

that's weird. It's like the toolkit project wasn't built yet? Sometimes that would happen earlier when the interface changed and we re-generated the winmd, but that hasn't changed since we merged into upstream.

manually clean all the Microsoft.CommandPalette.Extensions.winmd's you find? clean-sdk.ps1 should tidy that up.

I can help more after 0.90 is out.

@zadjii-msft
Fyi:

  • git clean -xfd: Did not solve.
  • clean-sdk.ps1: Did not solve.
  • Delete local clone and clone again: Did not solve.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@htcfreek
Copy link
Collaborator Author

@zadjii-msft
I fixed the build.

@github-actions

This comment has been minimized.

@htcfreek htcfreek added the Status-Blocked We can't make progress due to a dependency or issue label Mar 26, 2025
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@htcfreek htcfreek requested review from Copilot and moooyo April 6, 2025 15:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 7 out of 10 changed files in this pull request and generated no comments.

Files not reviewed (3)
  • .github/actions/spell-check/expect.txt: Language not supported
  • src/modules/cmdpal/Exts/Microsoft.CmdPal.Ext.TimeDate/Properties/Resources.Designer.cs: Language not supported
  • src/modules/cmdpal/Exts/Microsoft.CmdPal.Ext.TimeDate/Properties/Resources.resx: Language not supported
Comments suppressed due to low confidence (2)

src/modules/cmdpal/Exts/Microsoft.CmdPal.Ext.TimeDate/Helpers/TimeAndDateHelper.cs:240

  • Consider specifying a NumberStyles and CultureInfo (for example, NumberStyles.Float with CultureInfo.InvariantCulture) when calling double.TryParse in the OLE Automation date branch (and similarly in the exc/exf branches) to ensure consistent parsing behavior across different locales.
else if (Regex.IsMatch(input, @"^oa[+-]?\d+[,.0-9]*$") )

src/modules/cmdpal/Exts/Microsoft.CmdPal.Ext.TimeDate/Helpers/AvailableResultsList.cs:100

  • [nitpick] The variable 'value' is used both to hold the custom format string and to store the final formatted date. Consider renaming the intermediate variable (e.g. 'customFormat') to improve code clarity.
var value = TimeAndDateHelper.ConvertToCustomFormat(dtObject, unixTimestamp, unixTimestampMilliseconds, weekOfYear, eraShort, Regex.Replace(formatSyntax, "^UTC:", string.Empty), firstWeekRule, firstDayOfTheWeek);

@moooyo
Copy link
Contributor

moooyo commented Apr 7, 2025

We are preparing for a fix releasing.

Approve first. But we still need to wait. @htcfreek

@yeelam-gordon
Copy link
Contributor

/azp run

@yeelam-gordon yeelam-gordon removed Needs-Review This Pull Request awaits the review of a maintainer. in for .91 labels Apr 8, 2025
@yeelam-gordon yeelam-gordon added this to the PowerToys 0.91 milestone Apr 8, 2025
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@htcfreek
Copy link
Collaborator Author

@zadjii-msft
Can you unblock this?

@moooyo
Copy link
Contributor

moooyo commented Apr 11, 2025

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@moooyo
Copy link
Contributor

moooyo commented Apr 16, 2025

@htcfreek could you please merge the main branch?

We renamed the EXT folder to ext.. So there are some conflict.

If it's ready, I'll ping mike to unblock it.

@crutkas
Copy link
Member

crutkas commented Apr 16, 2025

this has a lot of conflicts now

@htcfreek
Copy link
Collaborator Author

this has a lot of conflicts now

@moooyo , @crutkas
I can't fix this before friday/saturday I think. Definitely not a nice situation now. 😅

@htcfreek
Copy link
Collaborator Author

@crutkas, @moooyo
New PR without conflicts: #38952

@htcfreek htcfreek closed this Apr 18, 2025
@htcfreek htcfreek deleted the PT_CPalTdCFormats branch April 18, 2025 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Idea-Enhancement New feature or request on an existing product Product-Command Palette Refers to the Command Palette utility

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants