LogLevel - Added support for TypeConverter

[snakefoot]

Also fixes bug where LogLevel always converts to ordinal. Even when asked for string.

See also: https://www.hanselman.com/blog/TypeConvertersTheresNotEnoughTypeDescripterGetConverterInTheWorld.aspx

Sadly enough it doesn't fully solve this issue:

https://stackoverflow.com/questions/56454590/im-having-problems-deserializing-enumerations

Because Json.Net doesn't make use of TypeConverter when class implements IConvertible. Guess there is a price of implementing IConvertible-interface for custom objects. One could consider creating a PullRequest for Json.Net to improve support for TypeConverter for classes that implements IConvertible. (Probably not a an easy task for an outsider, since by design: JamesNK/Newtonsoft.Json#676 )

https://github.com/JamesNK/Newtonsoft.Json/blob/4ab34b0461fb595805d092a46a58f35f66c84d6a/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs#L981 (See comment saying by design)

Alternative one could remove IConvertible-interface from LogLevel-class and only use TypeConverter-attribute.

Alternative one could change IConvertible-interface to return TypeCode.String for LogLevel. But it will probably have even more side-effects.

Alternative one could change LogLevel into an enum, instead of having it as class.

[codecov-io]

Codecov Report

Merging #3469 into master will decrease coverage by <1%.
The diff coverage is 80%.

@@           Coverage Diff           @@
##           master   #3469    +/-   ##
=======================================
- Coverage      80%     80%   -<1%     
=======================================
  Files         359     360     +1     
  Lines       28634   28667    +33     
  Branches     3817    3823     +6     
=======================================
+ Hits        22916   22940    +24     
- Misses       4620    4623     +3     
- Partials     1098    1104     +6

[304NotModified]

Thanks for the PR!

Alternative one could remove IConvertible-interface from LogLevel-class and only use TypeConverter-attribute.

That's pity, as the DB target / Type Converter is using that interface.

Could you please add some more tests?

PS: I like the extension, saves some boilerplate stuff

https://marketplace.visualstudio.com/items?itemName=RandomEngy.UnitTestBoilerplateGenerator

[snakefoot]

That's pity, as the DB target / Type Converter is using that interface.

Should not be that difficult to also support the TypeConverter-attribute. Then IConvertible is not needed for LogLevel.

[snakefoot]

@304NotModified Could you please add some more tests?

Have improved the test-coverage.

[304NotModified]

Thanks!

Coverage is still not ideal, but good enough for now.

[snakefoot]

@304NotModified You have added this to NLog 5.0-milestone. But it is included in the coming NLog 4.6.5 (since committed directly to master).

[304NotModified]

Yes, I noticed after merge 👼

[snakefoot]

Yes, I noticed after merge

Well it is not a breaking change. Actually think it fixes a bug, when converting LogLevel to string.