Fable: TimeSpan.ToString

[rfrerebe]

Need for TimeSpan.ToString

As discussed with @alfonsogarciacaro and with @MangelMaxime

There seems to be a need to improve TimeSpan.ToString().

From documentation there is 3 versions of TimeSpan.ToString()

1/ Without Parameter (equivalent to ("c", InvariantCulture))

2/ With one string ("c", "g" or "G") with current culture

3/ With a string format and a CultureInfo

String can be "c", "g", "G" or custom format (ex :hh:mm:ss)

Here is a full table of the different formatting provided by .NET

|TimeSpan|Format|InvariantCulture|"en-US"|"fr-FR"|
|---|---|---|---|---|
|TimeSpan(0,16,14,30,527)|"c"|16:14:30.5270000|16:14:30.5270000|16:14:30.5270000|
|TimeSpan(0,16,14,30,527)|"g"|16:14:30.527|16:14:30.527|16:14:30.527|
|TimeSpan(0,16,14,30,527)|"G"|0:16:14:30.5270000|0:16:14:30.5270000|0:16:14:30,5270000|
|TimeSpan(0,16,14,30,527)|"hh\:mm\:ss"|16:14:30|16:14:30|16:14:30|
|TimeSpan(0,38,30,15,42)|"c"|1.14:30:15.0420000|1.14:30:15.0420000|1.14:30:15.0420000|
|TimeSpan(0,38,30,15,42)|"g"|1:14:30:15.042|1:14:30:15.042|1:14:30:15,042|
|TimeSpan(0,38,30,15,42)|"G"|1:14:30:15.0420000|1:14:30:15.0420000|1:14:30:15,0420000|
|TimeSpan(0,38,30,15,42)|"hh\:mm\:ss"|14:30:15|14:30:15|14:30:15|

|TimeSpan.ToString with Parameters|Value|
|---|---|
|TimeSpan(0,16,14,30,527).ToString("c", CultureInfo.InvariantCulture)|16:14:30.5270000|
|TimeSpan(0,16,14,30,527).ToString("c", CultureInfo("en-US"))|16:14:30.5270000|
|TimeSpan(0,16,14,30,527).ToString("c", CultureInfo("fr-FR"))|16:14:30.5270000|
|TimeSpan(0,16,14,30,527).ToString("g", CultureInfo.InvariantCulture)|16:14:30.527|
|TimeSpan(0,16,14,30,527).ToString("g", CultureInfo("en-US"))|16:14:30.527|
|TimeSpan(0,16,14,30,527).ToString("g", CultureInfo("fr-FR"))|16:14:30,527|
|TimeSpan(0,16,14,30,527).ToString("G", CultureInfo.InvariantCulture)|0:16:14:30.5270000|
|TimeSpan(0,16,14,30,527).ToString("G", CultureInfo("en-US"))|0:16:14:30.5270000|
|TimeSpan(0,16,14,30,527).ToString("G", CultureInfo("fr-FR"))|0:16:14:30,5270000|
|TimeSpan(0,16,14,30,527).ToString("hh\:mm\:ss", CultureInfo.InvariantCulture)|16:14:30|
|TimeSpan(0,16,14,30,527).ToString("hh\:mm\:ss", CultureInfo("en-US"))|16:14:30|
|TimeSpan(0,16,14,30,527).ToString("hh\:mm\:ss", CultureInfo("fr-FR"))|16:14:30|
|||
|TimeSpan(0,38,30,15,42).ToString("c", CultureInfo.InvariantCulture)|1.14:30:15.0420000|
|TimeSpan(0,38,30,15,42).ToString("c", CultureInfo("en-US"))|1.14:30:15.0420000|
|TimeSpan(0,38,30,15,42).ToString("c", CultureInfo("fr-FR"))|1.14:30:15.0420000|
|TimeSpan(0,38,30,15,42).ToString("g", CultureInfo.InvariantCulture)|1:14:30:15.042|
|TimeSpan(0,38,30,15,42).ToString("g", CultureInfo("en-US"))|1:14:30:15.042|
|TimeSpan(0,38,30,15,42).ToString("g", CultureInfo("fr-FR"))|1:14:30:15,042|
|TimeSpan(0,38,30,15,42).ToString("G", CultureInfo.InvariantCulture)|1:14:30:15.0420000|
|TimeSpan(0,38,30,15,42).ToString("G", CultureInfo("en-US"))|1:14:30:15.0420000|
|TimeSpan(0,38,30,15,42).ToString("G", CultureInfo("fr-FR"))|1:14:30:15,0420000|
|TimeSpan(0,38,30,15,42).ToString("hh\:mm\:ss", CultureInfo.InvariantCulture)|14:30:15|
|TimeSpan(0,38,30,15,42).ToString("hh\:mm\:ss", CultureInfo("en-US"))|14:30:15|
|TimeSpan(0,38,30,15,42).ToString("hh\:mm\:ss", CultureInfo("fr-FR"))|14:30:15|

