crozone / FormatWith

String extensions for named parameterized string formatting.
MIT License
73 stars 13 forks source link

Updated release? #17

Closed JeremySkinner closed 4 years ago

JeremySkinner commented 4 years ago

Hi @crozone,

I'm the author of the FluentValidation library. We currently process named formats in our error messages using a regex, but I've been considering switching to use FormatWith as it performs better.

Unfortunately the current release on nuget doesn't support format arguments (such as "{foo:c3}"), which we require, but I notice that this was committed last year but hasn't been released.

I was wondering if this library is still under development and if you're planning on pushing out an updated release at any point? If not, would you mind if I forked the source code and made use of the parts that I need within FluentValidation?

Thanks!

crozone commented 4 years ago

Hi @JeremySkinner,

Apologies for the lack of a 3.0.0 release, I was intending to rework the parser to remove the use of IEnumerable<T>, use Span<T>, and then release that as 3.0.0, but other things came up and I never finished the feature.

Given you already have a use case for the current code and it works right now, I will release the current version as 3.0.0 to NuGet (I really should have done this a long time ago). I should have it up on NuGet.org within a day.

I do still intend to finish the Span<T> integration at some point, but that can become version 4.0.0 if/when that actually happens. This is probably a better strategy anyway since adding Span<T> will change the API and require netstandard2.1, which would lock a lot of people out of the format argument feature.

I was wondering if this library is still under development and if you're planning on pushing out an updated release at any point? If not, would you mind if I forked the source code and made use of the parts that I need within FluentValidation?

I still consider this library under active development, and fixing any issues that are found is a very high priority going forward. I'm also open to feature requests. However, you are also very welcome to fork the project or use the code in any way you wish, as long as it complies with the MIT license. Given FluentValidation appears to use the Apache 2.0 license there should be no issues with this 🙂

crozone commented 4 years ago

FormatWith 3.0.0 has been released to NuGet.org:

https://www.nuget.org/packages/FormatWith/3.0.0

JeremySkinner commented 4 years ago

Thanks! I'll try and have a play with this in more detail over the next couple of weeks.

Span<T> support would be awesome, but yes the netstandard2.1 requirement means we wouldn't be able to use it yet (we still target netstandard2.0 and net461 as our minimum supported platform, although I'll be reviewing this for the next major release). If you do decide to add this, it'd be helpful if you could multi-target so netstandard2 consumers would still make use of the non-span based implementation (although I appreciate that this would increase the maintenance overhead, so totally understand if you wouldn't want to).

The other thing that occurred to me which would be useful is the ability to tokenize the string but not perform the replacement immediately, which would allow for the tokenized string to be cached and then formatted at a later time. It's not possible to do this at the moment as FormatHelpers is internal. I was imagining something like this:

// this could then be cached & reused.
TokenizedString str = TokenizedString.Tokenize("Hello {Name}");

// ...later...
string result = str.FormatWith(placeholders);

This may not actually be worth it as it's so fast already, but was something that occurred to me. I can open that as a separate issue if you think it's a good idea.

crozone commented 4 years ago

If you do decide to add this, it'd be helpful if you could multi-target so netstandard2 consumers would still make use of the non-span based implementation

This should be fairly straightforward with ifdefs. Normally this could get messy, but the codebase is small enough that shouldn't be too bad to split it into two implementations (I'll probably use the partial class over multiple file trick that MS uses in .NET Core). Also, the Span<T> implementations can include wrappers that maintain backwards compatibility with the current API.

The other thing that occurred to me which would be useful is the ability to tokenize the string but not perform the replacement immediately, which would allow for the tokenized string to be cached and then formatted at a later time.

Currently we have the ability to spit out a FormattableString with the FormattableWith() methods which might work for some usecases, but making the Tokenizer more open as well shouldn't be too much of an issue. A separate issue would be great if you think it's a good idea 🙂 One thing to note is that the a Span<T> tokenizer would probably be zero-allocating static method, so caching the output would turn into a tradeoff between object allocations and computational cost.. just something to think about.