Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Nullables!!#192

Open
Happypig375 wants to merge 45 commits intoLayoutFarm:master from
Happypig375:nullable
Open

Nullables!! #192
Happypig375 wants to merge 45 commits intoLayoutFarm:master from
Happypig375:nullable

Conversation

@Happypig375
Copy link
Contributor

@Happypig375 Happypig375 commented Mar 25, 2020

Closes #191

Nullable reference types are now enabled for:

  • Typography.OpenFont
  • Typography.GlyphLayout
  • Typography.MsdfGen
  • Typography.TextBreak
  • Typography.TextFlow
  • Typography.TextServices
  • Typography.One

Nullable warnings from above projects will be treated as errors to ensure that nullability seen by users is truthful. If they stay as warnings, it is very possible that they will be ignored like the rest of Typography's warnings 😉 They are not enabled for PixelFarm projects - annotating them will need action from the PixelFarm repo as well.

Additional goodies:

  • C# 8.0 enabled for all projects
  • More code sharing - eliminated some duplicate files, e.g. FontManagement.cs was in both Typography.TextFlow and Demo/DevTextPrinter
  • SDK-style multi-targeting build projects! This is necessary because of <Nullable> has no effect in old-style csproj dotnet/project-system#5551
  • Typography.One now does not reference Unpack. This is because of
  1. The code in Unpack is too much to annotate
  2. Users will very likely use System.IO.Compression instead which includes all of Unpack's functionality (Brotli, Deflate and GZip compression) with the added benefit of performance

P.S. This is probably my largest PR ever 😆 🎉

Copy link
Member

prepare commented Mar 26, 2020

@Happypig375 ,

Thank you for your PR.

At this time,
The PixelFarm lib is under 'refactoring phase'.

After that => I will review this PR soon.

Copy link
Contributor

zwcloud commented Mar 26, 2020
edited
Loading

So many logic changes. Have you gone through all the visual demos to make sure nothing is broken?
It is not enough to be buildable.

Copy link
Contributor Author

I did not try the Android/iOS demos but Windows demos work just fine.

zwcloud reacted with thumbs up emoji

Copy link
Contributor

zwcloud commented Mar 26, 2020
edited
Loading

@prepare You can pick some important APIs and make a list of them. I'd like to help adding unit tests on them. Visual correctness is quite fragile, I think.

Happypig375 added 15 commits April 9, 2020 14:19
Copy link

@prepare I would suggest merging this in relatively fast if possible. Enabling NRTs is good hygiene and clarifies any current structure, which will only help any future refactoring. This PR represents a lot of work and conflicts will accumulate fast.

Copy link
Contributor Author

@prepare When will this be merged? I'll fix conflicts if you confirm that this will be merged.

Copy link
Member

prepare commented May 29, 2020

@Happypig375

I need to test this at least with the
PixelFarm and HtmlRenderer and agg-sharp (MatterHackers/agg-sharp#1307) too.

Copy link
Contributor Author

A few days ago the amount of conflicting files could fit inside the GitHub interface. Now it is off the charts.

Copy link
Contributor Author

@prepare What's the progress of testing this PR?

Copy link
Contributor Author

The silence is deafening.

Copy link

Note @Happypig375 :

#181 (comment)
I will remove old projects for .net2.0-3.5 in this December too.

The presence of the obsolete dotnet projects in this repo is causing a lot of the diff in this PR. I assume it won't be hard to merge any change removing these projects into this PR (the main work being the existing conflicts), since it would be mainly be deleting a lot of files. @prepare do you agree that would make this PR reviewable?

Copy link
Contributor

virzak commented Mar 5, 2021

So because #221 merged, you have to resolve now. Let me know if you need any help with it.

Copy link
Contributor Author

@virzak Seems not worth the work if it's not going to be merged anyways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Enable nullable?

Comments

AltStyle によって変換されたページ (->オリジナル) /