Proposition

I don't think it's a good idea to implement TimeSpan.ToString with a single parameter because it's using current culture.
I also don't thing it's a good idea to implement particular culture ("en-US" or "fr-FR")
I am unsure about the need to implement specific format string. (ex: hh:mm:ss)

I am ok to implement

• TimeSpan.ToString("c",CultureInfo.InvariantCulture)
• TimeSpan.ToString("g",CultureInfo.InvariantCulture)
• TimeSpan.ToString("G",CultureInfo.InvariantCulture)

What is your opinion about this ?

FYI, Currently there is an error if any parameter is passed to ToString().

[MangelMaxime]

In the case Thoth.Json, we are only interested in a CultureInfo.InvariantCulture support.

So if understood correctly, using no parameters is enough in this case.

Adding support for only a sub set of ToString format is a careful choice to make because users will probably expect to have all of them supported. If we say, only ToString() (no parameter) is supported it easy for the user to know what is supported and what is not.

A possibilities is to create a new library adding support for the formatting as I did in the past for Date formatting.
http://fable.io/fable-powerpack/date-format.html

[rfrerebe]

So if understood correctly, using no parameters is enough in this case.

Yes

Adding support for only a sub set of ToString format is a careful choice to make because users will probably expect to have all of them supported

I also agree.

There is no point to create a full library if nobody uses it

[alfonsogarciacaro]

Fable doesn't support specific cultures because that would a lot of work and would also probably make bundle sizes explode. In general Fable behaves as if InvariantCulture was specified even if .NET defaults to current culture. Because of this, Fable allows passing Globalization.CultureInfo.InvariantCulture when users want to be explicit or they want to share code with .NET and make sure both platforms output the same result (example1, example2).

So you can add an optional argument for the format and ignore the culture. Fable will just throw an error if users try to access any culture that's not InvariantCulture. For example:

NOTE: The last line of the code below is not correct, I just implemented the day separator as a sample but I'm not sure about the padding in other parts so I haven't touched them.

NOTE 2: Remember also to remove these lines to enable the format parameter.

```typescript
export function toString(ts: number, format = "c") {
if (["c", "g", "G"].indexOf(format) === -1) {
throw new Error("Custom formats are not supported");
}

const d = days(ts);
const h = hours(ts);
const m = minutes(ts);
const s = seconds(ts);
const ms = milliseconds(ts);
// tslint:disable-next-line:max-line-length
return ${d === 0 ? "" : d + (format === "c" ? "." : ":")}${padWithZeros(h, 2)}:${padWithZeros(m, 2)}:${padWithZeros(s, 2)}${ms === 0 ? "" : "." + padWithZeros(ms, 3)};
}```

[rfrerebe]

Ok it's clearer.
I understand it's not an issue to support a subset of ToString in fable.

I will send a PR when I can, and also update Thoth to use ToString("c", CultureInfo.InvariantCulture) when it will available.

[rfrerebe]

1707