Move Metadata and LicenseInfo so they can be accessed independent of Windows Forms

[aror92]

Winforms-independent functionality of Metadata, CreativeCommonsLicense, and CustomLicense is moved to the new classes MetadataBare, CreativeCommonsLicenseBare, and CustomLicenseBare in SIL.Core.Clearshare.

Metadata, CreativeCommonsLicense, and CustomLicense inherit from the Bare Winforms-free versions.

Make LicenseInfo class independent of Windows Forms and move it to SIL.Core.Clearshare. - Move the FromXmp method to LicenseUtils and LicenseWithImageUtils to construct Winforms-independent and Winforms-dependent license types respectively. - Move the FromToken method to LicenseWithLogo, so it can return the Winforms-dependent license types. FromToken is only used in libpalaso tests and examples, and a Winforms-independent version of this method is not needed. - Move the GetImage method to the ILicenseWithImage interface, which is implemented by CreativeCommonsLicense and CustomLicense.

---

This change is 

[github-actions[bot]]

Palaso Tests

     4 files  ± 0       4 suites  ±0   11m 10s :stopwatch: + 1m 24s  5 092 tests +22   4 858 :white_check_mark: +22  234 :zzz: ±0  0 :x: ±0  16 591 runs  +66  15 870 :white_check_mark: +66  721 :zzz: ±0  0 :x: ±0 

Results for commit f4e20406. ± Comparison against base commit ae035eb0.

This pull request removes 32 and adds 54 tests. Note that renamed tests count towards both.

SIL.Windows.Forms.Tests.ClearShare.LicenseInfoTests ‑ FromToken_Ask_GiveCustomLicense
SIL.Windows.Forms.Tests.ClearShare.LicenseInfoTests ‑ FromToken_Ask_GiveNullLicense
SIL.Windows.Forms.Tests.ClearShare.LicenseInfoTests ‑ FromToken_CreativeCommons_GiveExpectedAttributes
SIL.Windows.Forms.Tests.ClearShare.LicenseInfoTests ‑ Token_GivenCreativeCommonsLicense
SIL.Windows.Forms.Tests.ClearShare.LicenseInfoTests ‑ Token_GivenCustomLicense_IsCustom
SIL.Windows.Forms.Tests.ClearShare.LicenseInfoTests ‑ Token_GivenNullLicense_IsAsk
SIL.Windows.Forms.Tests.ClearShare.MetadataTests ‑ ChangeLicenseDetails_HasChanges_True
SIL.Windows.Forms.Tests.ClearShare.MetadataTests ‑ ChangeLicenseObject_HasChanges_True
SIL.Windows.Forms.Tests.ClearShare.MetadataTests ‑ GetCopyrightBy_Empty_ReturnsEmpty
SIL.Windows.Forms.Tests.ClearShare.MetadataTests ‑ GetCopyrightBy_HandlesMultilineCopyrightHolder
…

SIL.Tests.ClearShare.MetadataCoreTests ‑ CanRemoveRightsStatment
SIL.Tests.ClearShare.MetadataCoreTests ‑ ChangeLicenseDetails_HasChanges_True
SIL.Tests.ClearShare.MetadataCoreTests ‑ ChangeLicenseObject_HasChanges_True
SIL.Tests.ClearShare.MetadataCoreTests ‑ ChangingFromCCLicense_WorksOkay
SIL.Tests.ClearShare.MetadataCoreTests ‑ DeepCopy
SIL.Tests.ClearShare.MetadataCoreTests ‑ GetCopyrightBy_Empty_ReturnsEmpty
SIL.Tests.ClearShare.MetadataCoreTests ‑ GetCopyrightBy_HandlesMultilineCopyrightHolder
SIL.Tests.ClearShare.MetadataCoreTests ‑ GetCopyrightBy_HasCOPYRIGHTAndSymbolNoYear_ReturnsCopyrightHolder
SIL.Tests.ClearShare.MetadataCoreTests ‑ GetCopyrightBy_HasCopyrightAndSymbolAndComma_ReturnsCopyrightHolder
SIL.Tests.ClearShare.MetadataCoreTests ‑ GetCopyrightBy_HasCopyrightAndSymbolNoYear_ReturnsCopyrightHolder
…

