fix(unparser): format values with parser ParsingCulture for round-trip
FormatCommandLine wrote option and value arguments using the ambient thread culture via value.ToString(), while the parser reads them back with ParserSettings.ParsingCulture (invariant by default). On threads whose culture uses a comma decimal separator (for example de-DE) a double 1.5 was unparsed as "1,5" and reparsed as 15, and a sequence [1.5, 2.5] became "1,5,2,5" and reparsed as four elements [1,5,2,5], so the format/parse round-trip was broken. Format IFormattable values (numeric and date/time types) with the parser ParsingCulture instead of the current thread culture, for both scalar and separator-joined sequence values. Other types keep their default string representation, so behaviour on invariant/en-US is unchanged. Add round-trip tests under de-DE for a scalar double and a comma-separated double sequence.
This commit is contained in:
1 parent
6340b0ba56
commit
91e0d866bc
3 files changed
+104
-9
No files matched your search
@@ -3,6 +3,7 @@
|
|||||||
using System;
|
using System;
|
||||||
using System.Collections;
|
using System.Collections;
|
||||||
using System.Collections.Generic;
|
using System.Collections.Generic;
|
||||||
|
using System.Globalization;
|
||||||
using System.Linq;
|
using System.Linq;
|
||||||
using System.Text;
|
using System.Text;
|
||||||
using CommandLine.Core;
|
using CommandLine.Core;
|
||||||
@@ -132,6 +133,11 @@ namespace CommandLine
|
|||||||
configuration(settings);
|
configuration(settings);
|
||||||
settings.Consumed = true;
|
settings.Consumed = true;
|
||||||
|
|
||||||
|
// Values are formatted using the parser's parsing culture (invariant by
|
||||||
|
// default) so that the produced command line round-trips through the same
|
||||||
|
// parser even when the current thread uses a different culture.
|
||||||
|
var parsingCulture = parser.Settings.ParsingCulture;
|
||||||
|
|
||||||
var type = options.GetType();
|
var type = options.GetType();
|
||||||
var builder = new StringBuilder();
|
var builder = new StringBuilder();
|
||||||
|
|
||||||
@@ -188,14 +194,14 @@ namespace CommandLine
|
|||||||
optSpecs.ForEach(
|
optSpecs.ForEach(
|
||||||
opt =>
|
opt =>
|
||||||
builder
|
builder
|
||||||
.Append(FormatOption((OptionSpecification)opt.Specification, opt.Value, settings))
|
.Append(FormatOption((OptionSpecification)opt.Specification, opt.Value, settings, parsingCulture))
|
||||||
.Append(' ')
|
.Append(' ')
|
||||||
);
|
);
|
||||||
|
|
||||||
builder.AppendWhen(valSpecs.Any() && parser.Settings.EnableDashDash, "-- ");
|
builder.AppendWhen(valSpecs.Any() && parser.Settings.EnableDashDash, "-- ");
|
||||||
|
|
||||||
valSpecs.ForEach(
|
valSpecs.ForEach(
|
||||||
val => builder.Append(FormatValue(val.Specification, val.Value)).Append(' '));
|
val => builder.Append(FormatValue(val.Specification, val.Value, parsingCulture)).Append(' '));
|
||||||
|
|
||||||
return builder
|
return builder
|
||||||
.ToString().TrimEnd(' ');
|
.ToString().TrimEnd(' ');
|
||||||
@@ -213,18 +219,18 @@ namespace CommandLine
|
|||||||
{
|
{
|
||||||
return FormatCommandLine<T>(parser, options, configuration).SplitArgs();
|
return FormatCommandLine<T>(parser, options, configuration).SplitArgs();
|
||||||
}
|
}
|
||||||
private static string FormatValue(Specification spec, object value)
|
private static string FormatValue(Specification spec, object value, CultureInfo parsingCulture)
|
||||||
{
|
{
|
||||||
var builder = new StringBuilder();
|
var builder = new StringBuilder();
|
||||||
switch (spec.TargetType)
|
switch (spec.TargetType)
|
||||||
{
|
{
|
||||||
case TargetType.Scalar:
|
case TargetType.Scalar:
|
||||||
builder.Append(FormatWithQuotesIfString(value));
|
builder.Append(FormatWithQuotesIfString(value, parsingCulture));
|
||||||
break;
|
break;
|
||||||
case TargetType.Sequence:
|
case TargetType.Sequence:
|
||||||
var sep = spec.SeperatorOrSpace();
|
var sep = spec.SeperatorOrSpace();
|
||||||
Func<object, object> format = v
|
Func<object, object> format = v
|
||||||
=> sep == ' ' ? FormatWithQuotesIfString(v) : v;
|
=> sep == ' ' ? FormatWithQuotesIfString(v, parsingCulture) : FormatCulturedValue(v, parsingCulture);
|
||||||
var e = ((IEnumerable)value).GetEnumerator();
|
var e = ((IEnumerable)value).GetEnumerator();
|
||||||
while (e.MoveNext())
|
while (e.MoveNext())
|
||||||
builder.Append(format(e.Current)).Append(sep);
|
builder.Append(format(e.Current)).Append(sep);
|
||||||
@@ -234,9 +240,9 @@ namespace CommandLine
|
|||||||
return builder.ToString();
|
return builder.ToString();
|
||||||
}
|
}
|
||||||
|
|
||||||
private static object FormatWithQuotesIfString(object value)
|
private static object FormatWithQuotesIfString(object value, CultureInfo parsingCulture)
|
||||||
{
|
{
|
||||||
string s = value.ToString();
|
string s = FormatCulturedValue(value, parsingCulture);
|
||||||
if (!string.IsNullOrEmpty(s) && !s.Contains("\"") && s.Contains(" "))
|
if (!string.IsNullOrEmpty(s) && !s.Contains("\"") && s.Contains(" "))
|
||||||
return $"\"{s}\"";
|
return $"\"{s}\"";
|
||||||
|
|
||||||
@@ -248,17 +254,32 @@ namespace CommandLine
|
|||||||
? "\"".JoinTo(doubQt(v), "\"") : v, value);
|
? "\"".JoinTo(doubQt(v), "\"") : v, value);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Converts a value to its string representation using the parser's parsing
|
||||||
|
/// culture so that the unparsed command line round-trips through the parser
|
||||||
|
/// regardless of the current thread culture. Types implementing
|
||||||
|
/// <see cref="IFormattable"/> (such as numeric and date/time values) are
|
||||||
|
/// formatted with <paramref name="parsingCulture"/>; other types fall back to
|
||||||
|
/// their default string representation.
|
||||||
|
/// </summary>
|
||||||
|
private static string FormatCulturedValue(object value, CultureInfo parsingCulture)
|
||||||
|
{
|
||||||
|
return value is IFormattable formattable
|
||||||
|
? formattable.ToString(null, parsingCulture)
|
||||||
|
: value?.ToString();
|
||||||
|
}
|
||||||
|
|
||||||
private static char SeperatorOrSpace(this Specification spec)
|
private static char SeperatorOrSpace(this Specification spec)
|
||||||
{
|
{
|
||||||
return (spec as OptionSpecification).ToMaybe()
|
return (spec as OptionSpecification).ToMaybe()
|
||||||
.MapValueOrDefault(o => o.Separator != '\0' ? o.Separator : ' ', ' ');
|
.MapValueOrDefault(o => o.Separator != '\0' ? o.Separator : ' ', ' ');
|
||||||
}
|
}
|
||||||
|
|
||||||
private static string FormatOption(OptionSpecification spec, object value, UnParserSettings settings)
|
private static string FormatOption(OptionSpecification spec, object value, UnParserSettings settings, CultureInfo parsingCulture)
|
||||||
{
|
{
|
||||||
return new StringBuilder()
|
return new StringBuilder()
|
||||||
.Append(spec.FormatName(value, settings))
|
.Append(spec.FormatName(value, settings))
|
||||||
.AppendWhen(spec.TargetType != TargetType.Switch, FormatValue(spec, value))
|
.AppendWhen(spec.TargetType != TargetType.Switch, FormatValue(spec, value, parsingCulture))
|
||||||
.ToString();
|
.ToString();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,12 @@
|
|||||||
|
// Copyright 2005-2015 Giacomo Stelluti Scala & Contributors. All rights reserved. See License.md in the project root for license information.
|
||||||
|
|
||||||
|
using System.Collections.Generic;
|
||||||
|
|
||||||
|
namespace CommandLine.Tests.Fakes
|
||||||
|
{
|
||||||
|
class Options_With_Double_Sequence
|
||||||
|
{
|
||||||
|
[Option("dbls", Separator = ',')]
|
||||||
|
public IEnumerable<double> DoubleSequence { get; set; }
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -2,8 +2,10 @@
|
|||||||
|
|
||||||
using System;
|
using System;
|
||||||
using System.Collections.Generic;
|
using System.Collections.Generic;
|
||||||
|
using System.Globalization;
|
||||||
using System.IO;
|
using System.IO;
|
||||||
using System.Linq;
|
using System.Linq;
|
||||||
|
using System.Threading;
|
||||||
using Xunit;
|
using Xunit;
|
||||||
using FluentAssertions;
|
using FluentAssertions;
|
||||||
using CommandLine.Tests.Fakes;
|
using CommandLine.Tests.Fakes;
|
||||||
@@ -154,6 +156,66 @@ namespace CommandLine.Tests.Unit
|
|||||||
}
|
}
|
||||||
#endregion
|
#endregion
|
||||||
|
|
||||||
|
#region Parsing culture round-trip
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public static void UnParsing_scalar_double_uses_parsing_culture_so_it_round_trips_on_comma_decimal_cultures()
|
||||||
|
{
|
||||||
|
var originalCulture = Thread.CurrentThread.CurrentCulture;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
// A culture that uses ',' as the decimal separator (e.g. de-DE) would,
|
||||||
|
// when the ambient thread culture is used, format 1.5 as "1,5", which the
|
||||||
|
// invariant-culture parser reparses as 15 - breaking the round-trip.
|
||||||
|
Thread.CurrentThread.CurrentCulture = new CultureInfo("de-DE");
|
||||||
|
var parser = new Parser();
|
||||||
|
var options = new Simple_Options_With_Double_Value { DoubleValue = 1.5 };
|
||||||
|
|
||||||
|
var commandLine = parser.FormatCommandLine(options);
|
||||||
|
|
||||||
|
commandLine.Should().Contain("1.5");
|
||||||
|
commandLine.Should().NotContain("1,5");
|
||||||
|
|
||||||
|
var reparsed = parser.ParseArguments<Simple_Options_With_Double_Value>(commandLine.SplitArgs());
|
||||||
|
reparsed.Tag.Should().Be(ParserResultType.Parsed);
|
||||||
|
((Parsed<Simple_Options_With_Double_Value>)reparsed).Value.DoubleValue.Should().Be(1.5);
|
||||||
|
}
|
||||||
|
finally
|
||||||
|
{
|
||||||
|
Thread.CurrentThread.CurrentCulture = originalCulture;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public static void UnParsing_double_sequence_uses_parsing_culture_so_it_round_trips_on_comma_decimal_cultures()
|
||||||
|
{
|
||||||
|
var originalCulture = Thread.CurrentThread.CurrentCulture;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
// With a comma-decimal ambient culture each element would be formatted as
|
||||||
|
// "1,5"/"2,5"; joined by the ',' sequence separator this yields "1,5,2,5",
|
||||||
|
// which the parser reparses as four elements [1,5,2,5] instead of [1.5,2.5].
|
||||||
|
Thread.CurrentThread.CurrentCulture = new CultureInfo("de-DE");
|
||||||
|
var parser = new Parser();
|
||||||
|
var options = new Options_With_Double_Sequence { DoubleSequence = new[] { 1.5, 2.5 } };
|
||||||
|
|
||||||
|
var commandLine = parser.FormatCommandLine(options);
|
||||||
|
|
||||||
|
commandLine.Should().Be("--dbls 1.5,2.5");
|
||||||
|
|
||||||
|
var reparsed = parser.ParseArguments<Options_With_Double_Sequence>(commandLine.SplitArgs());
|
||||||
|
reparsed.Tag.Should().Be(ParserResultType.Parsed);
|
||||||
|
((Parsed<Options_With_Double_Sequence>)reparsed).Value.DoubleSequence
|
||||||
|
.Should().Equal(new[] { 1.5, 2.5 });
|
||||||
|
}
|
||||||
|
finally
|
||||||
|
{
|
||||||
|
Thread.CurrentThread.CurrentCulture = originalCulture;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#endregion
|
||||||
|
|
||||||
#region PR 550
|
#region PR 550
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|||||||
Reference in new issue
Block a user