-
-
Notifications
You must be signed in to change notification settings - Fork 557
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
Change JsDate.ToString to follow Date prototype logic #1425
Conversation
Jint/Native/JsDate.cs
Outdated
@@ -56,6 +56,6 @@ public override string ToString() | |||
return "Infinity"; | |||
} | |||
|
|||
return ToDateTime().ToString("ddd MMM dd yyyy HH:mm:ss 'GMT'zzz", CultureInfo.InvariantCulture); | |||
return _engine.Realm.Intrinsics.Date.PrototypeObject.ToDateString(DateValue).ToString(); |
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.
do other JS object follow this pattern?
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.
meaning JsNumber, ...
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.
well the primitives are quite limited with toString, should we just ignore this - don't you like the behavior?
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.
Well JsNumber does follow JS semantincs:
public override string ToString()
{
return TypeConverter.ToString(_value);
}
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.
Changed to use TypeConverter
here too
Not sure we need the new dependency. I love the library, use it in Fluid also, but in this case we just need a culture info, we can just use a string that is known from both linux and windows in dotnet core. |
fixes #1424