Skip to content

Conversation

@akeit0
Copy link
Collaborator

@akeit0 akeit0 commented Dec 13, 2025

@akeit0 akeit0 requested a review from Copilot December 13, 2025 08:41
Copy link

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.

Pull request overview

This PR fixes a bug where escape sequences in os.date format strings were not being properly handled. The issue occurred when a % character wasn't followed by a recognized format specifier - the code failed to set the escape sequence flag, causing subsequent characters to be processed incorrectly.

Key changes:

  • Added an else clause to properly set isEscapeSequence = true when % is encountered
  • Added a test with FakeTimeProvider to verify the fix works correctly
  • Added Microsoft.Extensions.TimeProvider.Testing package dependency for testing

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
tests/Lua.Tests/Lua.Tests.csproj Added testing package dependency for time provider mocking
tests/Lua.Tests/DateTimeTests.cs Added test case to verify date formatting with escape sequences
src/Lua/Standard/Internal/DateTimeHelper.cs Fixed escape sequence handling logic in date formatting

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

public class DateTimeTests
{
[Test]
public async Task Test_LocalFunction_Nil_1()
Copy link

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

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

The test name 'Test_LocalFunction_Nil_1' does not describe what is being tested. It should be renamed to something like 'Test_OsDate_FormatsDateTimeCorrectly' or 'Test_OsDate_EscapeSequenceHandling' to reflect that it's testing date formatting functionality.

Suggested change
public async Task Test_LocalFunction_Nil_1()
public async Task Test_OsDate_FormatsDateTimeCorrectly()

Copilot uses AI. Check for mistakes.
@akeit0 akeit0 merged commit 4f821af into main Dec 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants