Use internationally viable formatting for dates used in, for example, file paths

[Forgind]

Fixes #46727
Fixes #37932
(@MiYanni commented that it looks similar to the issue I'd been looking at. Turns out 46727 is a dupe, but since I have a PR out already, I'll leave them up and close both via this.)

This replaces an explicit format with "u" for two specific DateTime.ToString instances. Lowercase u is for a sortable, universal date/time pattern:

(https://learn.microsoft.com/dotnet/standard/base-types/standard-date-and-time-format-strings)

I saw a number of other instances in which we currently use an explicit string format like this. I was a little split on whether I should change them all without looking at it too closely, but I can if desired.

[Copilot]

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

[KalleOlaviNiemitalo]

The "u" format includes colons that are not valid for file paths on Windows.

[KalleOlaviNiemitalo]

Could instead use String.Create(IFormatProvider? provider, ref System.Runtime.CompilerServices.DefaultInterpolatedStringHandler handler) with CultureInfo.InvariantCulture.

[Forgind]

Could instead use String.Create(IFormatProvider? provider, ref System.Runtime.CompilerServices.DefaultInterpolatedStringHandler handler) with CultureInfo.InvariantCulture.

I'm trying to figure out how to make this make sense, and I don't, I'm afraid. The IFormatProvider can be CultureInfo.InvariantCulture, but what to use for the DefaultInterpolatedStringHandler? That essentially takes in a format string and some arguments and interpolates them, as the name suggests. There's a format in the current version already, so that would just mean replacing one format with another, and I don't see how that improves things.

I'm thinking about just replacing ':' (and perhaps ' ') with '_' in the 'u' format. Is there any reason that wouldn't work?

[KalleOlaviNiemitalo]

I meant like string.Create(CultureInfo.InvariantCulture, $"Microsoft.NET.Workload_{Environment.ProcessId}_{DateTime.Now:yyyyMMdd_HHmmss_fff}.log"), to prevent a user-specified culture from introducing quotation marks or other problematic characters.

https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-10.0/improved-interpolated-strings

[Forgind]

Thanks for the extra color 🙂 That sounds reasonable to me; I want to do some testing to make sure it works as expected, and if so, I can switch to that.

[Forgind]

Ok, made a console to check out the difference. This was the result:

Will push the change 🙂

[KalleOlaviNiemitalo]

Isn't this what's written as text to each log entry in the log file? I wouldn't expect the process ID and ".log" there. The nested interpolation looks unnecessarily complex, too.

[KalleOlaviNiemitalo]

For this, $"[{DateTime.Now:u}]" would be safe because the result is not used as part of a file path. However, the u format does not include milliseconds, unlike the current format. Have the milliseconds in the log been useful for tracking down performance problems?

[MiYanni]

Hmm. I'm confused here. How does string.Create(CultureInfo.InvariantCulture, apply the culture to the string?

[MiYanni]

To clarify, in the past I've used this version of ToString on the DateTime to set it to invariant culture.

date.ToString("yyyyMMdd", CultureInfo.InvariantCulture);

[KalleOlaviNiemitalo]

How does string.Create(CultureInfo.InvariantCulture, apply the culture to the string?

An expression like string.Create(CultureInfo.InvariantCulture, $"[{DateTime.Now:yyyyMMdd}]") calls public static string Create(IFormatProvider? provider, [InterpolatedStringHandlerArgument(nameof(provider))] ref DefaultInterpolatedStringHandler handler). The C# compiler generates code to convert the interpolated string $"[{DateTime.Now:yyyyMMdd}]" to a DefaultInterpolatedStringHandler instance. Because of the InterpolatedStringHandlerArgumentAttribute, the value of the IFormatProvider? provider parameter, i.e. CultureInfo.InvariantCulture, is passed to the DefaultInterpolatedStringHandler constructor.

SharpLab

[MiYanni]

@KalleOlaviNiemitalo Makes sense. I've only briefly worked with interpolated string handler in the past so I didn't think about it pulling apart things like that. Nice! Thanks for the explanation.