Convert lib to DotNet Standard and create sample Core and Framework p…

[alko89]

I downgraded CodePageStringRegion due to uncompatibility with DotNet Framework and moved the code to DotNet Standard.

I then moved the Program.cs code to core project and wrote a sample Framework project to test that it works with both frameworks (DotNet Core 2.0 and DotNet Framework 4.6.1).

[davispuh]

You've broken encoding handling for CodePageStringRegion, it won't work.

codepage = reader.ReadByte();
...
enc = Encoding.GetEncoding(codepage);

you can't do this... Encoding.GetEncoding will use Windows CodePage number, but here we have Tachograph code pages which are totally different.

I don't understand what incomparability you had. Current code works correctly on .NET Core. And I think it should work exactly same with Framework too.

[alko89]

I had an issue with GetValueOrDefault and had to create a quick fix for it. I tested the code with .NET portability analizer and it was the issue why I was unable to reference the project in .NET Framework.

I think its a good idea to use .NET Standard for portability reasons.

[davispuh]

Then correct way would be to just rewrite it like

if (charsetMapping.ContainsKey(codepage))
{
    encodingName = charsetMapping[codepage];
} else
{
    encodingName = "UNKNOWN";
}

and so on for all places.

[alko89]

I was not thinking much...do you mean like this?

protected override void ProcessInternal(CustomBinaryReader reader, XmlWriter writer)
        {
            // get the codepage
            var codepage = reader.ReadByte();
            // codePage specifies the part of the ISO/IEC 8859 used to code this string

            string encodingName;
            if (charsetMapping.ContainsKey(codepage))
            {
                encodingName = charsetMapping[codepage];
            }
            else
            {
                encodingName = "UNKNOWN";
            }

            Encoding enc;
            if (encodingCache.ContainsKey(encodingName))
            {
                enc = encodingCache[encodingName];
            }
            else
            {
                enc = null;
            }

            if (enc == null)
            {
                // we want to warn if we didn't recognize codepage because using wrong codepage will cause use of wrong codepoints and thus incorrect data
                WriteLine(LogLevel.WARN, "Unknown codepage {0}", codepage);
                enc = Encoding.ASCII;
            }

            // read string using encoding
            base.ProcessInternal(reader, enc);
            writer.WriteString(text);
        }

[davispuh]

Yeah, but it's actually even simpler.

            string encodingName = "UNKNOWN";
            if (charsetMapping.ContainsKey(codepage))
            {
                encodingName = charsetMapping[codepage];
            }

            Encoding enc = null;
            if (encodingCache.ContainsKey(encodingName))
            {
                enc = encodingCache[encodingName];
            }

Or alternatively, could implement GetValueOrDefault method here so it could be used.

[alko89]

Thanks for your help! I commited a fix CodePageStringRegion.

[davispuh]

You probably should squash all commits and remove those whitespace changes and wrong comment for CodePageStringRegion. It should contain only GetValueOrDefault change and not those other unrelated.

[alko89]

OK, I just have to test this in my project first and then I'll fix the comments and whitespaces.

[alko89]

Should I revert commits or can I just fix the whitespaces and comments?

[davispuh]

Squash commits using git, see https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History

Basically it's

$ git rebase -i HEAD~4
// there type so last 3 are "s"  

$ git push --force

[alko89]

Done. I think its ok now.

[alko89]

I made fixes to the project as we discused. Funny thing the examples can be exactly the same, but won't compile on a different framework if not used from standard lib.

[davispuh]

It looks really great now, thanks! But there's still few minor things, unrelated whitespace changes and ({). Also would be much nicer to clean up commit history by rebasing on master and squashing commits. But it's fine, I can do it myself when I'll have time on this.

Funny thing the examples can be exactly the same, but won't compile on a different framework if not used from standard lib.

that's really unfortunate and I really don't like this duplication. Did you tried to move it inside src and then reference in project file? what if it were inside a different directory like app/Program.cs and reference from it.

[alko89]

Sorry, the whitespaces are an issue with the IDE, I'll be careful next time.

I guess the right way would be, to make two examples that would only compile with Core or Framework each. But that is kind of tricky, since Framework and Core are actualy the same and not at the same time...the dotnet world is strange IMHO.

[davispuh]

I updated and merged this with 27089fb and d840e18