Enable nullable?

[Happypig375]

If you accept incorporating nullable reference types then I will start working on an annotation PR 😃

[zwcloud]

No. I don't think current Typography's API structures and code quality are good enough to be refactored to enable nullable.

Typography should be refactored thoroughly to clean up APIs first, then start migrate to nullable reference. Some points:

• abuse public method, properties: reduce public matters
• so many temp/middle classes and structs: reduce indirections and unnessary custom structures
• ambiguous naming: Glyph, GlyphPlan, GlyphSetPosition, GlyphPointF
• missing unit test: Typography's usefulness is proven through large numbers of visual demos but the lack of unit test is quite painful when one want to refactor: going through visual demos is not a efficient and human-friendly way to test if the refactor modification breaks anything.

[Happypig375]

I would say that code quality is independent of nullable annotations.

Nullable annotations require little to no refactors. It is just adding ?s after the types, which just expresses intent of whether nulls are accepted. Therefore, any problems arising from refactoring do not exist.

[zwcloud]

@Happypig375 Emm.. I think you can just have a try. There will be ? and ! everywhere and makes nullable reference meaningless, and in turn makes the code not readable.

And if nullable reference warnings are ignored, then there is no point to enable that.

[Happypig375]

Normally ! won't be required but I'll try.

[prepare]

Thank you @Happypig375 ,

I agree with @zwcloud.

The library needs some (many) arrangements.

see also: https://docs.microsoft.com/en-us/dotnet/csharp/nullable-attributes https://stackoverflow.com/questions/55492214/the-annotation-for-nullable-reference-types-should-only-be-used-in-code-within-a

https://devblogs.microsoft.com/dotnet/embracing-nullable-reference-types/

[charlesroddie]

@zwcloud There will be ? and ! everywhere

@Happypig375 when you implemented this what did you find? How much is safety improved, or is it that only the unsafe places have been identified? Did everything get ?s and !s or was the compiler identifying a lot of safe paths?

@zwcloud @prepare do you still maintain that other cleanup tasks have to be done first? I don't see why that argument holds; these seem independent. Do you have a suggested path to get NRTs in? Is the difficulty the need to test, or that the changes have been too extensive to review?

The official .Net guidance is that NRTs should become ubiquitous with in the .Net 5 timeframe, i.e. by Nov, but of course there are parts of .Net that have historically been slow to update.

[prepare]

@prepare do you still maintain that other cleanup tasks have to be done first?

Yes, This week task=> Typography.TextServices is under development and refactoring.

and I need to test with these too (https://github.com/LayoutFarm/Typography/pull/192#issuecomment-635820167)

[Happypig375]

when you implemented this what did you find?

Legacy-style projects (non-SDK projects) do not support nullable reference types: https://github.com/dotnet/project-system/issues/5551

I had to include refactors to merge the .NET Framework 2.0 projects with the .NET Standard 2.0 projects into multi-targeting SDK-style projects.

How much is safety improved, or is it that only the unsafe places have been identified?

Yes. However, initialization subroutines called by constructors which initializes fields to non-nullable states were not tracked, leading to the constructor not being recognized as initializing all non-nullable fields. This can be directly solved by MemberNotNullAttribute as specified in https://github.com/dotnet/csharplang/issues/3297 in the future, but modifying these initialization subroutines to be functions instead, and performing assignment in the constructor just makes the logic more clear.

Did everything get ?s and !s or was the compiler identifying a lot of safe paths?

I avoided the use of ! as much as possible because each ! represents a loss of nullable safety. In cases where the nullability of vars are inferred wrongly, spelling out the type usually solves the issue.

[zwcloud]

nullable just conflicts with my project's configuration, so I cannot include Typography's source code inside the project directly. To support that, I will be forced to make unbelievably HUGE effort to add nullable to my project.

Why I need to use Typography's source code? => Because it is not stable enough and often ran into exceptions.

[Happypig375]

Just add another project with nullable enabled containing Typography and reference it by your existing project.

[charlesroddie]

I had to include refactors to merge the .NET Framework 2.0 projects with the .NET Standard 2.0 projects into multi-targeting SDK-style projects.

@prepare I see that not all projects here have been updated to sdk/netstandard2.0. Is the change to netstandard still in progress?

[prepare]

Yes, not all projects have been updated to sdk/netstandard2.0

I support .net framework 2.0 and netstandard2.0 projects (https://github.com/LayoutFarm/Typography/tree/master/Build). (But still not in multi-targeting SDK-style projects)

Both set have active maintenance.

[charlesroddie]

I support .net framework 2.0 and netstandard2.0 projects

That is crazy!