andywilsonuk / StringTokenFormatter

Provides string extension methods for replacing tokens within strings (using the format '{name}') with their specified lookup value.
Apache License 2.0
37 stars 1 forks source link

v6.x not available on NuGet (yet) #22

Closed kp-xcess closed 2 years ago

kp-xcess commented 2 years ago

Hi, I was just investigating a case where a certain token (containing a colon) is not being replaced. Then I saw that there is now a version 6 that should be faster, plus more customizable. Perhaps it can even solve my current issue.

However; this version 6 is apparently not in NuGet (and yes; my project is already on .NET 6). I could of course clone this repository and cook up my own DLL to reference, but in this here era I strongly prefer using NuGet 😉

Can you push the new version to NuGet?

TonyValenti commented 2 years ago

I'll work on getting this done tomorrow. Please note that v6 changes the API surface completely so you may need to refactor some code.

TonyValenti commented 2 years ago

@kp-xcess Actually, can you post a small snippet of code that would repro the issue? I'll add a test case and make sure it work.

kp-xcess commented 2 years ago

Of course! I wrote a test that demonstrates the issue:

[InlineAutoData("TokenWithoutColon")]
[InlineAutoData("Token:With:Colons")]
public void Token_IsReplacedCorrectly(string token, string value)
{
    // Arrange
    string template = $"Some template containing {{{token}}}";
    string expected = $"Some template containing {value}";

    IDictionary<string, string> values = new Dictionary<string, string>
    {
        { token, value }
    };

    // Act
    string actual = template.FormatDictionary(values);

    // Assert
    Assert.That(actual, Is.EqualTo(expected));
}

First case (InlineAutoData) succeeds, the latter fails.

Another test that demonstrates what token it uses if the original contains a colon:

[InlineAutoData("TokenWithoutColon")]
[InlineAutoData("Token:With:Colons")]
public void Token_IsDetected(string token)
{
    // Arrange
    string template = $"Just wrapping {{{token}}} around a bit";
    string expected = token;

    // Act
    SegmentedString segmentedString = SegmentedString.Parse(template);

    IReadOnlyCollection<IMatchedToken> matchedTokens = segmentedString.Segments
        .OfType<IMatchedToken>()
        .ToArray();

    Assume.That(matchedTokens, Has.Exactly(1).Items);

    string actual = matchedTokens.First().Token;

    // Assert
    Assert.That(actual, Is.EqualTo(expected));
}

Just like with the previous test; the first case succeeds, second one fails. Message:

Expected string length 17 but was 5. Strings differ at index 5. Expected: "Token:With:Colons" But was: "Token" ----------------^

Note that we're using NUnit (with AutoFixture) for our tests.

kp-xcess commented 2 years ago

I noticed that in V6 of your library, the colon is used to optionally specify formatting information for the token's value (alike the way it works with string interpolation/String.Format(...)).

Therefore it will probably be a bit tricky to accept colons both as part of the token itself, and as a formatting information separator too. If it is, you might decide to just not support it. And I'd understand. A workaround however could of course be to allow users to optionally change the character/string to use as the formatting separator.

TonyValenti commented 2 years ago

I was thinking about this and I think you're right. From memory, the regex expects the colon to be part of a format specified so I'm not sure on a good way to also support it as part of the token name.

Are you able to replace the colon with another character? For example, an underscore?

kp-xcess commented 2 years ago

Even better; I already did. Decided to use the pipe character for my purposes 😉

kp-xcess commented 2 years ago

I'll work on getting this done tomorrow. Please note that v6 changes the API surface completely so you may need to refactor some code.

I noticed that you've closed the issue, but it appears V6 is not yet on NuGet. Can you still give the new version a push?