svg-net / SVG

Fork of the ms svg library
http://svg-net.github.io/SVG/
Microsoft Public License
1.15k stars 473 forks source link

Cannot inherit from SVGElement due to two internal abstract properties #1162

Open sunyudai opened 1 month ago

sunyudai commented 1 month ago

Description

I am trying to extend SvgElement with the intent to create some project specific types that will still render to proper svg. However, I am running into an issue where I cannot inherit from SvgElement directly because it has two members that are marked as both internal and abstract. These properties are ClassNames and AttributeName.

Used Versions / issue location

Found when referencing the 3.4.7 NuGet package for a .NET Framework 4.8 project, but I can see the issue in the master branch at SVG/Generators/AvailableElementsGenerator.cs on lines 248 and 250.

    internal abstract string AttributeName {{ get; }}

    internal abstract List<Type> ClassNames {{ get; }}

Edit: historical note - these properties have been marked internal abstract since 2021, the initial creation of the file.

Next Steps

I'm happy to quickly change that and create a Pull request if that is preferred, I'm opening the issue because I'm not quite clear on if there is a reason why these two properties might be marked as internal abstract.

If these should not be changed to, say, protected virtual, then what would be a more appropriate class to derive from?

mrbean-bremen commented 1 month ago

@wieslawsoltes - as you had added that code, can you please check if changing these methods to protected virtual would be a problem?

sunyudai commented 1 month ago

Looks like lines 314 and 316 also need to be protected (overrides for these two), after that all tests pass.

mrbean-bremen commented 1 month ago

I'll wait some time for @wieslawsoltes to give his opinion, otherwise I see no reason for not making that change.

wieslawsoltes commented 1 month ago

@wieslawsoltes - as you had added that code, can you please check if changing these methods to protected virtual would be a problem?

I guess we could make all properties & methods generated in AvailableElementsGenerator make protected so anyone can create manually new svg element classes not relying on generator.

sunyudai commented 1 month ago

Sounds good to me, I'll see if I can get that in tonight.

Thanks for the prompt answer!

mrbean-bremen commented 2 weeks ago

@sunyudai - are you still on it?