Add IList<object> support #154
Conversation
Update to latest changes
# Conflicts: # src/SmartFormat/Core/Formatting/FormatDetails.cs # src/SmartFormat/SmartFormatter.cs
|
5 failing CI tests seem to be related to |
Oh that's strange. I'll see if I can figure out what's going wrong, I wasn't able to get the tests to run on my machine but I think it was because I was using a Mac. |
src/SmartFormat/SmartFormatter.cs
Outdated
| /// <returns>Returns the formatted input with items replaced with their string representation.</returns> | ||
| public string FormatWithCache(ref FormatCache? cache, IList<object> args, string format) | ||
| { | ||
| args = args ?? k_Empty; |
There was a problem hiding this comment.
Should read:
args ??= k_Empty;
in case args is nullable - otherwise this line is redundant
There was a problem hiding this comment.
Ah yes our version of dotnet does not support that.
src/SmartFormat/SmartFormatter.cs
Outdated
| /// <param name="format">The format string.</param> | ||
| public void FormatWithCacheInto(ref FormatCache cache, IOutput output, IList<object> args, string format) | ||
| { | ||
| args = args ?? k_Empty; |
There was a problem hiding this comment.
Should read:
args ??= k_Empty;
in case args is nullable - otherwise this line is redundant
src/SmartFormat/SmartFormatter.cs
Outdated
| @@ -23,6 +22,8 @@ public class SmartFormatter | |||
| { | |||
| #region : EventHandlers : | |||
|
|
|||
| static readonly object[] k_Empty = { null }; | |||
src/SmartFormat/SmartFormatter.cs
Outdated
| @@ -149,9 +161,22 @@ public string Format(string format, params object[] args) | |||
| /// <returns>Returns the formatted input with items replaced with their string representation.</returns> | |||
| public string Format(IFormatProvider? provider, string format, params object[] args) | |||
| { | |||
| var output = new StringOutput(format.Length + args.Length * 8); | |||
| return Format(null, args, format); | |||
There was a problem hiding this comment.
| return Format(null, args, format); | |
| return Format(provider, args, format); |
This resolves failing unit tests
src/SmartFormat/SmartFormatter.cs
Outdated
| /// <param name="format">The format string.</param> | ||
| public void FormatInto(IOutput output, IList<object> args, string format) | ||
| { | ||
| args = args ?? k_Empty; |
There was a problem hiding this comment.
| args = args ?? k_Empty; | |
| args ??= k_Empty; |
argsis not marked nullable. In this case, the line is redundant.
If this is the only reason I would rather keep the existing order of arguments. |
|
is another source more appropriate? |
Codecov Report
@@ Coverage Diff @@
## version/v3.0 #154 +/- ##
===========================================
- Coverage 94% 94% -0%
===========================================
Files 42 42
Lines 1749 1769 +20
===========================================
+ Hits 1651 1667 +16
- Misses 98 102 +4
Continue to review full report at Codecov.
|
|
|
@gyssels What is the comment referring to? |
|
Looks good and makes sense, thanks. |
There's cases where it may change the behaviour. E.g if you are passing in a list as an argument at index 0, it would now be treated as a list of arguments instead. |
* Added support for IList<object> when passing arguments. * Changed order of params * Revert merge changes * Fixed some code that should not have been changed. * Fixed failing tests * Reverted order of params, added unit tests Co-authored-by: axunonb <[email protected]>
This changes the internal list from an array to an IList so it's more flexible and can work with Lists etc.
We use a lot of pooled Lists internally to reduce garbage and arrays make it hard to do this.
I added a version for IList to all the main Format methods and swapped the order of the arguments to prevent accidentally using the wrong version.