:recycle: This comment has been updated with latest results.

[andrew-polk]

Not worth a lot of head time, but worth a question: Is it better to release 17.0 before this goes in so clients can decide to take the l10nsharp breaking change without this breaking change? Or is it better to include two breaking changes together in one major version release?

Bloom, getting ready to start a full regression test pass before releasing, would probably prefer to do so on a version without this change. (We already have the l10nsharp change.) But we are currently just using the beta version, so I'm not sure it matters much. Where it could potentially matter is if we needed to hot-fix 17.0. (But we don't really have a build system set up to do that anyway, do we??)

[andrew-polk]

SIL.Core/SIL.Core.csproj line 24 at r2 (raw file):

    <PackageReference Include="Mono.Unix" Version="7.1.0-final.1.21458.1" />
    <PackageReference Include="L10NSharp" Version="9.0.0-*" />
    <PackageReference Include="TagLibSharp" Version="2.3.0" />

I wonder if there are implications of adding these libraries we need to think through.

1. Increases size of SIL.Core's dependencies by about 0.6MB (probably ok...).
2. We worked hard at some point in the past to remove L10NSharp from SIL.Core. Probably that was just because of the WinForms dependency? But I don't remember if there were other reasons. It would be a bit unfortunate to add the dependency just for a couple translations of license messages. Have you considered using Localizer instead?

Another option which likely isn't worth it is to put all this clearshare stuff in a new package called SIL.Core.Clearshare or SIL.Clearshare

[tombogle]

SIL.Core/ClearShare/CreativeCommonsLicenseBare.cs line 12 at r4 (raw file):

Previously, aror92 (Ariel Ror.) wrote…

What do you think of one of these names instead for the "Bare" version?: "CreativeCommonsLicenseText" or "CreativeCommonsLicenseInfo".

Jason put high priority on keeping things as much the same in the Winforms side as we can. I'm really wanting to keep the names in the Winforms versions as they were in order to minimize pain and confusion of the breaking change, and to break as few things as possible.

I had considered using the same names, but I think it would negatively impact code readability (e.g. which version is being used is harder to see). And it could add confusion, since there are plenty of instances where both namespaces are used in the same code.

CreativeCommonsLicenseInfo works

[jasonleenaylor]

SIL.Core/ClearShare/LicenseUtils.cs line 11 at r5 (raw file):

Previously, tombogle (Tom Bogle) wrote…

I think it might be more intuitive for each concrete subclass of licenseInfo to implement its own staticTryCreateFromMetadata method, and just call those in succession inside FromXmp

@tombogle It has been particularly tricky to get this to work well in consuming classes in an intuitive way. We have tried a number of different static and inheritance options. We put a high value on making this as non-disruptive as possible on the existing consumers who do not need the classes formerly known as 'bare'. Is this a make or break issue for you?

[tombogle]

SIL.Core/ClearShare/LicenseUtils.cs line 11 at r5 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

@tombogle It has been particularly tricky to get this to work well in consuming classes in an intuitive way. We have tried a number of different static and inheritance options. We put a high value on making this as non-disruptive as possible on the existing consumers who do not need the classes formerly known as 'bare'. Is this a make or break issue for you?

No, but I think what I'm proposing would not be a significant change that could break anything. I actually explored several more disruptive ideas, but they involved reflection, etc., and I wasn't really comfortable with any of them. Moving the implementation details would just get those details closer to the class that should know and care.

[tombogle]

SIL.Windows.Forms/ClearShare/WinFormsUI/MetadataEditorControl.cs line 76 at r10 (raw file):

				_copyrightBy.Text = _metadata.GetCopyrightBy();
				if (_metadata.License!=null && (_metadata.License is ILicenseWithImage))

Can also be simplified

Code snippet:

if (_metadata.License is ILicenseWithImage licenseWithImage)
    _licenseImage.Image = licenseWithImage.GetImage();