command-line-api
command-line-api copied to clipboard
dotnet
Make commandline arguments support different locales with an IFormatProvider
The following tests fail in my nl_NL locale:
- Values_can_be_correctly_converted_to_decimal_without_the_parser_specifying_a_custom_converter
- Values_can_be_correctly_converted_to_double_without_the_parser_specifying_a_custom_converter
- Values_can_be_correctly_converted_to_float_without_the_parser_specifying_a_custom_converter
- Values_can_be_correctly_converted_to_nullable_decimal_without_the_parser_specifying_a_custom_converter
- Values_can_be_correctly_converted_to_nullable_double_without_the_parser_specifying_a_custom_converter
- Values_can_be_correctly_converted_to_nullable_float_without_the_parser_specifying_a_custom_converter
This is because in Europe we write 123.456,78 as opposed to the US 123,456.78 format.
I have forked the project and made an attempt to fix this by accepting an IFormatProvider in the TryConvertString delegate; I'm quite sure this is the "correct" way to go (or at least something similar to this), but that turned out to be a bit more of a rabbithole than I currently have time for.
Here's my proposed change in `ArgumentConverter.StringConverters.cs`
// Copyright (c) .NET Foundation and contributors. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
using System.Collections.Generic;
using System.Globalization;
using System.IO;
namespace System.CommandLine.Binding;
internal static partial class ArgumentConverter
{
private delegate bool TryConvertString(string token, IFormatProvider formatProvider, out object? value);
private static readonly Dictionary<Type, TryConvertString> _stringConverters = new()
{
[typeof(bool)] = (string token, IFormatProvider formatProvider, out object? value) =>
{
if (bool.TryParse(token, out var parsed))
{
value = parsed;
return true;
}
value = default;
return false;
},
[typeof(DateTime)] = (string input, IFormatProvider formatProvider, out object? value) =>
{
if (DateTime.TryParse(input, formatProvider, DateTimeStyles.AssumeLocal, out var parsed))
{
value = parsed;
return true;
}
value = default;
return false;
},
[typeof(DateTimeOffset)] = (string input, IFormatProvider formatProvider, out object? value) =>
{
if (DateTimeOffset.TryParse(input, formatProvider, DateTimeStyles.AssumeLocal, out var parsed))
{
value = parsed;
return true;
}
value = default;
return false;
},
[typeof(decimal)] = (string input, IFormatProvider formatProvider, out object? value) =>
{
if (decimal.TryParse(input, NumberStyles.Float, formatProvider, out var parsed))
{
value = parsed;
return true;
}
value = default;
return false;
},
[typeof(DirectoryInfo)] = (string path, IFormatProvider formatProvider, out object? value) =>
{
if (string.IsNullOrEmpty(path))
{
value = default;
return false;
}
value = new DirectoryInfo(path);
return true;
},
[typeof(double)] = (string input, IFormatProvider formatProvider, out object? value) =>
{
if (double.TryParse(input, NumberStyles.Float, formatProvider, out var parsed))
{
value = parsed;
return true;
}
value = default;
return false;
},
[typeof(FileInfo)] = (string path, IFormatProvider formatProvider, out object? value) =>
{
if (string.IsNullOrEmpty(path))
{
value = default;
return false;
}
value = new FileInfo(path);
return true;
},
[typeof(FileSystemInfo)] = (string path, IFormatProvider formatProvider, out object? value) =>
{
if (string.IsNullOrEmpty(path))
{
value = default;
return false;
}
if (Directory.Exists(path))
{
value = new DirectoryInfo(path);
}
else if (path.EndsWith(Path.DirectorySeparatorChar.ToString(), StringComparison.Ordinal) ||
path.EndsWith(Path.AltDirectorySeparatorChar.ToString(), StringComparison.Ordinal))
{
value = new DirectoryInfo(path);
}
else
{
value = new FileInfo(path);
}
return true;
},
[typeof(float)] = (string input, IFormatProvider formatProvider, out object? value) =>
{
if (float.TryParse(input, NumberStyles.Float, formatProvider, out var parsed))
{
value = parsed;
return true;
}
value = default;
return false;
},
[typeof(Guid)] = (string input, IFormatProvider formatProvider, out object? value) =>
{
if (Guid.TryParse(input, out var parsed))
{
value = parsed;
return true;
}
value = default;
return false;
},
[typeof(int)] = (string token, IFormatProvider formatProvider, out object? value) =>
{
if (int.TryParse(token, NumberStyles.Integer, formatProvider, out var intValue))
{
value = intValue;
return true;
}
value = default;
return false;
},
[typeof(long)] = (string token, IFormatProvider formatProvider, out object? value) =>
{
if (long.TryParse(token, NumberStyles.Integer, formatProvider, out var longValue))
{
value = longValue;
return true;
}
value = default;
return false;
},
[typeof(short)] = (string token, IFormatProvider formatProvider, out object? value) =>
{
if (short.TryParse(token, NumberStyles.Integer, formatProvider, out var shortValue))
{
value = shortValue;
return true;
}
value = default;
return false;
},
[typeof(uint)] = (string token, IFormatProvider formatProvider, out object? value) =>
{
if (uint.TryParse(token, NumberStyles.Integer, formatProvider, out var uintValue))
{
value = uintValue;
return true;
}
value = default;
return false;
},
[typeof(sbyte)] = (string token, IFormatProvider formatProvider, out object? value) =>
{
if (sbyte.TryParse(token, NumberStyles.Integer, formatProvider, out var sbyteValue))
{
value = sbyteValue;
return true;
}
value = default;
return false;
},
[typeof(byte)] = (string token, IFormatProvider formatProvider, out object? value) =>
{
if (byte.TryParse(token, NumberStyles.Integer, formatProvider, out var byteValue))
{
value = byteValue;
return true;
}
value = default;
return false;
},
[typeof(string)] = (string input, IFormatProvider formatProvider, out object? value) =>
{
value = input;
return true;
},
[typeof(ulong)] = (string token, IFormatProvider formatProvider, out object? value) =>
{
if (ulong.TryParse(token, NumberStyles.Integer, formatProvider, out var ulongValue))
{
value = ulongValue;
return true;
}
value = default;
return false;
},
[typeof(ushort)] = (string token, IFormatProvider formatProvider, out object? value) =>
{
if (ushort.TryParse(token, NumberStyles.Integer, formatProvider, out var ushortValue))
{
value = ushortValue;
return true;
}
value = default;
return false;
},
[typeof(Uri)] = (string input, IFormatProvider formatProvider, out object? value) =>
{
if (Uri.TryCreate(input, UriKind.RelativeOrAbsolute, out var uri))
{
value = uri;
return true;
}
value = default;
return false;
},
[typeof(TimeSpan)] = (string input, IFormatProvider formatProvider, out object? value) =>
{
if (TimeSpan.TryParse(input, formatProvider, out var timeSpan))
{
value = timeSpan;
return true;
}
value = default;
return false;
},
};
}
I think the above can serve as a startingpoint.
If this is an attempt to fix the tests, i guess the correct way is to define the culture(s) the tests are running under. Basically it is a task of controlling the test environment in which a test is supposed to be run.
In XUnit (the test framework used by System.CommandLine), defining a culture for a test can be achieved with the help of the [CulturedFact] and [CulturedTheory] attributes.
No, fixing the tests sweeps the actual problem under the rug IMHO. Yes, we can force the tests to run (for example) under CultureInfo.InvariantCulture but the actual "problem" is that ideally you want to be able to (somehow) specify a culture and test for a few (random) cultures so commandline arguments can be passed in a user's locale if desired. Relying on the current culture is an option, ofcourse.
Alternatively we could default to the invariant culture and "hardcode" it; I just don't think that's the best option.
I don't understand you. With [CulturedFact] and [CulturedTheory] you actually specify cultures the tests run under. What problem is being swept under the rug here by specifying the cultures which is what you seem to want and what [CulturedFact] and [CulturedTheory] seem to accomplish?
What problem is being swept under the rug
Not being able to specify which culture the commandline parser should use. I may be a dutch user but depending on the application and target audience I'd like to be able to specify which culture to use for some applications, not relying on the CurrentCulture and/or CurrentUICulture. The same way as I can pass an IFormatProvider to methods like Int.Parse(...)
Ah, no, no it isn't. The tests just highlighted my issue. I should've chosen a better title and written a better description. My bad. I'll fix this after dinner. Sorry for the confusion.
Ah, alright, now i understand. Yeah, the title and report of the issue focus a lot on the tests. That's why i couldn't follow your statements. :-)
The issue title proposes one solution but I'd like to better understand the problem a bit better.
In particular, how would the format provider be chosen? Do we want end users to be able to choose the culture or just infer it from their machine settings? Would using the machine settings open the door to scripts that work differently on one machine than on another? Should there be a standard way to override it from the command line?
Some related discussion here: #951
Would using the machine settings open the door to scripts that work differently on one machine than on another?
Well, that's what the current implementation with TryParse(value, out var result) does; the TryParse methods use the CurrentCulture (e.g. Int.TryParse uses NumberFormatInfo.CurrentInfo which in turn uses Thread.CurrentThread.CurrentCulture.
This is fine for most cases I guess and we could default to the same CurrentCulture when no IFormatProvider is specified.
In particular, how would the format provider be chosen?
I can currently think of two options but I'm sure there will be more:
- An (optional, nullable)
IFormatProviderargument on theOption<T>constructor (maybe even inherited from parent commands all the way 'up' to the RootCommand but when set on a 'deeper' level that one is used?) - Maybe (again, optionally) pass an
IFormatProviderto theInvoke/InvokeAsyncmethods on a (root)command?
Should there be a standard way to override it from the command line?
Interesting question. I can imagine passing a --locale nl_NL or --locale 1043 or something but I guess that's maybe a little overkill. I don't think I would have a need for it, and if I did it would be easily implemented 'manually' as long as the id/string can then be converted to a culture / IFormatProvider and passed into the Invoke(Async) method for example.
As some prior art, the .NET CLI uses the DOTNET_CLI_UI_LANGUAGE environment variable to force a specific UI culture. You could easily trial a S.CL Middleware that did the equivalent with a shared environment variable like DOTNET_UI_LANGUAGE or something.
We can certainly design an API for passing an IFormatProvider into the parser, and in fact you can use a custom format provider already by passing a ParseArgument<T> to the Option<T> or Argument<T> constructors.
The questions I'd like to work through are about the experience of a person using your CLI app:
- How do they learn about the behavior? Can we come up with something that we think is appropriately generalized? (The directive syntax, e.g.
[culture:nl-NL], is intended to provide this in a way that won't conflict with options that someone might have defined for other reasons. An environment variable is a much more common convention, and we can support that from the command line through an environment variable directive, e.g.[env:CULTURE=123]. I'm not sure what name here would be too specific or too general.) - How do they specify a different format if they need to?
- Should the output culture (e.g. help) and input culture (e.g. parsing) be decoupled?
The discussion in #951 works through a lot of these details which is useful context for this.
and in fact you can use a custom format provider
...Which made me also realize we might want to be able to specify a format string (like dd-MM-yyyy).
and we can support that from the command line through an environment variable directive, e.g. [env:CULTURE=123]
Maybe the name of the environment variable should also be set in the Option?
How do they specify a different format if they need to?
I'm not sure if you'd want end-users to be able to specify different formats; I think you'd just want your application to support / require format A (B and C).
Should the output culture (e.g. help) and input culture (e.g. parsing) be decoupled?
Yes, in my opinion.
The discussion in #951 works through a lot of these
I'll have a look when I get a bit more time.